fix: `copy_dir` and `copy_file` to preserve symlinks instead of following them. #4671

cachebag · 2026-01-06T21:44:36Z

This PR employs symlink preservation in copy_file and adds it to copy_dir.

PR #1504 added symlink preservation to fix crashes in lldb-preview, which contains internal symlinks (liblldb was loading twice due to broken symlinks). Then, PR #1521 later changed copy_file to create symlinks pointing to the source path (for Homebrew integration), which may have regressed #1504.

This PR restores #1504's behavior: read the symlink target and preserve it. What I'm curious about is what the intended behavior for #1521 was because shouldn't users be able to run rustup-init --no-self-update? Is that not the recommended way to manage rustup via external package managers?

If this PR steers out of scope of the goals of rustup I'm happy to close it but I feel like the former behavior is preferable for most use cases.

rami3l · 2026-01-14T07:01:05Z

Thanks for making this patch! Reasonable support for external packagers are still expected, and whether a manual setup is required is largely based on the packagers' opinion: https://rust-lang.github.io/rustup/installation/other.html#using-a-package-manager

I have the feeling that we need to somehow support both cases based on whether that's copying the original binary or not...

rami3l

Do you have a specific use case where rustup is not working as you'd expect, which this PR allows to fix?

If you can find such a case, I agree with using this new implementation except for this:

rustup/src/cli/self_update.rs

Line 845 in 3382f6f

utils::copy_file(&this_exe_path, &rustup_path)?;

#1521 is probably too widespread as a fix, and it makes perfect sense to use different copy implementations during self installation and during toolchain installation.

Suggest refining the test cases accordingly if going this way.

rami3l · 2026-01-14T20:00:54Z

src/utils/raw.rs

-        let dest = dest.join(entry.file_name());
-        if kind.is_dir() {
-            copy_dir(&src, &dest)?;
+        let src_path = entry.path();


Nit: src and dest bindings in the loop body are preventing the input params from being used. Thus, I don't see a very convincing reason to rename them here.

cachebag · 2026-01-15T13:22:34Z

Do you have a specific use case where rustup is not working as you'd expect, which this PR allows to fix?

I don't, and I only encountered this looking at the code. It's my understanding that copying symlinks breaks their identity and can cause software to load libraries twice (like the crash that was fixed ad-hoc above). It's also just correct behavior afaik; cp -R preserves symlinks by default.

#1521 is probably too widespread as a fix, and it makes perfect sense to use different copy implementations during self installation and during toolchain installation.

I would agree with this and will adjust, so long as you think it's a reasonable patch. I figured at the least I could flag this to note the variance in functionality for copy's.

rami3l · 2026-01-15T20:49:11Z

@cachebag Thanks for your clarification. I think although it has no direct influence now, judging by what I'd expect from the function API then indeed, the old behavior feels more correct to me, and leaving it uncorrected might lead to misuse in the long run.

Looking back there is still a slight possibility that we could be missing other cases apart from self installation, so I'd say let's go on polishing this patch by carefully analyzing the call sites. Please don't hesitate to ask me if you don't feel certain about anything!

cachebag · 2026-01-15T21:04:37Z

@cachebag Thanks for your clarification. I think although it has no direct influence now, judging by what I'd expect from the function API then indeed, the old behavior feels more correct to me, and leaving it uncorrected might lead to misuse in the long run.

Looking back there is still a slight possibility that we could be missing other cases apart from self installation, so I'd say let's go on polishing this patch by carefully analyzing the call sites. Please don't hesitate to ask me if you don't feel certain about anything!

That makes sense to me, thank you!

rami3l · 2026-01-16T22:31:58Z

src/utils/mod.rs

+
+/// Copy a file, preserving symlink targets instead of creating a link to the source.
+/// We use this for component installation where internal symlink structure must be maintained.
+pub(crate) fn copy_file_preserving_symlink(src: &Path, dest: &Path) -> Result<()> {


If I'm not mistaken, the new function you have introduced is identical to the old function you have modified, which means you are not distinguishing the two approaches.

To align with the title of your PR, the symlink-preserving copy_file should just be named copy_file(). Also, it might not be worth it to duplicate code here since the fact that the function is duplicated without any changes is not very obvious in my review already; try to DRY it up a bit, for example by making copy_file() and copy_file_symlink_to_source() shorthands of copy_file_impl(preserve_symlink: bool) calls.

Also, the tests are not enough to prove the fact that the both functions are working as intended.

You might need to add more tests to demonstrate this point.

Hopefully I followed that train of thought correctly. I should have led with the unified path instead, sorry about that.

As far as tests, do you have suggestions on how to approach that, or specific cases I should be covering?

Previously, copy_dir would follow symlinks and copy the target contents. Now it preserves symlinks by reading the link target and creating a new symlink with the same target.

- copy_file() preserves symlinks (for component installation) - copy_file_symlink_to_source() follows symlinks (for self-installation) Uses copy_file in Transaction::copy_file and modify_file. Uses copy_file_symlink_to_source in self_update.rs and windows.rs. Adds regression tests for symlink preservation.

cachebag force-pushed the preserve-symlinks branch from 7ce58b5 to 5460b37 Compare January 6, 2026 22:04

rami3l self-assigned this Jan 8, 2026

rami3l reviewed Jan 14, 2026

View reviewed changes

cachebag force-pushed the preserve-symlinks branch 2 times, most recently from 09c35e1 to f4c5445 Compare January 16, 2026 02:24

cachebag requested a review from rami3l January 16, 2026 03:20

rami3l requested changes Jan 16, 2026

View reviewed changes

cachebag force-pushed the preserve-symlinks branch from f4c5445 to 071f34f Compare January 16, 2026 23:40

cachebag added 2 commits January 16, 2026 18:44

fix: preserve symlinks in copy_dir instead of following them

dd4aeb7

Previously, copy_dir would follow symlinks and copy the target contents. Now it preserves symlinks by reading the link target and creating a new symlink with the same target.

cachebag force-pushed the preserve-symlinks branch from 071f34f to 19eada1 Compare January 16, 2026 23:44

cachebag requested a review from rami3l January 16, 2026 23:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: `copy_dir` and `copy_file` to preserve symlinks instead of following them. #4671

fix: `copy_dir` and `copy_file` to preserve symlinks instead of following them. #4671

cachebag commented Jan 6, 2026 •

edited

Loading

Uh oh!

rami3l commented Jan 14, 2026 •

edited

Loading

Uh oh!

rami3l left a comment •

edited

Loading

Uh oh!

rami3l Jan 14, 2026

Uh oh!

cachebag commented Jan 15, 2026

Uh oh!

rami3l commented Jan 15, 2026

Uh oh!

cachebag commented Jan 15, 2026

Uh oh!

rami3l Jan 16, 2026 •

edited

Loading

Uh oh!

cachebag Jan 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: copy_dir and copy_file to preserve symlinks instead of following them. #4671

Are you sure you want to change the base?

fix: copy_dir and copy_file to preserve symlinks instead of following them. #4671

Conversation

cachebag commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rami3l commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rami3l left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rami3l Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

cachebag commented Jan 15, 2026

Uh oh!

rami3l commented Jan 15, 2026

Uh oh!

cachebag commented Jan 15, 2026

Uh oh!

rami3l Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cachebag Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: `copy_dir` and `copy_file` to preserve symlinks instead of following them. #4671

fix: `copy_dir` and `copy_file` to preserve symlinks instead of following them. #4671

cachebag commented Jan 6, 2026 •

edited

Loading

rami3l commented Jan 14, 2026 •

edited

Loading

rami3l left a comment •

edited

Loading

rami3l Jan 16, 2026 •

edited

Loading