diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 90899704e6..d024f65767 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -458,7 +458,9 @@ impl Flags { Some(files.clone()) } else if let Run(RunFlags { script }) = &self.subcommand { if let Ok(module_specifier) = deno_core::resolve_url_or_path(script) { - if module_specifier.scheme() == "file" { + if module_specifier.scheme() == "file" + || module_specifier.scheme() == "npm" + { if let Ok(p) = module_specifier.to_file_path() { Some(vec![p]) } else { @@ -2145,16 +2147,17 @@ fn lock_arg<'a>() -> Arg<'a> { Arg::new("lock") .long("lock") .value_name("FILE") - .help("Check the specified lock file") + .help("Check the specified lock file. If value is not provided, defaults to \"deno.lock\" in the current working directory.") .takes_value(true) + .min_values(0) + .max_values(1) .value_hint(ValueHint::FilePath) } fn lock_write_arg<'a>() -> Arg<'a> { Arg::new("lock-write") .long("lock-write") - .requires("lock") - .help("Write lock file (use with --lock)") + .help("Force overwriting the lock file.") } static CONFIG_HELP: Lazy = Lazy::new(|| { @@ -3098,7 +3101,11 @@ fn lock_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) { fn lock_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches) { if matches.is_present("lock") { - let lockfile = matches.value_of("lock").unwrap(); + let lockfile = if let Some(path) = matches.value_of("lock") { + path + } else { + "./deno.lock" + }; flags.lock = Some(PathBuf::from(lockfile)); } } @@ -5335,6 +5342,57 @@ mod tests { ..Flags::default() } ); + + let r = flags_from_vec(svec![ + "deno", + "run", + "--lock", + "--lock-write", + "script.ts" + ]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Run(RunFlags { + script: "script.ts".to_string(), + }), + lock_write: true, + lock: Some(PathBuf::from("./deno.lock")), + ..Flags::default() + } + ); + + let r = flags_from_vec(svec![ + "deno", + "run", + "--lock-write", + "--lock", + "lock.json", + "script.ts" + ]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Run(RunFlags { + script: "script.ts".to_string(), + }), + lock_write: true, + lock: Some(PathBuf::from("lock.json")), + ..Flags::default() + } + ); + + let r = flags_from_vec(svec!["deno", "run", "--lock-write", "script.ts"]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Run(RunFlags { + script: "script.ts".to_string(), + }), + lock_write: true, + ..Flags::default() + } + ); } #[test] diff --git a/cli/args/mod.rs b/cli/args/mod.rs index e46313858e..64755a4948 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -24,6 +24,7 @@ use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::AnyError; use deno_core::normalize_path; +use deno_core::parking_lot::Mutex; use deno_core::url::Url; use deno_runtime::colors; use deno_runtime::deno_tls::rustls::RootCertStore; @@ -33,6 +34,7 @@ use std::collections::BTreeMap; use std::env; use std::net::SocketAddr; use std::path::PathBuf; +use std::sync::Arc; use crate::args::config_file::JsxImportSourceConfig; use crate::deno_dir::DenoDir; @@ -61,11 +63,16 @@ pub struct CliOptions { // application need not concern itself with, so keep these private flags: Flags, maybe_config_file: Option, + maybe_lockfile: Option>>, overrides: CliOptionOverrides, } impl CliOptions { - pub fn new(flags: Flags, maybe_config_file: Option) -> Self { + pub fn new( + flags: Flags, + maybe_config_file: Option, + maybe_lockfile: Option, + ) -> Self { if let Some(insecure_allowlist) = flags.unsafely_ignore_certificate_errors.as_ref() { @@ -80,8 +87,11 @@ impl CliOptions { eprintln!("{}", colors::yellow(msg)); } + let maybe_lockfile = maybe_lockfile.map(|l| Arc::new(Mutex::new(l))); + Self { maybe_config_file, + maybe_lockfile, flags, overrides: Default::default(), } @@ -89,7 +99,9 @@ impl CliOptions { pub fn from_flags(flags: Flags) -> Result { let maybe_config_file = ConfigFile::discover(&flags)?; - Ok(Self::new(flags, maybe_config_file)) + let maybe_lock_file = + Lockfile::discover(&flags, maybe_config_file.as_ref())?; + Ok(Self::new(flags, maybe_config_file, maybe_lock_file)) } pub fn maybe_config_file_specifier(&self) -> Option { @@ -210,13 +222,8 @@ impl CliOptions { .map(|host| InspectorServer::new(host, version::get_user_agent())) } - pub fn resolve_lock_file(&self) -> Result, AnyError> { - if let Some(filename) = &self.flags.lock { - let lockfile = Lockfile::new(filename.clone(), self.flags.lock_write)?; - Ok(Some(lockfile)) - } else { - Ok(None) - } + pub fn maybe_lock_file(&self) -> Option>> { + self.maybe_lockfile.clone() } pub fn resolve_tasks_config( diff --git a/cli/lockfile.rs b/cli/lockfile.rs index 78f5f2ab8b..b0cf689177 100644 --- a/cli/lockfile.rs +++ b/cli/lockfile.rs @@ -15,9 +15,11 @@ use std::path::PathBuf; use std::rc::Rc; use std::sync::Arc; +use crate::args::ConfigFile; use crate::npm::NpmPackageReq; use crate::npm::NpmResolutionPackage; use crate::tools::fmt::format_json; +use crate::Flags; #[derive(Debug)] pub struct LockfileError(String); @@ -73,6 +75,16 @@ pub struct LockfileContent { pub npm: NpmContent, } +impl LockfileContent { + fn empty() -> Self { + Self { + version: "2".to_string(), + remote: BTreeMap::new(), + npm: NpmContent::default(), + } + } +} + #[derive(Debug, Clone)] pub struct Lockfile { pub overwrite: bool, @@ -82,36 +94,91 @@ pub struct Lockfile { } impl Lockfile { + pub fn discover( + flags: &Flags, + maybe_config_file: Option<&ConfigFile>, + ) -> Result, AnyError> { + let filename = match flags.lock { + Some(ref lock) => PathBuf::from(lock), + None => match maybe_config_file { + Some(config_file) => { + if config_file.specifier.scheme() == "file" { + let mut path = config_file.specifier.to_file_path().unwrap(); + path.set_file_name("deno.lock"); + path + } else { + return Ok(None); + } + } + None => return Ok(None), + }, + }; + + let lockfile = Self::new(filename, flags.lock_write)?; + Ok(Some(lockfile)) + } + pub fn new(filename: PathBuf, overwrite: bool) -> Result { // Writing a lock file always uses the new format. - let content = if overwrite { + if overwrite { + return Ok(Lockfile { + overwrite, + has_content_changed: false, + content: LockfileContent::empty(), + filename, + }); + } + + let result = match std::fs::read_to_string(&filename) { + Ok(content) => Ok(content), + Err(e) => { + if e.kind() == std::io::ErrorKind::NotFound { + return Ok(Lockfile { + overwrite, + has_content_changed: false, + content: LockfileContent::empty(), + filename, + }); + } else { + Err(e) + } + } + }; + + let s = result.with_context(|| { + format!("Unable to read lockfile: \"{}\"", filename.display()) + })?; + let value: serde_json::Value = + serde_json::from_str(&s).with_context(|| { + format!( + "Unable to parse contents of the lockfile \"{}\"", + filename.display() + ) + })?; + let version = value.get("version").and_then(|v| v.as_str()); + let content = if version == Some("2") { + serde_json::from_value::(value).with_context(|| { + format!( + "Unable to parse contents of the lockfile \"{}\"", + filename.display() + ) + })? + } else { + // If there's no version field, we assume that user is using the old + // version of the lockfile. We'll migrate it in-place into v2 and it + // will be writte in v2 if user uses `--lock-write` flag. + let remote: BTreeMap = serde_json::from_value(value) + .with_context(|| { + format!( + "Unable to parse contents of the lockfile \"{}\"", + filename.display() + ) + })?; LockfileContent { version: "2".to_string(), - remote: BTreeMap::new(), + remote, npm: NpmContent::default(), } - } else { - let s = std::fs::read_to_string(&filename).with_context(|| { - format!("Unable to read lockfile: {}", filename.display()) - })?; - let value: serde_json::Value = serde_json::from_str(&s) - .context("Unable to parse contents of the lockfile")?; - let version = value.get("version").and_then(|v| v.as_str()); - if version == Some("2") { - serde_json::from_value::(value) - .context("Unable to parse lockfile")? - } else { - // If there's no version field, we assume that user is using the old - // version of the lockfile. We'll migrate it in-place into v2 and it - // will be writte in v2 if user uses `--lock-write` flag. - let remote: BTreeMap = - serde_json::from_value(value).context("Unable to parse lockfile")?; - LockfileContent { - version: "2".to_string(), - remote, - npm: NpmContent::default(), - } - } }; Ok(Lockfile { @@ -209,10 +276,15 @@ impl Lockfile { .unwrap_or(&package.dist.shasum); if &package_info.integrity != integrity { return Err(LockfileError(format!( - "Integrity check failed for npm package: \"{}\". - Cache has \"{}\" and lockfile has \"{}\". - Use \"--lock-write\" flag to update the lockfile.", - package.id, integrity, package_info.integrity + "Integrity check failed for npm package: \"{}\". Unable to verify that the package +is the same as when the lockfile was generated. + +This could be caused by: + * the lock file may be corrupt + * the source itself may be corrupt + +Use \"--lock-write\" flag to regenerate the lockfile at \"{}\".", + package.id, self.filename.display() ))); } } else { @@ -338,9 +410,9 @@ mod tests { } #[test] - fn new_nonexistent_lockfile() { + fn create_lockfile_for_nonexistent_path() { let file_path = PathBuf::from("nonexistent_lock_file.json"); - assert!(Lockfile::new(file_path, false).is_err()); + assert!(Lockfile::new(file_path, false).is_ok()); } #[test] diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index a3f516615a..3a0906636d 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -2916,6 +2916,8 @@ impl Inner { ..Default::default() }, self.maybe_config_file.clone(), + // TODO(#16510): add support for lockfile + None, ); cli_options.set_import_map_specifier(self.maybe_import_map_uri.clone()); diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 3ea4214ab5..148f44923d 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -168,9 +168,7 @@ impl ProcState { Some(progress_bar.clone()), )?; - let lockfile = cli_options - .resolve_lock_file()? - .map(|f| Arc::new(Mutex::new(f))); + let lockfile = cli_options.maybe_lock_file(); let maybe_import_map_specifier = cli_options.resolve_import_map_specifier()?; @@ -296,7 +294,6 @@ impl ProcState { npm_package_reqs.push(package_ref.req); } } - let roots = roots .into_iter() .map(|s| (s, ModuleKind::Esm)) @@ -312,6 +309,13 @@ impl ProcState { self.options.type_check_mode() != TypeCheckMode::None, false, ) { + // TODO(bartlomieju): this is strange... ideally there should be only + // one codepath in `prepare_module_load` so we don't forget things + // like writing a lockfile. Figure a way to refactor this function. + if let Some(ref lockfile) = self.lockfile { + let g = lockfile.lock(); + g.write()?; + } return result; } } diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs index a5b4431716..72af72a76c 100644 --- a/cli/tests/integration/npm_tests.rs +++ b/cli/tests/integration/npm_tests.rs @@ -998,6 +998,54 @@ fn lock_file_missing_top_level_package() { ); } +#[test] +fn auto_discover_lock_file() { + let _server = http_server(); + + let deno_dir = util::new_deno_dir(); + let temp_dir = util::TempDir::new(); + + // write empty config file + temp_dir.write("deno.json", "{}"); + + // write a lock file with borked integrity + let lock_file_content = r#"{ + "version": "2", + "remote": {}, + "npm": { + "specifiers": { "@denotest/bin": "@denotest/bin@1.0.0" }, + "packages": { + "@denotest/bin@1.0.0": { + "integrity": "sha512-foobar", + "dependencies": {} + } + } + } + }"#; + temp_dir.write("deno.lock", lock_file_content); + + let deno = util::deno_cmd_with_deno_dir(&deno_dir) + .current_dir(temp_dir.path()) + .arg("run") + .arg("--unstable") + .arg("-A") + .arg("npm:@denotest/bin/cli-esm") + .arg("test") + .envs(env_vars()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let output = deno.wait_with_output().unwrap(); + assert!(!output.status.success()); + assert_eq!(output.status.code(), Some(10)); + + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!(stderr.contains( + "Integrity check failed for npm package: \"@denotest/bin@1.0.0\"" + )); +} + fn env_vars_no_sync_download() -> Vec<(String, String)> { vec![ ("DENO_NODE_COMPAT_URL".to_string(), util::std_file_url()), diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 99e122c69b..68d15bfd9b 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -609,12 +609,6 @@ itest!(private_field_presence_no_check { output: "run/private_field_presence.ts.out", }); -itest!(lock_write_requires_lock { - args: "run --lock-write some_file.ts", - output: "run/lock_write_requires_lock.out", - exit_code: 1, -}); - // TODO(bartlomieju): remove --unstable once Deno.spawn is stabilized itest!(lock_write_fetch { args: @@ -3634,3 +3628,10 @@ fn websocket_server_idletimeout() { assert!(child.wait().unwrap().success()); } + +itest!(auto_discover_lockfile { + args: "run run/auto_discover_lockfile/main.ts", + output: "run/auto_discover_lockfile/main.out", + http_server: true, + exit_code: 10, +}); diff --git a/cli/tests/testdata/jsx/deno.lock b/cli/tests/testdata/jsx/deno.lock new file mode 100644 index 0000000000..64429f30a1 --- /dev/null +++ b/cli/tests/testdata/jsx/deno.lock @@ -0,0 +1,7 @@ +{ + "version": "2", + "remote": { + "http://localhost:4545/jsx/jsx-dev-runtime": "7cac3d940791b3c8e671b24f9678ca37d87d40487ed2b3720a2a40891aa6173d", + "http://localhost:4545/jsx/jsx-runtime": "7cac3d940791b3c8e671b24f9678ca37d87d40487ed2b3720a2a40891aa6173d" + } +} diff --git a/cli/tests/testdata/npm/lock_file/main.out b/cli/tests/testdata/npm/lock_file/main.out index 4c034d03b1..b8447bee63 100644 --- a/cli/tests/testdata/npm/lock_file/main.out +++ b/cli/tests/testdata/npm/lock_file/main.out @@ -1,4 +1,9 @@ Download [WILDCARD] -error: Integrity check failed for npm package: "@babel/parser@7.19.0". - Cache has "sha512-74bEXKX2h+8rrfQUfsBfuZZHzsEs6Eql4pqy/T4Nn6Y9wNPggQOqD6z6pn5Bl8ZfysKouFZT/UXEH94ummEeQw==" and lockfile has "sha512-foobar!". - Use "--lock-write" flag to update the lockfile. +error: Integrity check failed for npm package: "@babel/parser@7.19.0". Unable to verify that the package +is the same as when the lockfile was generated. + +This could be caused by: + * the lock file may be corrupt + * the source itself may be corrupt + +Use "--lock-write" flag to regenerate the lockfile at "[WILDCARD]lock.json". diff --git a/cli/tests/testdata/run/auto_discover_lockfile/deno.json b/cli/tests/testdata/run/auto_discover_lockfile/deno.json new file mode 100644 index 0000000000..90faa728a1 --- /dev/null +++ b/cli/tests/testdata/run/auto_discover_lockfile/deno.json @@ -0,0 +1,3 @@ +{ + "tasks": {} +} diff --git a/cli/tests/testdata/run/auto_discover_lockfile/deno.lock b/cli/tests/testdata/run/auto_discover_lockfile/deno.lock new file mode 100644 index 0000000000..059f66789f --- /dev/null +++ b/cli/tests/testdata/run/auto_discover_lockfile/deno.lock @@ -0,0 +1,7 @@ +{ + "version": "2", + "remote": { + "http://localhost:4545/subdir/mod2.ts": "cae1d3e9f3c38cd415ff52dff854be8f3d17d35f8d7b3d285e813fb0f6393a2f", + "http://localhost:4545/subdir/print_hello.ts": "foobar" + } +} diff --git a/cli/tests/testdata/run/auto_discover_lockfile/main.out b/cli/tests/testdata/run/auto_discover_lockfile/main.out new file mode 100644 index 0000000000..28f4724e98 --- /dev/null +++ b/cli/tests/testdata/run/auto_discover_lockfile/main.out @@ -0,0 +1,5 @@ +Download http://localhost:4545/subdir/mod2.ts +Download http://localhost:4545/subdir/print_hello.ts +error: The source code is invalid, as it does not match the expected hash in the lock file. + Specifier: http://localhost:4545/subdir/print_hello.ts + Lock file: [WILDCARD]auto_discover_lockfile[WILDCARD]deno.lock diff --git a/cli/tests/testdata/run/auto_discover_lockfile/main.ts b/cli/tests/testdata/run/auto_discover_lockfile/main.ts new file mode 100644 index 0000000000..baa52775d4 --- /dev/null +++ b/cli/tests/testdata/run/auto_discover_lockfile/main.ts @@ -0,0 +1 @@ +import "http://localhost:4545/subdir/mod2.ts"; diff --git a/cli/tests/testdata/run/config_types/deno.lock b/cli/tests/testdata/run/config_types/deno.lock new file mode 100644 index 0000000000..157bd98a2e --- /dev/null +++ b/cli/tests/testdata/run/config_types/deno.lock @@ -0,0 +1,6 @@ +{ + "version": "2", + "remote": { + "http://localhost:4545/run/config_types/types.d.ts": "741c39165e37de0c12acc5c081841f4362487e3f17dc4cad7017b70af72c4605" + } +} diff --git a/cli/tests/testdata/run/lock_write_requires_lock.out b/cli/tests/testdata/run/lock_write_requires_lock.out deleted file mode 100644 index 7cc5906f6c..0000000000 --- a/cli/tests/testdata/run/lock_write_requires_lock.out +++ /dev/null @@ -1,3 +0,0 @@ -error: The following required arguments were not provided: - --lock -[WILDCARD] \ No newline at end of file