Skip to content

Use absolute_path instead of fs::canonicalize #3590

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

Closed
wants to merge 8 commits into from
Closed
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ structopt = "0.3"

rustfmt-config_proc_macro = { version = "0.2", path = "config_proc_macro" }

[target.'cfg(windows)'.dependencies]
winapi = { version = "0.3", features = ["errhandlingapi", "fileapi"] }

# A noop dependency that changes in the Rust repository, it's a bit of a hack.
# See the `src/tools/rustc-workspace-hack/README.md` file in `rust-lang/rust`
# for more information.
Expand Down
8 changes: 4 additions & 4 deletions src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use std::str::FromStr;
use getopts::{Matches, Options};

use crate::rustfmt::{
load_config, CliOptions, Color, Config, Edition, EmitMode, FileLines, FileName,
absolute_path, load_config, CliOptions, Color, Config, Edition, EmitMode, FileLines, FileName,
FormatReportFormatterBuilder, Input, Session, Verbosity,
};

Expand Down Expand Up @@ -453,9 +453,9 @@ fn determine_operation(matches: &Matches) -> Result<Operation, OperationError> {
let files: Vec<_> = free_matches
.map(|s| {
let p = PathBuf::from(s);
// we will do comparison later, so here tries to canonicalize first
// to get the expected behavior.
p.canonicalize().unwrap_or(p)
// we will do comparison later, so here tries to get the absolute
// path first to get the expected behavior.
absolute_path(&p).unwrap_or(p)
})
.collect();

Expand Down
12 changes: 7 additions & 5 deletions src/cargo-fmt/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
#![deny(warnings)]

use cargo_metadata;
use rustfmt_nightly as rustfmt;

use std::cmp::Ordering;
use std::collections::{BTreeMap, BTreeSet};
use std::env;
use std::fs;
use std::hash::{Hash, Hasher};
use std::io::{self, Write};
use std::iter::FromIterator;
Expand Down Expand Up @@ -58,6 +58,8 @@ pub struct Opts {
format_all: bool,
}

use crate::rustfmt::absolute_path;

fn main() {
let exit_status = execute();
std::io::stdout().flush().unwrap();
Expand Down Expand Up @@ -250,10 +252,10 @@ pub struct Target {
impl Target {
pub fn from_target(target: &cargo_metadata::Target) -> Self {
let path = PathBuf::from(&target.src_path);
let canonicalized = fs::canonicalize(&path).unwrap_or(path);
let path = absolute_path(&path).unwrap_or(path);

Target {
path: canonicalized,
path,
kind: target.kind[0].clone(),
edition: target.edition.clone(),
}
Expand Down Expand Up @@ -338,14 +340,14 @@ fn get_targets_root_only(
targets: &mut BTreeSet<Target>,
) -> Result<(), io::Error> {
let metadata = get_cargo_metadata(manifest_path, false)?;
let workspace_root_path = PathBuf::from(&metadata.workspace_root).canonicalize()?;
let workspace_root_path = absolute_path(PathBuf::from(&metadata.workspace_root))?;
let (in_workspace_root, current_dir_manifest) = if let Some(target_manifest) = manifest_path {
(
workspace_root_path == target_manifest,
target_manifest.canonicalize()?,
)
} else {
let current_dir = env::current_dir()?.canonicalize()?;
let current_dir = absolute_path(env::current_dir()?)?;
(
workspace_root_path == current_dir,
current_dir.join("Cargo.toml"),
Expand Down
13 changes: 7 additions & 6 deletions src/config/file_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ use std::{cmp, fmt, iter, str};

use serde::{ser, Deserialize, Deserializer, Serialize, Serializer};
use serde_json as json;

use syntax::source_map::{self, SourceFile};

use crate::utils::absolute_path;

/// A range of lines in a file, inclusive of both ends.
pub struct LineRange {
pub file: Rc<SourceFile>,
Expand Down Expand Up @@ -242,7 +243,7 @@ impl FileLines {
Some(ref map) => map,
};

match canonicalize_path_string(file_name).and_then(|file| map.get(&file)) {
match absolute_path_string(file_name).and_then(|file| map.get(&file)) {
Some(ranges) => ranges.iter().any(f),
None => false,
}
Expand Down Expand Up @@ -281,9 +282,9 @@ impl<'a> iter::Iterator for Files<'a> {
}
}

fn canonicalize_path_string(file: &FileName) -> Option<FileName> {
fn absolute_path_string(file: &FileName) -> Option<FileName> {
match *file {
FileName::Real(ref path) => path.canonicalize().ok().map(FileName::Real),
FileName::Real(ref path) => absolute_path(path).ok().map(FileName::Real),
_ => Some(file.clone()),
}
}
Expand Down Expand Up @@ -313,8 +314,8 @@ pub struct JsonSpan {
impl JsonSpan {
fn into_tuple(self) -> Result<(FileName, Range), String> {
let (lo, hi) = self.range;
let canonical = canonicalize_path_string(&self.file)
.ok_or_else(|| format!("Can't canonicalize {}", &self.file))?;
let canonical = absolute_path_string(&self.file)
.ok_or_else(|| format!("Can't get absolute path {}", &self.file))?;
Ok((canonical, Range::new(lo, hi)))
}
}
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub use crate::config::{
load_config, CliOptions, Color, Config, Edition, EmitMode, FileLines, FileName, NewlineStyle,
Range, Verbosity,
};
pub use crate::utils::absolute_path;

pub use crate::format_report_formatter::{FormatReportFormatter, FormatReportFormatterBuilder};

Expand Down
43 changes: 43 additions & 0 deletions src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use std::borrow::Cow;
use std::io;
use std::path;

use rustc_target::spec::abi;
use syntax::ast::{
Expand Down Expand Up @@ -653,6 +655,47 @@ pub(crate) fn unicode_str_width(s: &str) -> usize {
s.width()
}

#[cfg(windows)]
pub fn absolute_path<P: AsRef<path::Path>>(p: P) -> io::Result<path::PathBuf> {
use std::ffi::OsString;
use std::iter::once;
use std::os::windows::ffi::{OsStrExt, OsStringExt};
use std::ptr::null_mut;
use winapi::um::errhandlingapi::GetLastError;
use winapi::um::fileapi::GetFullPathNameW;

// FIXME: This `MAX_PATH` may be valid only from Windows 10, version 1607.
// https://docs.microsoft.com/ja-jp/windows/desktop/FileIO/naming-a-file#paths
const MAX_PATH: usize = 32767;
let wide: Vec<u16> = p
.as_ref()
.as_os_str()
.encode_wide()
.chain(once(0))
.collect();
let mut buffer: Vec<u16> = vec![0; MAX_PATH];
unsafe {
let result = GetFullPathNameW(
wide.as_ptr(),
MAX_PATH as u32,
buffer.as_mut_ptr(),
null_mut(),
);
if result == 0 {
Err(std::io::Error::from_raw_os_error(GetLastError() as i32))
} else {
Ok(path::PathBuf::from(OsString::from_wide(
&buffer[..result as usize],
)))
}
}
}

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

#[cfg(test)]
mod test {
use super::*;
Expand Down
2 changes: 2 additions & 0 deletions tests/source/mods-relative-paths/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#[path = "../mods-relative-paths/mod_b.rs"]
mod b;
7 changes: 7 additions & 0 deletions tests/source/mods-relative-paths/mod_b.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
fn



foo() {
println!("toto")
}
2 changes: 2 additions & 0 deletions tests/target/mods-relative-paths/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#[path = "../mods-relative-paths/mod_b.rs"]
mod b;
3 changes: 3 additions & 0 deletions tests/target/mods-relative-paths/mod_b.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn foo() {
println!("toto")
}