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(vendor): support modifying remote files in vendor folder without checksum errors #23979

Merged
merged 19 commits into from
May 28, 2024
Merged
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
30 changes: 14 additions & 16 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ deno_ast = { version = "=0.38.2", features = ["transpiling"] }
deno_core = { version = "0.283.0" }

deno_bench_util = { version = "0.147.0", path = "./bench_util" }
deno_lockfile = "0.19.0"
deno_lockfile = "0.20.0"
deno_media_type = { version = "0.1.4", features = ["module_specifier"] }
deno_permissions = { version = "0.13.0", path = "./runtime/permissions" }
deno_runtime = { version = "0.161.0", path = "./runtime" }
Expand Down Expand Up @@ -100,7 +100,7 @@ chrono = { version = "0.4", default-features = false, features = ["std", "serde"
console_static_text = "=0.8.1"
data-encoding = "2.3.3"
data-url = "=0.3.0"
deno_cache_dir = "=0.7.1"
deno_cache_dir = "=0.10.0"
dlopen2 = "0.6.1"
ecb = "=0.1.2"
elliptic-curve = { version = "0.13.4", features = ["alloc", "arithmetic", "ecdh", "std", "pem"] }
Expand Down
10 changes: 5 additions & 5 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,17 @@ deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "proposa
deno_cache_dir = { workspace = true }
deno_config = "=0.16.3"
deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
deno_doc = { version = "=0.135.0", features = ["html", "syntect"] }
deno_emit = "=0.40.3"
deno_graph = { version = "=0.75.2", features = ["tokio_executor"] }
deno_doc = { version = "=0.137.0", features = ["html", "syntect"] }
deno_emit = "=0.41.0"
deno_graph = { version = "=0.77.0", features = ["tokio_executor"] }
Copy link
Member Author

Choose a reason for hiding this comment

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

It seems some wpt tests are failing due to denoland/deno_graph#483

See https://github.com/denoland/deno/actions/runs/9262872207

I'm unsure how to interpret the wpt test output. Are we incorrectly failing now?

Copy link
Member

Choose a reason for hiding this comment

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

I have updated the expectations. We were previously failing these tests, now the tests pass. The expectations file was assuming the tests to fail though, so them passing caused the suite to fail.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks!

deno_lint = { version = "=0.58.4", features = ["docs"] }
deno_lockfile.workspace = true
deno_npm = "=0.20.2"
deno_npm = "=0.21.0"
deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting"] }
deno_semver = "=0.5.4"
deno_task_shell = "=0.16.1"
deno_terminal.workspace = true
eszip = "=0.69.0"
eszip = "=0.70.1"
napi_sym.workspace = true

async-trait.workspace = true
Expand Down
34 changes: 32 additions & 2 deletions cli/args/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,20 @@

use std::path::PathBuf;

use deno_core::anyhow::Context;
use deno_core::error::AnyError;
use deno_runtime::deno_node::PackageJson;

use crate::args::ConfigFile;
use crate::cache;
use crate::util::fs::atomic_write_file;
use crate::Flags;

use super::DenoSubcommand;
use super::InstallFlags;
use super::InstallKind;

pub use deno_lockfile::Lockfile;
pub use deno_lockfile::LockfileError;

pub fn discover(
flags: &Flags,
Expand Down Expand Up @@ -54,6 +56,34 @@ pub fn discover(
},
};

let lockfile = Lockfile::new(filename, flags.lock_write)?;
let lockfile = if flags.lock_write {
Lockfile::new_empty(filename, true)
} else {
read_lockfile_at_path(filename)?
};
Ok(Some(lockfile))
}

pub fn read_lockfile_at_path(filename: PathBuf) -> Result<Lockfile, AnyError> {
match std::fs::read_to_string(&filename) {
Ok(text) => Ok(Lockfile::with_lockfile_content(filename, &text, false)?),
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
Ok(Lockfile::new_empty(filename, false))
}
Err(err) => Err(err).with_context(|| {
format!("Failed reading lockfile '{}'", filename.display())
}),
}
}

pub fn write_lockfile_if_has_changes(
lockfile: &Lockfile,
) -> Result<(), AnyError> {
let Some(bytes) = lockfile.resolve_write_bytes() else {
return Ok(()); // nothing to do
};
// do an atomic write to reduce the chance of multiple deno
// processes corrupting the file
atomic_write_file(&lockfile.filename, bytes, cache::CACHE_PERM)
.context("Failed writing lockfile.")
}
3 changes: 2 additions & 1 deletion cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ pub use deno_config::TsConfigType;
pub use deno_config::TsTypeLib;
pub use deno_config::WorkspaceConfig;
pub use flags::*;
pub use lockfile::read_lockfile_at_path;
pub use lockfile::write_lockfile_if_has_changes;
pub use lockfile::Lockfile;
pub use lockfile::LockfileError;
pub use package_json::PackageJsonDepsProvider;

use deno_ast::ModuleSpecifier;
Expand Down
2 changes: 1 addition & 1 deletion cli/cache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ impl Loader for FetchCacher {
LoaderCacheSetting::Reload => {
if matches!(file_fetcher.cache_setting(), CacheSetting::Only) {
return Err(deno_core::anyhow::anyhow!(
"Failed to resolve version constraint. Try running again without --cached-only"
"Could not resolve version constraint using only cached data. Try running again without --cached-only"
));
}
Some(CacheSetting::ReloadAll)
Expand Down
6 changes: 4 additions & 2 deletions cli/cache/module_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,9 @@ pub struct ModuleInfoCacheModuleAnalyzer<'a> {
parser: &'a dyn ModuleParser,
}

#[async_trait::async_trait(?Send)]
impl<'a> deno_graph::ModuleAnalyzer for ModuleInfoCacheModuleAnalyzer<'a> {
fn analyze(
async fn analyze(
&self,
specifier: &ModuleSpecifier,
source: Arc<str>,
Expand All @@ -176,8 +177,9 @@ impl<'a> deno_graph::ModuleAnalyzer for ModuleInfoCacheModuleAnalyzer<'a> {
}

// otherwise, get the module info from the parsed source cache
// todo(23858): take advantage of this being async
let analyzer = ParserModuleAnalyzer::new(self.parser);
let module_info = analyzer.analyze(specifier, source, media_type)?;
let module_info = analyzer.analyze(specifier, source, media_type).await?;

// then attempt to cache it
if let Err(err) = self.module_info_cache.set_module_info(
Expand Down
55 changes: 44 additions & 11 deletions cli/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use deno_core::error::AnyError;
use deno_graph::source::ResolveError;
use deno_graph::ModuleError;
use deno_graph::ModuleGraphError;
use deno_graph::ModuleLoadError;
use deno_graph::ResolutionError;
use import_map::ImportMapError;
use std::fmt::Write;
Expand All @@ -27,24 +28,56 @@ fn get_diagnostic_class(_: &ParseDiagnostic) -> &'static str {
}

fn get_module_graph_error_class(err: &ModuleGraphError) -> &'static str {
use deno_graph::JsrLoadError;
use deno_graph::NpmLoadError;
use deno_graph::WorkspaceLoadError;

match err {
ModuleGraphError::ResolutionError(err)
| ModuleGraphError::TypesResolutionError(err) => {
get_resolution_error_class(err)
}
ModuleGraphError::ModuleError(err) => match err {
ModuleError::LoadingErr(_, _, err) => get_error_class_name(err.as_ref()),
ModuleError::InvalidTypeAssertion { .. } => "SyntaxError",
ModuleError::ParseErr(_, diagnostic) => get_diagnostic_class(diagnostic),
ModuleError::UnsupportedMediaType { .. }
| ModuleError::UnsupportedImportAttributeType { .. } => "TypeError",
ModuleError::Missing(_, _)
| ModuleError::MissingDynamic(_, _)
| ModuleError::MissingWorkspaceMemberExports { .. }
| ModuleError::UnknownExport { .. }
| ModuleError::UnknownPackage { .. }
| ModuleError::UnknownPackageReq { .. } => "NotFound",
ModuleError::Missing(_, _) | ModuleError::MissingDynamic(_, _) => {
"NotFound"
}
ModuleError::LoadingErr(_, _, err) => match err {
ModuleLoadError::Loader(err) => get_error_class_name(err.as_ref()),
ModuleLoadError::HttpsChecksumIntegrity(_)
| ModuleLoadError::TooManyRedirects => "Error",
ModuleLoadError::NodeUnknownBuiltinModule(_) => "NotFound",
ModuleLoadError::Decode(_) => "TypeError",
ModuleLoadError::Npm(err) => match err {
NpmLoadError::NotSupportedEnvironment
| NpmLoadError::PackageReqResolution(_)
| NpmLoadError::RegistryInfo(_) => "Error",
NpmLoadError::PackageReqReferenceParse(_) => "TypeError",
},
ModuleLoadError::Jsr(err) => match err {
JsrLoadError::UnsupportedManifestChecksum
| JsrLoadError::PackageFormat(_) => "TypeError",
JsrLoadError::ContentLoadExternalSpecifier
| JsrLoadError::ContentLoad(_)
| JsrLoadError::ContentChecksumIntegrity(_)
| JsrLoadError::PackageManifestLoad(_, _)
| JsrLoadError::PackageVersionManifestChecksumIntegrity(..)
| JsrLoadError::PackageVersionManifestLoad(_, _)
| JsrLoadError::RedirectInPackage(_) => "Error",
JsrLoadError::PackageNotFound(_)
| JsrLoadError::PackageReqNotFound(_)
| JsrLoadError::PackageVersionNotFound(_)
| JsrLoadError::UnknownExport { .. } => "NotFound",
},
ModuleLoadError::Workspace(err) => match err {
WorkspaceLoadError::MemberInvalidExportPath { .. } => "TypeError",
WorkspaceLoadError::MissingMemberExports { .. } => "NotFound",
},
},
},
ModuleGraphError::ResolutionError(err)
| ModuleGraphError::TypesResolutionError(err) => {
get_resolution_error_class(err)
}
}
}

Expand Down
1 change: 0 additions & 1 deletion cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,6 @@ impl CliFactory {
self.options.clone(),
self.npm_resolver().await?.clone(),
self.module_graph_builder().await?.clone(),
self.maybe_lockfile().clone(),
self.type_checker().await?.clone(),
)))
})
Expand Down
23 changes: 19 additions & 4 deletions cli/file_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,15 +253,30 @@ impl FileFetcher {
deno_core::resolve_import(redirect_to, specifier.as_str())?;
return Ok(Some(FileOrRedirect::Redirect(redirect)));
}
let Some(bytes) = self.http_cache.read_file_bytes(
let result = self.http_cache.read_file_bytes(
&cache_key,
maybe_checksum
.as_ref()
.map(|c| deno_cache_dir::Checksum::new(c.as_str())),
deno_cache_dir::GlobalToLocalCopy::Allow,
)?
else {
return Ok(None);
);
let bytes = match result {
Ok(Some(bytes)) => bytes,
Ok(None) => return Ok(None),
Err(err) => match err {
deno_cache_dir::CacheReadFileError::Io(err) => return Err(err.into()),
deno_cache_dir::CacheReadFileError::ChecksumIntegrity(err) => {
// convert to the equivalent deno_graph error so that it
// enhances it if this is passed to deno_graph
return Err(
deno_graph::source::ChecksumIntegrityError {
actual: err.actual,
expected: err.expected,
}
.into(),
);
}
},
};

Ok(Some(FileOrRedirect::File(File {
Expand Down
Loading