Skip to content

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

Open
costajohnt wants to merge 3 commits into
uutils:mainfrom
costajohnt:ln-replace-link-symlink-parent
Open

costajohnt wants to merge 3 commits into
uutils:mainfrom
costajohnt:ln-replace-link-symlink-parent

Conversation

@costajohnt

Copy link
Copy Markdown
Contributor

replace_link in uucore falls back to opening the destination's parent with O_NOFOLLOW on EEXIST. When the parent's last component is a symlink to a directory (ln -sf target dst/ucc/modules with dst/ucc -> somedir), that open fails with ENOTDIR, a regression from #14623.

This drops NOFOLLOW on the parent open (earlier path components were already followed, so it only broke this case) and adds test_force_replace_in_symlinked_directory.

Closes #14795

@codspeed

codspeed Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 16.32%

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 387 untouched benchmarks
⏩ 54 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Simulation three_39_bit_primes 467.5 ms 770.9 ms -39.36%
⚡ Simulation five_38_bit_primes 1.8 s 1.6 s +15.48%

Tip

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


Comparing costajohnt:ln-replace-link-symlink-parent (fb256eb) with main (a6d1eb3)

Open in CodSpeed

Footnotes

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

Comment thread tests/by-util/test_ln.rs
fn test_force_replace_in_symlinked_directory() {
let (at, mut ucmd) = at_and_ucmd!();
at.mkdir("real");
at.symlink_dir("real", "dirlink");

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.

could you please also cover the hard link case (-f without -s)? it goes through the same path

@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skipping an intermittent issue tests/date/resolution (passes in this run but fails in the 'main' branch)

@costajohnt

Copy link
Copy Markdown
Contributor Author

Added the hard link case in fb256eb (test_force_replace_hard_link_in_symlinked_directory): ln -f new dirlink/link where dirlink points at real, then checks real/link and new share an inode.

The CodSpeed failure is on factor::three_39_bit_primes, which doesn't go through replace_link, and five_38_bit_primes moved the other way in the same run, so it looks like benchmark noise rather than something from this change.

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: ln -s -f fails with ENOTDIR when the destination's parent path is a symlink

2 participants