From e83338b046c9fad6ea748979c6cc61d649beff3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Fri, 17 Mar 2023 19:18:47 +0100 Subject: [PATCH] Use `try_canonicalize` over `canonicalize` --- src/cargo/core/compiler/compile_kind.rs | 5 +- src/cargo/core/compiler/fingerprint/mod.rs | 8 +-- src/cargo/ops/vendor.rs | 6 +-- src/cargo/util/config/mod.rs | 14 ++--- src/cargo/util/mod.rs | 62 +++++++++++----------- 5 files changed, 48 insertions(+), 47 deletions(-) diff --git a/src/cargo/core/compiler/compile_kind.rs b/src/cargo/core/compiler/compile_kind.rs index 45a464684a6..73d8f89cc44 100644 --- a/src/cargo/core/compiler/compile_kind.rs +++ b/src/cargo/core/compiler/compile_kind.rs @@ -3,7 +3,7 @@ use crate::core::Target; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; -use crate::util::{Config, StableHasher}; +use crate::util::{try_canonicalize, Config, StableHasher}; use anyhow::Context as _; use serde::Serialize; use std::collections::BTreeSet; @@ -138,8 +138,7 @@ impl CompileTarget { // If `name` ends in `.json` then it's likely a custom target // specification. Canonicalize the path to ensure that different builds // with different paths always produce the same result. - let path = Path::new(name) - .canonicalize() + let path = try_canonicalize(Path::new(name)) .with_context(|| format!("target path {:?} is not a valid file", name))?; let name = path diff --git a/src/cargo/core/compiler/fingerprint/mod.rs b/src/cargo/core/compiler/fingerprint/mod.rs index e47ae07f844..366de026bb9 100644 --- a/src/cargo/core/compiler/fingerprint/mod.rs +++ b/src/cargo/core/compiler/fingerprint/mod.rs @@ -367,9 +367,9 @@ use serde::{Deserialize, Serialize}; use crate::core::compiler::unit_graph::UnitDep; use crate::core::Package; -use crate::util; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; +use crate::util::{self, try_canonicalize}; use crate::util::{internal, path_args, profile, StableHasher}; use crate::{Config, CARGO_ENV}; @@ -1941,8 +1941,8 @@ pub fn translate_dep_info( ) -> CargoResult<()> { let depinfo = parse_rustc_dep_info(rustc_dep_info)?; - let target_root = target_root.canonicalize()?; - let pkg_root = pkg_root.canonicalize()?; + let target_root = try_canonicalize(target_root)?; + let pkg_root = try_canonicalize(pkg_root)?; let mut on_disk_info = EncodedDepInfo::default(); on_disk_info.env = depinfo.env; @@ -1985,7 +1985,7 @@ pub fn translate_dep_info( // If canonicalization fails, just use the abs path. There is currently // a bug where --remap-path-prefix is affecting .d files, causing them // to point to non-existent paths. - let canon_file = abs_file.canonicalize().unwrap_or_else(|_| abs_file.clone()); + let canon_file = try_canonicalize(&abs_file).unwrap_or_else(|_| abs_file.clone()); let (ty, path) = if let Ok(stripped) = canon_file.strip_prefix(&target_root) { (DepInfoPathType::TargetRootRelative, stripped) diff --git a/src/cargo/ops/vendor.rs b/src/cargo/ops/vendor.rs index 8c507d87b91..3ee46db3284 100644 --- a/src/cargo/ops/vendor.rs +++ b/src/cargo/ops/vendor.rs @@ -4,7 +4,7 @@ use crate::core::{GitReference, Package, Workspace}; use crate::ops; use crate::sources::path::PathSource; use crate::sources::CRATES_IO_REGISTRY; -use crate::util::{CargoResult, Config}; +use crate::util::{try_canonicalize, CargoResult, Config}; use anyhow::{bail, Context as _}; use cargo_util::{paths, Sha256}; use serde::Serialize; @@ -83,7 +83,7 @@ fn sync( workspaces: &[&Workspace<'_>], opts: &VendorOptions<'_>, ) -> CargoResult { - let canonical_destination = opts.destination.canonicalize(); + let canonical_destination = try_canonicalize(opts.destination); let canonical_destination = canonical_destination.as_deref().unwrap_or(opts.destination); let dest_dir_already_exists = canonical_destination.exists(); @@ -125,7 +125,7 @@ fn sync( // Don't delete actual source code! if pkg.source_id().is_path() { if let Ok(path) = pkg.source_id().url().to_file_path() { - if let Ok(path) = path.canonicalize() { + if let Ok(path) = try_canonicalize(path) { to_remove.remove(&path); } } diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index 22915918270..dad7e9c727b 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -72,9 +72,9 @@ use crate::core::{features, CliUnstable, Shell, SourceId, Workspace, WorkspaceRo use crate::ops::{self, RegistryCredentialConfig}; use crate::util::auth::Secret; use crate::util::errors::CargoResult; -use crate::util::validate_package_name; use crate::util::CanonicalUrl; use crate::util::{internal, toml as cargo_toml}; +use crate::util::{try_canonicalize, validate_package_name}; use crate::util::{FileLock, Filesystem, IntoUrl, IntoUrlWithBase, Rustc}; use anyhow::{anyhow, bail, format_err, Context as _}; use cargo_util::paths; @@ -433,11 +433,11 @@ impl Config { // commands that use Cargo as a library to inherit (via `cargo `) // or set (by setting `$CARGO`) a correct path to `cargo` when the current exe // is not actually cargo (e.g., `cargo-*` binaries, Valgrind, `ld.so`, etc.). - let exe = self - .get_env_os(crate::CARGO_ENV) - .map(PathBuf::from) - .ok_or_else(|| anyhow!("$CARGO not set"))? - .canonicalize()?; + let exe = try_canonicalize( + self.get_env_os(crate::CARGO_ENV) + .map(PathBuf::from) + .ok_or_else(|| anyhow!("$CARGO not set"))?, + )?; Ok(exe) }; @@ -446,7 +446,7 @@ impl Config { // The method varies per operating system and might fail; in particular, // it depends on `/proc` being mounted on Linux, and some environments // (like containers or chroots) may not have that available. - let exe = env::current_exe()?.canonicalize()?; + let exe = try_canonicalize(env::current_exe()?)?; Ok(exe) } diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index 56bfd38da74..39edef4587b 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -157,6 +157,9 @@ pub fn try_canonicalize>(path: P) -> std::io::Result { return Err(Error::new(ErrorKind::NotFound, "the path was not found")); } + // This code is based on the unstable `std::path::aboslute` and could be replaced with it + // if it's stabilized. + let path = path.as_ref().as_os_str(); let mut path_u16 = Vec::with_capacity(path.len() + 1); path_u16.extend(path.encode_wide()); @@ -168,39 +171,38 @@ pub fn try_canonicalize>(path: P) -> std::io::Result { } path_u16.push(0); - unsafe { - SetLastError(0); - let len = GetFullPathNameW(path_u16.as_ptr(), 0, &mut [] as *mut u16, ptr::null_mut()); - if len == 0 { - let error = GetLastError(); - if error != 0 { - return Err(Error::from_raw_os_error(error as i32)); + loop { + unsafe { + SetLastError(0); + let len = + GetFullPathNameW(path_u16.as_ptr(), 0, &mut [] as *mut u16, ptr::null_mut()); + if len == 0 { + let error = GetLastError(); + if error != 0 { + return Err(Error::from_raw_os_error(error as i32)); + } } - } - let mut result: Vec = std::iter::repeat(0).take(len as usize).collect(); - - let write_len = GetFullPathNameW( - path_u16.as_ptr(), - result.len().try_into().unwrap(), - result.as_mut_ptr().cast::(), - ptr::null_mut(), - ); - if write_len == 0 { - let error = GetLastError(); - if error != 0 { - return Err(Error::from_raw_os_error(error as i32)); + let mut result = vec![0u16; len as usize]; + + let write_len = GetFullPathNameW( + path_u16.as_ptr(), + result.len().try_into().unwrap(), + result.as_mut_ptr().cast::(), + ptr::null_mut(), + ); + if write_len == 0 { + let error = GetLastError(); + if error != 0 { + return Err(Error::from_raw_os_error(error as i32)); + } + } + + if write_len <= len { + return Ok(PathBuf::from(OsString::from_wide( + &result[0..(write_len as usize)], + ))); } } - assert_eq!( - write_len + 1, - len, - "mismatching requested and written lengths for path {:?}", - path - ); - - Ok(PathBuf::from(OsString::from_wide( - &result[0..(write_len as usize)], - ))) } }) }