Skip to content

Commit 38f3ffe

Browse files
authored
refactor: reduce cwd lookups and enforce canonicalize_path (#32250)
1. Enforces using `resolve_cwd`, which is more efficient and reduces. 2. Uses `canonicalize_path` instead of `.canonicalize()` because it strips the unc path on Windows without us having to remember to do that.
1 parent 43b1c85 commit 38f3ffe

32 files changed

+146
-117
lines changed

cli/args/flags.rs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ use serde::Deserialize;
6767
use serde::Serialize;
6868

6969
use super::flags_net;
70+
use crate::util::env::resolve_cwd;
7071
use crate::util::fs::canonicalize_path;
7172

7273
#[derive(Clone, Debug, Default, Eq, PartialEq)]
@@ -1415,7 +1416,7 @@ impl Flags {
14151416
exclude: excluded_paths,
14161417
..
14171418
})) => {
1418-
let cwd = std::env::current_dir()?;
1419+
let cwd = resolve_cwd(self.initial_cwd.as_deref())?;
14191420
PathOrPatternSet::from_exclude_relative_path_or_patterns(
14201421
&cwd,
14211422
excluded_paths,
@@ -6145,7 +6146,8 @@ fn completions_parse(
61456146
unsafe {
61466147
std::env::set_var("COMPLETE", &shell);
61476148
}
6148-
handle_shell_completion_with_args(std::env::args_os().take(1))?;
6149+
let cwd = resolve_cwd(None)?;
6150+
handle_shell_completion_with_args(std::env::args_os().take(1), &cwd)?;
61496151
Ok(())
61506152
}),
61516153
));
@@ -7024,8 +7026,8 @@ fn task_parse(
70247026
Ok(())
70257027
}
70267028

7027-
pub fn handle_shell_completion() -> Result<(), AnyError> {
7028-
handle_shell_completion_with_args(std::env::args_os())
7029+
pub fn handle_shell_completion(cwd: &Path) -> Result<(), AnyError> {
7030+
handle_shell_completion_with_args(std::env::args_os(), cwd)
70297031
}
70307032

70317033
struct ZshCompleterUnsorted;
@@ -7112,6 +7114,7 @@ compdef _clap_dynamic_completer_NAME BIN"#
71127114

71137115
fn handle_shell_completion_with_args(
71147116
args: impl IntoIterator<Item = OsString>,
7117+
cwd: &Path,
71157118
) -> Result<(), AnyError> {
71167119
let args = args.into_iter().collect::<Vec<_>>();
71177120
let app = clap_root();
@@ -7124,7 +7127,7 @@ fn handle_shell_completion_with_args(
71247127
&clap_complete::env::Powershell,
71257128
&ZshCompleterUnsorted,
71267129
]))
7127-
.try_complete(args, Some(&std::env::current_dir()?))?;
7130+
.try_complete(args, Some(cwd))?;
71287131

71297132
// we should only run this function when we're doing completions
71307133
assert!(ran_completion);
@@ -12560,12 +12563,12 @@ mod tests {
1256012563
#[test]
1256112564
fn test_config_path_args() {
1256212565
let flags = flags_from_vec(svec!["deno", "run", "foo.js"]).unwrap();
12563-
let cwd = std::env::current_dir().unwrap();
12566+
let cwd = resolve_cwd(None).unwrap().into_owned();
1256412567

1256512568
assert_eq!(flags.config_path_args(&cwd), Some(vec![cwd.clone()]));
1256612569

1256712570
let flags = flags_from_vec(svec!["deno", "run", "sub_dir/foo.js"]).unwrap();
12568-
let cwd = std::env::current_dir().unwrap();
12571+
let cwd = resolve_cwd(None).unwrap().into_owned();
1256912572
assert_eq!(
1257012573
flags.config_path_args(&cwd),
1257112574
Some(vec![cwd.join("sub_dir").clone()])

cli/args/mod.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1785,13 +1785,14 @@ mod test {
17851785
use pretty_assertions::assert_eq;
17861786

17871787
use super::*;
1788+
use crate::util::env::resolve_cwd;
17881789

17891790
#[test]
17901791
fn resolve_import_map_flags_take_precedence() {
17911792
let config_text = r#"{
17921793
"importMap": "import_map.json"
17931794
}"#;
1794-
let cwd = &std::env::current_dir().unwrap();
1795+
let cwd = &resolve_cwd(None).unwrap();
17951796
let config_specifier = Url::parse("file:///deno/deno.jsonc").unwrap();
17961797
let config_file = ConfigFile::new(config_text, config_specifier).unwrap();
17971798
let actual = resolve_import_map_specifier(
@@ -1859,7 +1860,7 @@ mod test {
18591860

18601861
#[test]
18611862
fn test_flags_to_permission_options() {
1862-
let base_dir = std::env::current_dir().unwrap().join("sub");
1863+
let base_dir = resolve_cwd(None).unwrap().join("sub");
18631864
{
18641865
let flags = PermissionFlags::default();
18651866
let config = PermissionsObjectWithBase {

cli/build.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ fn compress_decls(out_dir: &Path) {
135135
}
136136

137137
fn process_node_types(out_dir: &Path) {
138+
#[allow(clippy::disallowed_methods)] // build script
138139
let root_dir = Path::new(".").canonicalize().unwrap();
139140
let dts_dir = root_dir.join("tsc").join("dts");
140141
let node_dir = dts_dir.join("node");
@@ -201,6 +202,7 @@ fn process_node_types(out_dir: &Path) {
201202
}
202203

203204
fn compress_source(out_dir: &Path, file: &str) {
205+
#[allow(clippy::disallowed_methods)] // build script
204206
let path = Path::new(file)
205207
.canonicalize()
206208
.unwrap_or_else(|_| panic!("expected file \"{file}\" to exist"));

cli/clippy.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
disallowed-methods = [
22
{ path = "reqwest::Client::new", reason = "create an HttpClient via an HttpClientProvider instead" },
3+
{ path = "std::fs::canonicalize", reason = "use crate::util::fs::canonicalize_path instead" },
4+
{ path = "std::path::Path::canonicalize", reason = "use crate::util::fs::canonicalize_path instead" },
5+
{ path = "std::env::current_dir", reason = "use crate::util::env::resolve_cwd instead and prefer passing it the initial_cwd" },
36
{ path = "std::process::exit", reason = "use deno_runtime::exit instead" },
47
{ path = "clap::Arg::env", reason = "ensure environment variables are resolved after loading the .env file instead" },
58
{ path = "tokio::signal::ctrl_c", reason = "use deno_signals crate instead" },

cli/factory.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,7 +1150,6 @@ impl CliFactory {
11501150
self.npm_installer_if_managed().await?.cloned(),
11511151
npm_resolver.clone(),
11521152
self.text_only_progress_bar().clone(),
1153-
self.sys(),
11541153
self.create_cli_main_worker_options()?,
11551154
self.root_permissions_container()?.clone(),
11561155
))
@@ -1228,7 +1227,7 @@ impl CliFactory {
12281227
create_hmr_runner,
12291228
maybe_coverage_dir,
12301229
default_npm_caching_strategy: cli_options.default_npm_caching_strategy(),
1231-
maybe_initial_cwd: Some(Arc::new(initial_cwd)),
1230+
initial_cwd: Arc::new(initial_cwd),
12321231
})
12331232
}
12341233

cli/lib.rs

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,13 @@ use deno_runtime::tokio_util::create_and_run_current_thread_with_maybe_metrics;
5555
use deno_telemetry::OtelConfig;
5656
use deno_terminal::colors;
5757
use factory::CliFactory;
58+
use util::fs::canonicalize_path;
5859

5960
const MODULE_NOT_FOUND: &str = "Module not found";
6061
const UNSUPPORTED_SCHEME: &str = "Unsupported scheme";
6162

6263
use self::util::draw_thread::DrawThread;
64+
use self::util::env::resolve_cwd;
6365
use crate::args::CompletionsFlags;
6466
use crate::args::DenoSubcommand;
6567
use crate::args::Flags;
@@ -193,9 +195,9 @@ async fn run_subcommand(
193195
DenoSubcommand::Fmt(fmt_flags) => spawn_subcommand(async move {
194196
tools::fmt::format(Arc::new(flags), fmt_flags).await
195197
}),
196-
DenoSubcommand::Init(init_flags) => {
197-
spawn_subcommand(async { tools::init::init_project(init_flags).await })
198-
}
198+
DenoSubcommand::Init(init_flags) => spawn_subcommand(async {
199+
tools::init::init_project(flags, init_flags).await
200+
}),
199201
DenoSubcommand::Info(info_flags) => spawn_subcommand(async {
200202
tools::info::info(Arc::new(flags), info_flags).await
201203
}),
@@ -416,7 +418,7 @@ async fn run_subcommand(
416418
unsafe {
417419
env::set_var(
418420
"DENO_COVERAGE_DIR",
419-
PathBuf::from(coverage_dir).canonicalize()?,
421+
canonicalize_path(&PathBuf::from(coverage_dir))?,
420422
)
421423
};
422424
}
@@ -659,15 +661,17 @@ pub fn main() {
659661
(None, None, None);
660662

661663
let args = waited_args.unwrap_or(args);
662-
let initial_cwd = waited_cwd.map(Some).unwrap_or_else(|| {
663-
match std::env::current_dir().with_context(|| "Failed getting cwd.") {
664-
Ok(cwd) => Some(cwd),
665-
Err(err) => {
666-
log::error!("Failed getting cwd: {err}");
667-
None
668-
}
669-
}
670-
});
664+
#[allow(clippy::disallowed_methods)] // ok because initialization of cwd
665+
let initial_cwd =
666+
waited_cwd
667+
.map(Some)
668+
.unwrap_or_else(|| match std::env::current_dir() {
669+
Ok(cwd) => Some(cwd),
670+
Err(err) => {
671+
log::error!("Failed getting cwd: {err}");
672+
None
673+
}
674+
});
671675

672676
// NOTE(lucacasonato): due to new PKU feature introduced in V8 11.6 we need to
673677
// initialize the V8 platform on a parent thread of all threads that will spawn
@@ -706,7 +710,8 @@ async fn resolve_flags_and_init(
706710
// this env var is used by clap to enable dynamic completions, it's set by the shell when
707711
// executing deno to get dynamic completions.
708712
if std::env::var("COMPLETE").is_ok() {
709-
crate::args::handle_shell_completion()?;
713+
let cwd = resolve_cwd(initial_cwd.as_deref())?;
714+
crate::args::handle_shell_completion(&cwd)?;
710715
deno_runtime::exit(0);
711716
}
712717

cli/lsp/completions.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ use super::tsc;
3838
use crate::jsr::JsrFetchResolver;
3939
use crate::lsp::registries::DocumentationCompletionItemData;
4040
use crate::lsp::tsgo;
41+
use crate::util::env::resolve_cwd;
4142
use crate::util::path::is_importable_ext;
4243
use crate::util::path::relative_specifier;
4344

@@ -434,7 +435,7 @@ fn get_local_completions(
434435
.ok()?;
435436
let resolved_parent_path = url_to_file_path(&resolved_parent).ok()?;
436437
if resolved_parent_path.is_dir() {
437-
let cwd = std::env::current_dir().ok()?;
438+
let cwd = resolve_cwd(None).ok()?;
438439
let entries = std::fs::read_dir(resolved_parent_path).ok()?;
439440
let items = entries
440441
.filter_map(|de| {

cli/lsp/config.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ use crate::http_util::HttpClientProvider;
7070
use crate::lsp::logging::lsp_warn;
7171
use crate::npm::CliNpmCacheHttpClient;
7272
use crate::sys::CliSys;
73+
use crate::util::fs::canonicalize_path;
7374
use crate::util::fs::canonicalize_path_maybe_not_exists;
7475
use crate::util::progress_bar::ProgressBar;
7576
use crate::util::progress_bar::ProgressBarStyle;
@@ -1845,7 +1846,7 @@ impl ConfigTree {
18451846
let pkg_json_cache = PackageJsonMemCache::default();
18461847
let workspace_cache = WorkspaceMemCache::default();
18471848
let mut scopes = BTreeMap::new();
1848-
let fs_root_url = std::fs::canonicalize("/")
1849+
let fs_root_url = canonicalize_path(Path::new("/"))
18491850
.ok()
18501851
.and_then(|p| Url::from_directory_path(p).ok())
18511852
.unwrap_or_else(|| Url::parse("file:///").unwrap());

cli/lsp/language_server.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ use crate::tools::fmt::format_file;
118118
use crate::tools::fmt::format_parsed_source;
119119
use crate::tools::upgrade::check_for_upgrades_for_lsp;
120120
use crate::tools::upgrade::upgrade_check_enabled;
121+
use crate::util::env::resolve_cwd;
121122
use crate::util::fs::remove_dir_all_if_exists;
122123
use crate::util::path::to_percent_decoded_str;
123124
use crate::util::sync::AsyncFlag;
@@ -584,9 +585,7 @@ impl Inner {
584585
cache.deno_dir(),
585586
&http_client_provider,
586587
));
587-
let initial_cwd = std::env::current_dir().unwrap_or_else(|_| {
588-
panic!("Could not resolve current working directory")
589-
});
588+
let initial_cwd = resolve_cwd(None).unwrap().into_owned();
590589

591590
Self {
592591
ambient_modules_regex_cache: Default::default(),
@@ -3987,7 +3986,7 @@ struct PrepareCacheResult {
39873986
impl Inner {
39883987
async fn initialized(&mut self) -> Vec<Registration> {
39893988
let mut registrations = Vec::with_capacity(2);
3990-
init_log_file(self.config.log_file());
3989+
init_log_file(self.config.log_file(), &self.initial_cwd);
39913990
self.update_debug_flag();
39923991
self.update_global_cache().await;
39933992
self.refresh_workspace_files();

cli/lsp/logging.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,11 @@ impl LogFile {
5353
}
5454
}
5555

56-
pub fn init_log_file(enabled: bool) {
56+
pub fn init_log_file(enabled: bool, cwd: &Path) {
5757
let prepare_path = || {
5858
if !enabled {
5959
return None;
6060
}
61-
let cwd = std::env::current_dir().ok()?;
6261
let now = SystemTime::now();
6362
let now: DateTime<Utc> = now.into();
6463
let now = now.to_rfc3339().replace(':', "_");

0 commit comments

Comments
 (0)