Skip to content

feat(vendor): support modifying remote files in vendor folder without checksum errors #23979

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

Merged
merged 19 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from 10 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
8 changes: 4 additions & 4 deletions .dprint.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@
"ext/websocket/autobahn/reports"
],
"plugins": [
"https://plugins.dprint.dev/typescript-0.90.5.wasm",
"https://plugins.dprint.dev/json-0.19.2.wasm",
"https://plugins.dprint.dev/markdown-0.17.0.wasm",
"https://plugins.dprint.dev/toml-0.6.1.wasm",
"https://plugins.dprint.dev/typescript-0.91.0.wasm",
"https://plugins.dprint.dev/json-0.19.3.wasm",
"https://plugins.dprint.dev/markdown-0.17.1.wasm",
"https://plugins.dprint.dev/toml-0.6.2.wasm",
"https://plugins.dprint.dev/exec-0.4.4.json@c207bf9b9a4ee1f0ecb75c594f774924baf62e8e53a2ce9d873816a408cecbf7"
]
}
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
23 changes: 21 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,23 @@ pub fn discover(
},
};

let lockfile = Lockfile::new(filename, flags.lock_write)?;
let lockfile = if flags.lock_write {
Lockfile::new_empty(filename, true)
} else {
let file_text = std::fs::read_to_string(&filename).with_context(|| {
format!("Failed reading lockfile '{}'", filename.display())
})?;
Lockfile::with_lockfile_content(filename, &file_text, false)?
};
Ok(Some(lockfile))
}

pub fn write_lockfile_if_has_changes(
lockfile: &Lockfile,
) -> Result<(), AnyError> {
let Some(bytes) = lockfile.resolve_write_bytes() else {
return Ok(()); // nothing to do
};
atomic_write_file(&lockfile.filename, bytes, cache::CACHE_PERM)
.context("Failed writing lockfile.")
}
2 changes: 1 addition & 1 deletion cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ pub use deno_config::TsConfigType;
pub use deno_config::TsTypeLib;
pub use deno_config::WorkspaceConfig;
pub use flags::*;
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
24 changes: 17 additions & 7 deletions cli/cache/module_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ use deno_ast::ModuleSpecifier;
use deno_core::error::AnyError;
use deno_core::serde_json;
use deno_graph::ModuleInfo;
use deno_graph::ModuleParser;
use deno_graph::ParserModuleAnalyzer;
use deno_runtime::deno_webstorage::rusqlite::params;

use super::cache_db::CacheDB;
use super::cache_db::CacheDBConfiguration;
use super::cache_db::CacheFailure;
use super::FastInsecureHasher;
use super::ParsedSourceCache;

const SELECT_MODULE_INFO: &str = "
SELECT
Expand Down Expand Up @@ -136,22 +136,23 @@ impl ModuleInfoCache {

pub fn as_module_analyzer<'a>(
&'a self,
parser: &'a dyn ModuleParser,
parsed_source_cache: Arc<ParsedSourceCache>,
) -> ModuleInfoCacheModuleAnalyzer<'a> {
ModuleInfoCacheModuleAnalyzer {
module_info_cache: self,
parser,
parsed_source_cache,
}
}
}

pub struct ModuleInfoCacheModuleAnalyzer<'a> {
module_info_cache: &'a ModuleInfoCache,
parser: &'a dyn ModuleParser,
parsed_source_cache: Arc<ParsedSourceCache>,
}

#[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,17 @@ impl<'a> deno_graph::ModuleAnalyzer for ModuleInfoCacheModuleAnalyzer<'a> {
}

// otherwise, get the module info from the parsed source cache
let analyzer = ParserModuleAnalyzer::new(self.parser);
let module_info = analyzer.analyze(specifier, source, media_type)?;
let module_info = deno_core::unsync::spawn_blocking({
let cache = self.parsed_source_cache.clone();
let specifier = specifier.clone();
move || {
let parser = cache.as_capturing_parser();
let analyzer = ParserModuleAnalyzer::new(&parser);
analyzer.analyze_sync(&specifier, source, media_type)
}
})
.await
.unwrap()?;

// 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