Skip to content

uucore: share replace_link's atomic replace - #15122

Open
abendrothj wants to merge 2 commits into
uutils:mainfrom
abendrothj:uucore-replace-entry-at
Open

abendrothj wants to merge 2 commits into
uutils:mainfrom
abendrothj:uucore-replace-entry-at

Conversation

@abendrothj

@abendrothj abendrothj commented Oct 6, 2026 •

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 replace_entry_at, which creates the entry through a callback relative to an open 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 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.

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 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 2.92%

⚠️ 79 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

⚠️ 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

⚡ 3 improved benchmarks
❌ 2 regressed benchmarks
✅ 144 untouched benchmarks
⏩ 306 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ three_39_bit_primes 583.2 ms 719.2 ms -18.9%
❌ ⚠️ hostname_ip_lookup[100000] 169.5 µs 177.3 µs -4.38%
⚡ five_38_bit_primes 1.6 s 1.5 s +4.42%
⚡ ⚠️ cksum_crc 33.6 ms 32.6 ms +3.25%
⚡ ⚠️ cksum_default 33.9 ms 32.8 ms +3.13%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing abendrothj:uucore-replace-entry-at (f5b87ae) with main (6292c03)

Open in CodSpeed

Footnotes

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

@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 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 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 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.

Move the temporary name and renameat logic of replace_link into
replace_entry_at, which creates the entry through a callback relative to
an open directory, so other callers can replace entries the same way.

The parent is now opened with DirFd::open_anchor, so ln -sf replacing a
link in a directory with write and search but no read permission works,
as it does with GNU, instead of failing with EACCES.
@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 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 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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants