generalize some mv code for ln, cp by sylvestre · Pull Request #14623 · uutils/coreutils

sylvestre · 2026-09-16T21:06:06Z

No description provided.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

replace links atomically instead of unlinking first Forced replacement was unlink(dst) then create. The gap lets another user claim dst in a directory they can write to -- including a name the sticky bit forbids them to remove -- so a privileged ln hands them a trusted name.

Renaming the temp onto a destination that is already a link to the same inode does nothing and returns success, so the temp name survived: `ln -f a b` with b already hard-linked to a left a stray Cu* file. Unlink the temp after the rename either way; on the normal path it is already gone.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

github-actions · 2026-09-16T23:24:25Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/retry (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/symlink (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/tail/tail-n0f is now being skipped but was previously passing.
Congrats! The gnu test tests/cut/bounded-memory is now passing!
Skip an intermittent issue tests/pr/bounded-memory (was skipped on 'main', now failing)

codspeed · 2026-09-16T23:58:45Z

Merging this PR will improve performance by 3.24%

⚠️

Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 366 untouched benchmarks
⏩ 50 skipped benchmarks¹

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
⚡	Simulation	`du_wide_tree[(5000, 500)]`	18.9 ms	18.3 ms	+3.24%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.

_{Comparing sylvestre:fix-3834-atomic-link-replace (8fcb17a) with main (ebcdac1)}

50 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

xtqqczze · 2026-09-17T01:00:25Z

test_force_replace_never_leaves_the_destination_name_free is failing on wasm32-wasip1 target since this was merged, tracked by #14632.

A THIRD independent root cause, and this one is repo-wide: it blocks the Manifest job on EVERY variant, and nothing in this repository changed to cause it. ln: failed to create symbolic link '/bin/sh': Not a directory The `Install dependencies` step installs uutils, which replaces coreutils in PATH, so the very next line runs uutils' ln. uutils 0.12.0 rewrote the -f path to "replace links atomically instead of unlinking first" (uutils/coreutils#14623; a platform-specific follow-up is already filed as #14632). `ln -sf` takes exactly that rewritten path. The container is pinned by digest but `apk add` is not, so the bump landed mid-run and split one run cleanly by the clock. Build Bonito 35202293222, one run, same image, same command: base 09:23, base-hwe 09:45, base-nvidia 09:49, cosmic 09:52, niri 09:52, gnome 09:54 ALL PASS (uutils 0.11.0-r1) xfce 09:58:55, kde 10:02:03 BOTH FAIL (uutils 0.12.0-r0) Build Gurnard 35205505527 hit the identical error at 10:11 in pantheon/Manifest, which is what rules out a bonito-specific cause. `rm` followed by a plain `ln -s` never enters the rewritten -f path and behaves the same under GNU, busybox and uutils. NOT pinned to uutils 0.11.0-r1 on purpose. A pin here works, but it needs someone to notice and lift it once upstream fixes the regression, and this step already showed what an unpinned moving dependency costs. Removing /bin/sh mid-script is safe even though the step itself runs under `sh -e`: a running shell survives its own binary being unlinked, and its subshells fork the loaded image rather than re-exec the path. Verified both that and the symlinked-parent case (/bin -> usr/bin, matching wolfi-base) locally. Validated: yamllint under ./.yamllint.yml exits 0 with the same 7 pre-existing line-length warnings as origin/main; the workflow still parses; the 235 workflow tests pass. This merges origin/main first so the change does not revert #2559's Renovate bump of setup-runner to a23fee8. CANNOT be proven by this PR's CI: the job runs on workflow_dispatch/schedule against main, never on pull requests. The real test is the first build after merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L59jmWZu8kiH2G9Jx7tcws

xtqqczze · 2026-09-18T18:26:24Z

By the way, rustix::fs::symlinkat doesn't exist on redox, so the fs feature now fails to build:

$ cargo clippy -p uucore --features fs --all-targets --target x86_64-unknown-redox
error[E0425]: cannot find function `symlinkat` in module `rustix::fs`
    --> src/uucore/src/lib/features/fs.rs:1250:21
     |
1250 |         rustix::fs::symlinkat(target, rustix::fs::CWD, dest).map_err(Into::into)

sylvestre · 2026-09-18T18:32:48Z

this is why we need CI :)

oech3 · 2026-09-19T08:29:49Z

I think we should abandon redox as no one can maintain.

xtqqczze · 2026-09-19T20:31:32Z

@sylvestre Sorry, I think we should revert this; it is the only thing blocking a clean build on Redox.

Then we can add check-only Redox job to CI:

cargo check -all-targets --features feat_os_unix_redox --target x86_64-unknown-redox

sylvestre · 2026-09-19T20:33:33Z

i am sorry but i am not going to revert such change for an OS for which we don't have CI

xtqqczze · 2026-09-19T20:50:24Z

@sylvestre I have uutils building without errors locally now, so I think we’re very close to being able to add CI. From my side, the remaining pieces are:

revert this PR
fix: address redox build issues #14620
libc 0.2.190 (which is days away)
Revert "tests: replace rlimit dependency with rustix::process" #14739

I’d really like to avoid letting the code rot in the meantime.

sylvestre · 2026-09-19T20:59:15Z

this PR fixes security issues
so, i am sorry but reverting isn't an option

This reverts commit 4bf2727.

* generalize the mv code for ln, cp replace links atomically instead of unlinking first Forced replacement was unlink(dst) then create. The gap lets another user claim dst in a directory they can write to -- including a name the sticky bit forbids them to remove -- so a privileged ln hands them a trusted name. * ln: don't leave the temp behind when the rename is a no-op Renaming the temp onto a destination that is already a link to the same inode does nothing and returns success, so the temp name survived: `ln -f a b` with b already hard-linked to a left a stray Cu* file. Unlink the temp after the rename either way; on the normal path it is already gone.

Copilot AI lite review requested due to automatic review settings September 16, 2026 21:06

Copilot AI reviewed Sep 16, 2026

sylvestre added 2 commits September 16, 2026 23:48

Copilot AI review requested due to automatic review settings September 16, 2026 21:51

sylvestre force-pushed the fix-3834-atomic-link-replace branch from f70c1c4 to 8fcb17a Compare September 16, 2026 21:51

Copilot AI reviewed Sep 16, 2026

sylvestre merged commit 4bf2727 into uutils:main Sep 16, 2026
112 of 113 checks passed

sylvestre deleted the fix-3834-atomic-link-replace branch September 16, 2026 22:12

hanthor mentioned this pull request Sep 17, 2026

fix(ci): three independent root causes — UID 1000 never freed on Arch, an ISO label two chars too long, and uutils 0.12.0 breaking ln -sf tuna-os/tunaOS#2558

Merged

xtqqczze mentioned this pull request Sep 19, 2026

uucore: fs feature fails to compile on redox #14704

Open

xtqqczze added a commit to xtqqczze/uutils-coreutils that referenced this pull request Sep 20, 2026

Revert "generalize some mv code for ln, cp (uutils#14623)"

41abf5a

This reverts commit 4bf2727.

varranvar mentioned this pull request Sep 21, 2026

ln: ln -s -f fails with ENOTDIR when the destination's parent path is a symlink #14795

Open

costajohnt mentioned this pull request Sep 24, 2026

ln: follow a symlinked parent when replacing a link #14851

Open

Navigation Menu

Uh oh!

generalize some mv code for ln, cp - #14623

generalize some mv code for ln, cp#14623
sylvestre merged 2 commits into
uutils:mainfrom
sylvestre:fix-3834-atomic-link-replace

sylvestre commented Sep 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 16, 2026

Uh oh!

codspeed Bot commented Sep 16, 2026

Uh oh!

xtqqczze commented Sep 17, 2026

Uh oh!

xtqqczze commented Sep 18, 2026

Uh oh!

sylvestre commented Sep 18, 2026

Uh oh!

oech3 commented Sep 19, 2026

Uh oh!

xtqqczze commented Sep 19, 2026

Uh oh!

sylvestre commented Sep 19, 2026

Uh oh!

xtqqczze commented Sep 19, 2026 •

edited

Loading

Uh oh!

sylvestre commented Sep 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

sylvestre commented Sep 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 16, 2026

Uh oh!

codspeed Bot commented Sep 16, 2026

Merging this PR will improve performance by 3.24%

Performance Changes

Footnotes

Uh oh!

xtqqczze commented Sep 17, 2026

Uh oh!

xtqqczze commented Sep 18, 2026

Uh oh!

sylvestre commented Sep 18, 2026

Uh oh!

oech3 commented Sep 19, 2026

Uh oh!

xtqqczze commented Sep 19, 2026

Uh oh!

sylvestre commented Sep 19, 2026

Uh oh!

xtqqczze commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sylvestre commented Sep 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xtqqczze commented Sep 19, 2026 •

edited

Loading