fix(install): store tags associated with package in node_modules dir (#26000)

Fixes #25998. Fixes https://github.com/denoland/deno/issues/25928.

Originally I was just going to make this an error message instead of a
panic, but once I got to a minimal repro I felt that this really should
work.

The panic occurs when you have `nodeModulesDir: manual` (or a
package.json present), and you have an npm package with a tag in your
deno.json (see the spec test that illustrates this).

This code path only actually executes when trying to choose an
appropriate package version from `node_modules/.deno`, so we should be
able to fix it by storing some extra data at install time.

The fix proposed here is to repurpose the `.initialized` file that we
store in `node_modules` to store the tags associated with a package.
Basically, if you have a version requirement with a tag (e.g.
`npm:chalk@latest`), when we set up the node_modules folder for that
package, we store the tag (`latest`) in `.initialized`. Then, when doing
BYONM resolution, if we have a version requirement with a tag, we read
that file and check if the tag is present.

The downside is that we do more work when setting up `node_modules`. We
_could_ do this only when BYONM is enabled, but that would have the
downside of needing to re-run `deno install` when you switch from auto
-> manual, though maybe that's not a big deal.
This commit is contained in:
Nathan Whitaker 2024-10-02 17:16:46 -07:00 committed by GitHub
parent 1e0c9b8c5b
commit 275418473e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 109 additions and 3 deletions

View File

@ -343,6 +343,14 @@ async fn sync_resolution_with_fs(
},
);
let packages_with_deprecation_warnings = Arc::new(Mutex::new(Vec::new()));
let mut package_tags: HashMap<&PackageNv, Vec<&str>> = HashMap::new();
for (package_req, package_nv) in snapshot.package_reqs() {
if let Some(tag) = package_req.version_req.tag() {
package_tags.entry(package_nv).or_default().push(tag);
}
}
for package in &package_partitions.packages {
if let Some(current_pkg) =
newest_packages_by_name.get_mut(&package.id.nv.name)
@ -357,11 +365,29 @@ async fn sync_resolution_with_fs(
let package_folder_name =
get_package_folder_id_folder_name(&package.get_package_cache_folder_id());
let folder_path = deno_local_registry_dir.join(&package_folder_name);
let tags = package_tags
.get(&package.id.nv)
.map(|tags| tags.join(","))
.unwrap_or_default();
enum PackageFolderState {
UpToDate,
Uninitialized,
TagsOutdated,
}
let initialized_file = folder_path.join(".initialized");
let package_state = std::fs::read_to_string(&initialized_file)
.map(|s| {
if s != tags {
PackageFolderState::TagsOutdated
} else {
PackageFolderState::UpToDate
}
})
.unwrap_or(PackageFolderState::Uninitialized);
if !cache
.cache_setting()
.should_use_for_npm_package(&package.id.nv.name)
|| !initialized_file.exists()
|| matches!(package_state, PackageFolderState::Uninitialized)
{
// cache bust the dep from the dep setup cache so the symlinks
// are forced to be recreated
@ -371,6 +397,7 @@ async fn sync_resolution_with_fs(
let bin_entries_to_setup = bin_entries.clone();
let packages_with_deprecation_warnings =
packages_with_deprecation_warnings.clone();
cache_futures.push(async move {
tarball_cache
.ensure_package(&package.id.nv, &package.dist)
@ -389,7 +416,7 @@ async fn sync_resolution_with_fs(
move || {
clone_dir_recursive(&cache_folder, &package_path)?;
// write out a file that indicates this folder has been initialized
fs::write(initialized_file, "")?;
fs::write(initialized_file, tags)?;
Ok::<_, AnyError>(())
}
@ -410,6 +437,8 @@ async fn sync_resolution_with_fs(
drop(pb_guard); // explicit for clarity
Ok::<_, AnyError>(())
});
} else if matches!(package_state, PackageFolderState::TagsOutdated) {
fs::write(initialized_file, tags)?;
}
let sub_node_modules = folder_path.join("node_modules");

View File

@ -253,7 +253,24 @@ impl<Fs: DenoResolverFs> ByonmNpmResolver<Fs> {
let Ok(version) = Version::parse_from_npm(version) else {
continue;
};
if req.version_req.matches(&version) {
if let Some(tag) = req.version_req.tag() {
let initialized_file =
node_modules_deno_dir.join(&entry.name).join(".initialized");
let Ok(contents) = self.fs.read_to_string_lossy(&initialized_file)
else {
continue;
};
let mut tags = contents.split(',').map(str::trim);
if tags.any(|t| t == tag) {
if let Some((best_version_version, _)) = &best_version {
if version > *best_version_version {
best_version = Some((version, entry.name));
}
} else {
best_version = Some((version, entry.name));
}
}
} else if req.version_req.matches(&version) {
if let Some((best_version_version, _)) = &best_version {
if version > *best_version_version {
best_version = Some((version, entry.name));

View File

@ -0,0 +1,44 @@
{
"tempDir": true,
"tests": {
"tag_with_byonm": {
"steps": [
{
"args": "install",
"output": "[WILDCARD]"
},
{
"args": "run -A main.ts",
"output": ""
}
]
},
"no_tag_then_tag": {
"steps": [
{
"args": "run -A replace-version-req.ts 1.0.0",
"output": ""
},
{
"args": "install",
"output": "[WILDCARD]"
},
{
"args": "run -A replace-version-req.ts latest",
"output": ""
},
{
"args": "run -A main.ts",
"output": "node_modules_out_of_date.out",
"exitCode": 1
},
{
"args": "install",
"output": "[WILDCARD]"
},
{ "args": "run -A main.ts", "output": "" }
]
}
}
}

View File

@ -0,0 +1,5 @@
{
"imports": {
"@denotest/esm-basic": "npm:@denotest/esm-basic@latest"
}
}

View File

@ -0,0 +1 @@
import { add } from "@denotest/esm-basic";

View File

@ -0,0 +1,2 @@
error: Could not find a matching package for 'npm:@denotest/esm-basic@latest' in the node_modules directory. Ensure you have all your JSR and npm dependencies listed in your deno.json or package.json, then run `deno install`. Alternatively, turn on auto-install by specifying `"nodeModulesDir": "auto"` in your deno.json file.
at [WILDCARD]main.ts:1:21

View File

@ -0,0 +1 @@
{}

View File

@ -0,0 +1,7 @@
const newReq = Deno.args[0]?.trim();
if (!newReq) {
throw new Error("Missing required argument");
}
const config = JSON.parse(Deno.readTextFileSync("deno.json"));
config.imports["@denotest/esm-basic"] = `npm:@denotest/esm-basic@${newReq}`;
Deno.writeTextFileSync("deno.json", JSON.stringify(config));