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

Add try_canonicalize and use it over std::fs::canonicalize #11866

Merged
merged 3 commits into from
Apr 8, 2023
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
5 changes: 2 additions & 3 deletions src/cargo/core/compiler/compile_kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions src/cargo/core/compiler/fingerprint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions src/cargo/ops/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -83,7 +83,7 @@ fn sync(
workspaces: &[&Workspace<'_>],
opts: &VendorOptions<'_>,
) -> CargoResult<VendorConfig> {
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();

Expand Down Expand Up @@ -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);
}
}
Expand Down
14 changes: 7 additions & 7 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -433,11 +433,11 @@ impl Config {
// commands that use Cargo as a library to inherit (via `cargo <subcommand>`)
// 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)
};

Expand All @@ -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)
}

Expand Down
74 changes: 74 additions & 0 deletions src/cargo/util/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::fmt;
use std::path::{Path, PathBuf};
use std::time::Duration;

pub use self::canonical_url::CanonicalUrl;
Expand Down Expand Up @@ -133,6 +134,79 @@ pub fn truncate_with_ellipsis(s: &str, max_width: usize) -> String {
prefix
}

#[cfg(not(windows))]
#[inline]
pub fn try_canonicalize<P: AsRef<Path>>(path: P) -> std::io::Result<PathBuf> {
std::fs::canonicalize(&path)
}

#[cfg(windows)]
#[inline]
pub fn try_canonicalize<P: AsRef<Path>>(path: P) -> std::io::Result<PathBuf> {
use std::ffi::OsString;
use std::io::Error;
use std::os::windows::ffi::{OsStrExt, OsStringExt};
use std::{io::ErrorKind, ptr};
use windows_sys::Win32::Foundation::{GetLastError, SetLastError};
use windows_sys::Win32::Storage::FileSystem::GetFullPathNameW;

// On Windows `canonicalize` may fail, so we fall back to getting an absolute path.
std::fs::canonicalize(&path).or_else(|_| {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we wait for the stabilization of std::path::aboslute? We should also have a TODO here and call out where this piece of code come from. (Looks like fn absolute on windows to me.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would probably take a while.

// Return an error if a file does not exist for better compatiblity with `canonicalize`
if !path.as_ref().try_exists()? {
return Err(Error::new(ErrorKind::NotFound, "the path was not found"));
}

// This code is based on the unstable `std::path::absolute` 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());
if path_u16.iter().find(|c| **c == 0).is_some() {
return Err(Error::new(
ErrorKind::InvalidInput,
"strings passed to WinAPI cannot contain NULs",
));
}
path_u16.push(0);

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![0u16; len as usize];

let write_len = GetFullPathNameW(
path_u16.as_ptr(),
result.len().try_into().unwrap(),
result.as_mut_ptr().cast::<u16>(),
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)],
)));
}
}
}
})
}

#[cfg(test)]
mod test {
use super::*;
Expand Down