From 5df735d3dad81feffe598a4561cdcbbe1bd0bd45 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 9 Jun 2023 08:56:11 -0400 Subject: [PATCH] fix(config): do not canonicalize config file path before loading (#19436) I'm unsure why we canonicalize the config file path when loading and the canonicalization is causing issues in #19431 because everything in the lsp is not canonicalized except the config file (actually, the config file is only canonicalized when auto-discovered and not whens pecified). We also don't canonicalize module paths when loading them. Canonicalization was added in https://github.com/denoland/deno/pull/7621 --- cli/args/config_file.rs | 91 ++++++++++++++++++----------------------- cli/args/mod.rs | 2 - 2 files changed, 40 insertions(+), 53 deletions(-) diff --git a/cli/args/config_file.rs b/cli/args/config_file.rs index 61f7778d9e..328a6e5748 100644 --- a/cli/args/config_file.rs +++ b/cli/args/config_file.rs @@ -2,7 +2,6 @@ use crate::args::ConfigFlag; use crate::args::Flags; -use crate::util::fs::canonicalize_path; use crate::util::path::specifier_parent; use crate::util::path::specifier_to_file_path; @@ -709,6 +708,24 @@ impl ConfigFile { start: &Path, checked: &mut HashSet, ) -> Result, AnyError> { + fn is_skippable_err(e: &AnyError) -> bool { + if let Some(ioerr) = e.downcast_ref::() { + use std::io::ErrorKind::*; + match ioerr.kind() { + InvalidInput | PermissionDenied | NotFound => { + // ok keep going + true + } + _ => { + const NOT_A_DIRECTORY: i32 = 20; + cfg!(unix) && ioerr.raw_os_error() == Some(NOT_A_DIRECTORY) + } + } + } else { + false + } + } + /// Filenames that Deno will recognize when discovering config. const CONFIG_FILE_NAMES: [&str; 2] = ["deno.json", "deno.jsonc"]; @@ -729,20 +746,11 @@ impl ConfigFile { log::debug!("Config file found at '{}'", f.display()); return Ok(Some(cf)); } + Err(e) if is_skippable_err(&e) => { + // ok, keep going + } Err(e) => { - if let Some(ioerr) = e.downcast_ref::() { - use std::io::ErrorKind::*; - match ioerr.kind() { - InvalidInput | PermissionDenied | NotFound => { - // ok keep going - } - _ => { - return Err(e); // Unknown error. Stop. - } - } - } else { - return Err(e); // Parse error or something else. Stop. - } + return Err(e); } } } @@ -755,50 +763,31 @@ impl ConfigFile { pub fn read(config_path: &Path) -> Result { debug_assert!(config_path.is_absolute()); - // perf: Check if the config file exists before canonicalizing path. - if !config_path.exists() { - return Err( - std::io::Error::new( - std::io::ErrorKind::InvalidInput, - format!( - "Could not find the config file: {}", - config_path.to_string_lossy() - ), - ) - .into(), - ); - } - - let config_path = canonicalize_path(config_path).map_err(|_| { - std::io::Error::new( - std::io::ErrorKind::InvalidInput, - format!( - "Could not find the config file: {}", - config_path.to_string_lossy() - ), - ) - })?; - let config_specifier = ModuleSpecifier::from_file_path(&config_path) - .map_err(|_| { + let specifier = + ModuleSpecifier::from_file_path(config_path).map_err(|_| { anyhow!( - "Could not convert path to specifier. Path: {}", + "Could not convert config file path to specifier. Path: {}", config_path.display() ) })?; - Self::from_specifier(config_specifier) + Self::from_specifier_and_path(specifier, config_path) } pub fn from_specifier(specifier: ModuleSpecifier) -> Result { - let config_path = specifier_to_file_path(&specifier)?; - let config_text = match std::fs::read_to_string(config_path) { - Ok(text) => text, - Err(err) => bail!( - "Error reading config file {}: {}", - specifier, - err.to_string() - ), - }; - Self::new(&config_text, specifier) + let config_path = + specifier_to_file_path(&specifier).with_context(|| { + format!("Invalid config file path for '{}'.", specifier) + })?; + Self::from_specifier_and_path(specifier, &config_path) + } + + fn from_specifier_and_path( + specifier: ModuleSpecifier, + config_path: &Path, + ) -> Result { + let text = std::fs::read_to_string(config_path) + .with_context(|| format!("Error reading config file '{}'.", specifier))?; + Self::new(&text, specifier) } pub fn new(text: &str, specifier: ModuleSpecifier) -> Result { diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 6dab0c973b..d740b9ab08 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -396,8 +396,6 @@ fn discover_package_json( // `package.json` is ignored in bundle/compile/etc. if let Some(package_json_dir) = flags.package_json_search_dir(current_dir) { - let package_json_dir = - canonicalize_path_maybe_not_exists(&package_json_dir)?; return package_json::discover_from(&package_json_dir, maybe_stop_at); }