Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try symlinking proxies first, falling back to hardlinking if that fails #4023

Merged
merged 2 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/cli/self_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -731,8 +731,8 @@ pub(crate) fn install_proxies(process: &Process) -> Result<()> {
let mut tool_handles = Vec::new();
let mut link_afterwards = Vec::new();

// Try to hardlink all the Rust exes to the rustup exe. Some systems,
// like Android, does not support hardlinks, so we fallback to symlinks.
// Try to symlink all the Rust exes to the rustup exe. Some systems,
// like Windows, do not always support symlinks, so we fallback to hard links.
//
// Note that this function may not be running in the context of a fresh
// self update but rather as part of a normal update to fill in missing
Expand All @@ -751,7 +751,7 @@ pub(crate) fn install_proxies(process: &Process) -> Result<()> {
// actually delete files (they'll say they're deleted but they won't
// actually be on Windows). As a result we manually drop all the
// `tool_handles` later on. This'll allow us, afterwards, to actually
// overwrite all the previous hard links with new ones.
// overwrite all the previous soft or hard links with new ones.
for tool in TOOLS {
let tool_path = bin_path.join(format!("{tool}{EXE_SUFFIX}"));
if let Ok(handle) = Handle::from_path(&tool_path) {
Expand All @@ -766,7 +766,7 @@ pub(crate) fn install_proxies(process: &Process) -> Result<()> {
for tool in DUP_TOOLS {
let tool_path = bin_path.join(format!("{tool}{EXE_SUFFIX}"));
if let Ok(handle) = Handle::from_path(&tool_path) {
// Like above, don't clobber anything that's already hardlinked to
// Like above, don't clobber anything that's already linked to
// avoid extraneous errors from being returned.
if rustup == handle {
continue;
Expand All @@ -790,12 +790,12 @@ pub(crate) fn install_proxies(process: &Process) -> Result<()> {
continue;
}
}
utils::hard_or_symlink_file(&rustup_path, &tool_path)?;
utils::symlink_or_hardlink_file(&rustup_path, &tool_path)?;
}

drop(tool_handles);
for path in link_afterwards {
utils::hard_or_symlink_file(&rustup_path, &path)?;
utils::symlink_or_hardlink_file(&rustup_path, &path)?;
}

Ok(())
Expand Down
5 changes: 0 additions & 5 deletions src/utils/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,11 +226,6 @@ fn symlink_junction_inner(target: &Path, junction: &Path) -> io::Result<()> {
}
}

pub(crate) fn hardlink(src: &Path, dest: &Path) -> io::Result<()> {
let _ = fs::remove_file(dest);
fs::hard_link(src, dest)
}

pub fn remove_dir(path: &Path) -> io::Result<()> {
if fs::symlink_metadata(path)?.file_type().is_symlink() {
#[cfg(windows)]
Expand Down
31 changes: 19 additions & 12 deletions src/utils/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,23 +312,31 @@ where
})
}

pub(crate) fn hard_or_symlink_file(src: &Path, dest: &Path) -> Result<()> {
/// Attempts to symlink a file, falling back to hard linking if that fails.
///
/// If `dest` already exists then it will be replaced.
pub(crate) fn symlink_or_hardlink_file(src: &Path, dest: &Path) -> Result<()> {
let _ = fs::remove_file(dest);
// The error is only used by macos
let Err(_err) = symlink_file(src, dest) else {
return Ok(());
};

// Some mac filesystems can do hardlinks to symlinks, some can't.
// See rust-lang/rustup#3136 for why it's better never to use them.
#[cfg(target_os = "macos")]
let force_symlink = fs::symlink_metadata(src)
if fs::symlink_metadata(src)
.map(|m| m.file_type().is_symlink())
.unwrap_or(false);
#[cfg(not(target_os = "macos"))]
let force_symlink = false;
if force_symlink || hardlink_file(src, dest).is_err() {
symlink_file(src, dest)?;
.unwrap_or(false)
{
return Err(_err);
}
Ok(())

hardlink_file(src, dest)
}

pub fn hardlink_file(src: &Path, dest: &Path) -> Result<()> {
raw::hardlink(src, dest).with_context(|| RustupError::LinkingFile {
fs::hard_link(src, dest).with_context(|| RustupError::LinkingFile {
src: PathBuf::from(src),
dest: PathBuf::from(dest),
})
Expand All @@ -344,11 +352,10 @@ fn symlink_file(src: &Path, dest: &Path) -> Result<()> {

#[cfg(windows)]
fn symlink_file(src: &Path, dest: &Path) -> Result<()> {
// we are supposed to not use symlink on windows
Err(anyhow!(RustupError::LinkingFile {
std::os::windows::fs::symlink_file(src, dest).with_context(|| RustupError::LinkingFile {
src: PathBuf::from(src),
dest: PathBuf::from(dest),
}))
})
}

pub(crate) fn copy_dir<'a, N>(
Expand Down
Loading