Skip to content
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

feat: Add DENO_CACHED_ONLY env var as alternative to --cached-only #12769

Closed
wants to merge 9 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
39 changes: 16 additions & 23 deletions cli/file_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,13 +375,7 @@ impl FileFetcher {
}

if self.cache_setting == CacheSetting::Only {
return Err(custom_error(
"NotCached",
format!(
"Specifier not found in cache: \"{}\", --cached-only is specified.",
specifier
),
));
return Err(cached_only_error(specifier));
}

let (source, content_type) = get_source_from_data_url(specifier)?;
Expand Down Expand Up @@ -424,13 +418,7 @@ impl FileFetcher {
}

if self.cache_setting == CacheSetting::Only {
return Err(custom_error(
"NotCached",
format!(
"Specifier not found in cache: \"{}\", --cached-only is specified.",
specifier
),
));
return Err(cached_only_error(specifier));
}

let blob = {
Expand Down Expand Up @@ -510,14 +498,7 @@ impl FileFetcher {
}

if self.cache_setting == CacheSetting::Only {
return futures::future::err(custom_error(
"NotCached",
format!(
"Specifier not found in cache: \"{}\", --cached-only is specified.",
specifier
),
))
.boxed();
return futures::future::err(cached_only_error(specifier)).boxed();
}

log::log!(
Expand Down Expand Up @@ -669,6 +650,16 @@ impl FileFetcher {
}
}

fn cached_only_error(specifier: &ModuleSpecifier) -> AnyError {
custom_error(
"NotCached",
format!(
"Specifier not found in cache: \"{}\", --cached-only or DENO_CACHED_ONLY is specified.",
specifier
),
)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -1518,7 +1509,9 @@ mod tests {
assert!(result.is_err());
let err = result.unwrap_err();
assert_eq!(get_custom_error_class(&err), Some("NotCached"));
assert_eq!(err.to_string(), "Specifier not found in cache: \"http://localhost:4545/002_hello.ts\", --cached-only is specified.");
assert!(err.to_string().contains(
"Specifier not found in cache: \"http://localhost:4545/002_hello.ts\""
));

let result = file_fetcher_02
.fetch(&specifier, &mut Permissions::allow_all())
Expand Down
7 changes: 4 additions & 3 deletions cli/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,13 +377,14 @@ static ENV_VARIABLES_HELP: &str = r#"ENVIRONMENT VARIABLES:
hostnames to use when fetching remote modules from
private repositories
(e.g. "abcde12345@deno.land;54321edcba@github.com")
DENO_TLS_CA_STORE Comma-separated list of order dependent certificate stores
(system, mozilla)
(defaults to mozilla)
DENO_CACHED_ONLY Only use already downloaded modules from cache.
DENO_CERT Load certificate authority from PEM encoded file
DENO_DIR Set the cache directory
DENO_INSTALL_ROOT Set deno install's output directory
(defaults to $HOME/.deno/bin)
DENO_TLS_CA_STORE Comma-separated list of order dependent certificate stores
(system, mozilla)
(defaults to mozilla)
DENO_WEBGPU_TRACE Directory to use for wgpu traces
HTTP_PROXY Proxy address for HTTP requests
(module downloads, fetch)
Expand Down
21 changes: 20 additions & 1 deletion cli/proc_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ impl ProcState {
eprintln!("{}", colors::yellow(msg));
}

let cache_usage = if flags.cached_only {
let cache_usage = if flags.cached_only || binary_env_var("DENO_CACHED_ONLY")
{
CacheSetting::Only
} else if !flags.cache_blocklist.is_empty() {
CacheSetting::ReloadSome(flags.cache_blocklist.clone())
Expand Down Expand Up @@ -695,3 +696,21 @@ fn source_map_from_code(code: String) -> Option<Vec<u8>> {
None
}
}

fn binary_env_var(name: &str) -> bool {
if let Ok(v) = env::var(name) {
if v == "1" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know my position on this, we should accept "true" and "false" for consistency.

This also works with NO_COLOR (evidence).

true
} else if v == "0" {
false
} else {
eprintln!(
"env var '{}' has a bad value '{}'. Use '1' or '0'.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for showing feedback when the env var has an unsupported value.

name, v
);
std::process::exit(1);
}
} else {
false
}
}
48 changes: 48 additions & 0 deletions cli/tests/integration/run_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2355,6 +2355,54 @@ fn issue12453() {
assert!(status.success());
}

#[test]
fn cached_only_env() {
let _g = util::http_server();
let deno_dir = util::new_deno_dir();

// First run with DENO_CACHED_ONLY=1 should fail, because nothing is cached.
let output = util::deno_cmd_with_deno_dir(deno_dir.path())
.current_dir(util::testdata_path())
.env("DENO_CACHED_ONLY", "1")
.arg("run")
.arg("--reload")
.arg("http://127.0.0.1:4545/019_media_types.ts")
.stderr(std::process::Stdio::piped())
.spawn()
.unwrap()
.wait_with_output()
.unwrap();
assert!(!output.status.success());
let stderr = std::str::from_utf8(&output.stderr).unwrap();
assert!(stderr.contains("Specifier not found in cache"));

// Now cache the program.
let status = util::deno_cmd_with_deno_dir(deno_dir.path())
.current_dir(util::testdata_path())
.arg("cache")
.arg("http://127.0.0.1:4545/019_media_types.ts")
.spawn()
.unwrap()
.wait()
.unwrap();
assert!(status.success());

// Run again with DENO_CACHED_ONLY=1, it should succeed.
let status = util::deno_cmd_with_deno_dir(deno_dir.path())
.current_dir(util::testdata_path())
.env("DENO_CACHED_ONLY", "1")
.arg("run")
.arg("--reload")
.arg("http://127.0.0.1:4545/019_media_types.ts")
.stderr(std::process::Stdio::null())
.stdout(std::process::Stdio::null())
.spawn()
.unwrap()
.wait()
.unwrap();
assert!(status.success());
}

/// Regression test for https://github.com/denoland/deno/issues/12740.
#[test]
fn issue12740() {
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/testdata/035_cached_only_flag.out
Original file line number Diff line number Diff line change
@@ -1 +1 @@
error: Specifier not found in cache: "http://127.0.0.1:4545/019_media_types.ts", --cached-only is specified.
error: Specifier not found in cache: "http://127.0.0.1:4545/019_media_types.ts", --cached-only or DENO_CACHED_ONLY is specified.