Skip to content

Commit

Permalink
fix(config): do not canonicalize config file path before loading (#19436
Browse files Browse the repository at this point in the history
)

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 #7621
  • Loading branch information
dsherret committed Jun 9, 2023
1 parent f182c8a commit 5df735d
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 53 deletions.
91 changes: 40 additions & 51 deletions cli/args/config_file.rs
Expand Up @@ -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;

Expand Down Expand Up @@ -709,6 +708,24 @@ impl ConfigFile {
start: &Path,
checked: &mut HashSet<PathBuf>,
) -> Result<Option<ConfigFile>, AnyError> {
fn is_skippable_err(e: &AnyError) -> bool {
if let Some(ioerr) = e.downcast_ref::<std::io::Error>() {
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"];

Expand All @@ -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::<std::io::Error>() {
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);
}
}
}
Expand All @@ -755,50 +763,31 @@ impl ConfigFile {
pub fn read(config_path: &Path) -> Result<Self, AnyError> {
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<Self, AnyError> {
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<Self, AnyError> {
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<Self, AnyError> {
Expand Down
2 changes: 0 additions & 2 deletions cli/args/mod.rs
Expand Up @@ -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);
}

Expand Down

0 comments on commit 5df735d

Please sign in to comment.