Skip to content

uucore: share replace_link's atomic replace - #15122

Open
abendrothj wants to merge 1 commit into
uutils:mainfrom
abendrothj:uucore-replace-entry-at
Open

uucore: share replace_link's atomic replace#15122
abendrothj wants to merge 1 commit into
uutils:mainfrom
abendrothj:uucore-replace-entry-at

Conversation

@abendrothj

@abendrothj abendrothj commented Oct 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

replace_link creates a temporary link and renames it over the target. The temporary-name and renameat part moves into DirFd::replace_entry_at, which creates the entry through a callback relative to the directory, so mv can replace special files and directory entries the same way.

The parent is now opened with DirFd::open_anchor, so ln -sf and cp -sf replacing an entry in a directory with write and search but no read permission work, as with GNU 9.12, instead of failing with EACCES. Running out of temporary names reports a translated error.

Closes #15112

Copilot AI balanced review requested due to automatic review settings October 6, 2026 07:27
Copilot AI reviewed Oct 6, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

This was referenced Oct 6, 2026
Draft
Closed
Copilot AI balanced review requested due to automatic review settings October 6, 2026 07:58
Copilot AI reviewed Oct 6, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@codspeed

codspeed Bot commented Oct 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will improve performance by 6.02%

1 warning, 1 note

⚠️ 118 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

ℹ️ 1 benchmark has no flamegraph

The flamegraph wasn't generated: most of the time was spent in runtime internals, which would leave nothing useful to analyze.

See affected benchmarks

⚡ 1 improved benchmark
✅ 367 untouched benchmarks
⏩ 103 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ CPU Simulation hostname_ip_lookup[100000] 175.8 µs 165.9 µs +6.02%

Tip

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

Comparing abendrothj:uucore-replace-entry-at (4d02b5c) with main (366c18c)2

Open in CodSpeed

Footnotes

  1. 103 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. ↩

  2. No successful run was found on main (fecc31c) during the generation of this report, so 366c18c was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@abendrothj
abendrothj force-pushed the uucore-replace-entry-at branch from c7045d6 to b872c65 Compare October 7, 2026 00:54
Copilot AI balanced review requested due to automatic review settings October 7, 2026 00:54
Copilot AI reviewed Oct 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@abendrothj abendrothj mentioned this pull request Oct 7, 2026
Draft
@abendrothj
abendrothj force-pushed the uucore-replace-entry-at branch from b872c65 to 5d10e9b Compare October 7, 2026 08:21
Copilot AI balanced review requested due to automatic review settings October 7, 2026 08:21
Copilot AI reviewed Oct 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@abendrothj
abendrothj force-pushed the uucore-replace-entry-at branch from 5d10e9b to f5b87ae Compare October 12, 2026 10:05
Copilot AI balanced review requested due to automatic review settings October 12, 2026 10:05
Copilot AI reviewed Oct 12, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@abendrothj abendrothj mentioned this pull request Oct 19, 2026
Open
@xtqqczze

xtqqczze commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

@abendrothj please rebase for a clean history

sylvestre
sylvestre reviewed Oct 19, 2026
View reviewed changes
Comment thread src/uucore/Cargo.toml
sylvestre
sylvestre reviewed Oct 19, 2026
View reviewed changes
Comment thread src/uucore/src/lib/features/fs.rs Outdated
sylvestre
sylvestre reviewed Oct 19, 2026
View reviewed changes
Comment thread tests/by-util/test_ln.rs
@abendrothj
abendrothj force-pushed the uucore-replace-entry-at branch from f5b87ae to 8c7924c Compare October 20, 2026 00:27
Copilot AI balanced review requested due to automatic review settings October 20, 2026 00:27
Copilot AI reviewed Oct 20, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

@abendrothj

abendrothj commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main and squashed into one commit.

Copilot AI balanced review requested due to automatic review settings October 22, 2026 06:44
@abendrothj
abendrothj force-pushed the uucore-replace-entry-at branch from 8c7924c to 94af8e7 Compare October 22, 2026 06:44
Copilot AI reviewed Oct 22, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

@abendrothj @sylvestre
uucore: share replace_link's atomic replace
4d02b5c 
Move the temporary name and renameat logic of replace_link into
DirFd::replace_entry_at, which creates the entry through a callback
relative to the directory, so other callers can replace entries the
same way.

The parent is now opened with DirFd::open_anchor, so ln -sf and cp -sf
replacing an entry in a directory with write and search but no read
permission work, as they do with GNU, instead of failing with EACCES.
@sylvestre
sylvestre force-pushed the uucore-replace-entry-at branch from 94af8e7 to 4d02b5c Compare October 25, 2026 08:29
Copilot AI balanced review requested due to automatic review settings October 25, 2026 08:29
Copilot AI reviewed Oct 25, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sylvestre sylvestre sylvestre left review comments
Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
+1 more reviewer
@xtqqczze xtqqczze xtqqczze left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ln, cp: replacing an entry in a write-only directory fails with EACCES

4 participants

@abendrothj @xtqqczze @sylvestre