Skip to content

Commit dadf8c4

Browse files
authored
chore(pm): clarify comments after Corepack shim removal (#2706)
Update remaining Corepack-related source comments to reflect the removal of Corepack integration. Related: #2391
1 parent a72b389 commit dadf8c4

3 files changed

Lines changed: 26 additions & 36 deletions

File tree

crates/vp_error/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,9 @@ impl Error {
121121

122122
/// Details of a failed `packageManager` integrity check.
123123
///
124-
/// `basis` names the artifact that vp hashed. Corepack hashes the extracted CLI
125-
/// for Yarn 2+, and the npm tarball for every other package manager. A message
126-
/// that says only "hash mismatch" reads like a corrupt download.
124+
/// `basis` names the artifact that vp hashed: the extracted CLI for Yarn 2+ or
125+
/// the npm package tarball for npm, pnpm, and Yarn Classic. Naming the artifact
126+
/// distinguishes a wrong hash basis from a corrupt download.
127127
#[derive(Error, Debug)]
128128
#[error(
129129
"Hash mismatch for {name}@{version}: expected {expected}, got {actual}\n\

crates/vp_global_cli/src/commands/env/setup.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -575,9 +575,9 @@ pub(crate) async fn cleanup_legacy_windows_shim(bin_dir: &vt_path::AbsolutePath,
575575
let cmd_path = bin_dir.join(format!("{tool}.cmd"));
576576
let _ = tokio::fs::remove_file(&cmd_path).await;
577577

578-
// Remove .ps1 launchers (corepack's cmd-shim writes them; PowerShell
579-
// resolves `<tool>.ps1` ahead of `<tool>.exe`, so a leftover would shadow
580-
// the trampoline). Vite+ never creates per-tool .ps1 files in bin.
578+
// Remove legacy .ps1 launchers because PowerShell resolves them ahead of
579+
// `<tool>.exe`, which would shadow the trampoline. Vite+ does not create
580+
// per-tool .ps1 files in bin.
581581
let ps1_path = bin_dir.join(format!("{tool}.ps1"));
582582
let _ = tokio::fs::remove_file(&ps1_path).await;
583583

crates/vp_pm_cli/src/package_manager.rs

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,8 @@ impl PackageManagerType {
107107
}
108108
}
109109

110-
/// Whether a Corepack pin for this version covers the extracted CLI binary
111-
/// and not the npm tarball.
110+
/// Whether the `packageManager` integrity hash for this version covers the
111+
/// extracted CLI binary rather than the npm package tarball.
112112
#[must_use]
113113
pub fn uses_cli_binary_hash(self, version: &str) -> bool {
114114
Version::parse(version).is_ok_and(|version| self.hashes_cli_binary_of(&version))
@@ -138,17 +138,15 @@ const VERIFIED_PIN_RECORD: &str = ".verified-pin";
138138

139139
/// Path of the Yarn CLI inside `@yarnpkg/cli-dist`, relative to the package root.
140140
///
141-
/// Corepack hashes this file to pin Yarn 2+. Three places must name the same
142-
/// path: the download, the cached-CLI check, and the error message.
141+
/// Yarn 2+ integrity pins cover this file. The download, cached-CLI check, and
142+
/// error message must use the same path.
143143
const YARN_CLI_ENTRY: &str = "bin/yarn.js";
144144

145145
/// Whether a Yarn version is Berry (Yarn 2 and later).
146146
///
147-
/// Corepack splits Yarn at 2.0.0. It matches that range with
148-
/// `satisfiesWithPrereleases`, which drops the prerelease tag first. Every 2.x
149-
/// prerelease is therefore a Berry version, so this function compares the major
150-
/// number alone. `VersionReq(">=2.0.0")` excludes `4.0.0-rc.53` and sends it to
151-
/// the Yarn Classic package, which never published that version.
147+
/// Compare the major version so Yarn 2+ prereleases also use Berry behavior
148+
/// and the `@yarnpkg/cli-dist` package. `VersionReq(">=2.0.0")` excludes
149+
/// `4.0.0-rc.53`, which the Yarn Classic package never published.
152150
pub(crate) fn is_yarn_berry(version: &Version) -> bool {
153151
version.major >= 2
154152
}
@@ -1144,8 +1142,7 @@ pub async fn download_package_manager(
11441142

11451143
let is_modern_yarn = package_manager_type.hashes_cli_binary_of(&parsed_version);
11461144
let mut package_name: Str = package_manager_type.to_string().into();
1147-
// handle yarn >= 2.0.0 to use `@yarnpkg/cli-dist` as package name
1148-
// @see https://github.com/nodejs/corepack/blob/main/config.json#L135
1145+
// Yarn 2+ releases, including prereleases, use `@yarnpkg/cli-dist`.
11491146
if is_modern_yarn {
11501147
package_name = "@yarnpkg/cli-dist".into();
11511148
}
@@ -1198,8 +1195,8 @@ pub async fn download_package_manager(
11981195
let target_dir_tmp = tmp_dir.path().to_path_buf();
11991196

12001197
let download_message = format!("Downloading {package_manager_type} v{version}...");
1201-
// A Corepack Yarn 2+ pin covers only the CLI. The rest of the archive stays
1202-
// unauthenticated, so vp never writes it to disk.
1198+
// A Yarn 2+ `packageManager` hash covers only the CLI. The rest of the archive
1199+
// is not authenticated by that hash, so vp never writes it to disk.
12031200
let archive_file = is_modern_yarn.then(|| PathBuf::from(format!("package/{YARN_CLI_ENTRY}")));
12041201
download_and_extract_tgz_with_hash(
12051202
&tgz_url,
@@ -1283,21 +1280,16 @@ pub async fn download_package_manager(
12831280

12841281
/// Verify a cached CLI against a pin that covers it.
12851282
///
1286-
/// vp hashes the CLI once, when it installs the package manager, and records
1287-
/// the pin it verified. A later command compares its own pin against that
1288-
/// record, so it never hashes the multi-megabyte CLI again.
1283+
/// vp records the verified pin when installing Yarn 2+. A matching record avoids
1284+
/// rehashing the cached CLI. If the record is missing or the pin changes, vp
1285+
/// hashes the CLI and updates the record.
12891286
///
1290-
/// The record is missing after an install by an older vp, and it differs after
1291-
/// the project changes its pin. Both cases hash the CLI once more.
1287+
/// A matching record does not detect a CLI modified after installation. The
1288+
/// record sits beside the file, so a writer that can replace one can replace
1289+
/// the other. Write access to the managed install directory is the trust boundary.
12921290
///
1293-
/// vp does not detect a CLI that changed on disk after the install, which is
1294-
/// the guarantee Corepack gives its own cache. The record sits beside the file
1295-
/// it describes, so a writer that can replace one can replace the other. The
1296-
/// trust boundary is write access to `$VP_HOME`.
1297-
///
1298-
/// Only a Corepack Yarn 2+ pin covers a file that vp keeps. Every other pin
1299-
/// names a tarball that vp deletes after it extracts it, so this function
1300-
/// accepts those without a check.
1291+
/// Only Yarn 2+ pins cover a retained CLI file. Other pins cover tarballs that
1292+
/// vp discards after extraction and are not rechecked here.
13011293
async fn verify_cached_cli_hash(
13021294
package_manager_type: PackageManagerType,
13031295
target_dir: &AbsolutePath,
@@ -2455,9 +2447,7 @@ mod tests {
24552447
assert!(!PackageManagerType::Pnpm.uses_cli_binary_hash("10.0.0"));
24562448
assert!(!PackageManagerType::Yarn.uses_cli_binary_hash("latest"));
24572449

2458-
// Corepack drops the prerelease tag before it matches its `>=2.0.0`
2459-
// range. A 2.x prerelease pin is therefore a Berry pin there too.
2460-
// `corepack use yarn@4.0.0-rc.53` writes a hash of `bin/yarn.js`.
2450+
// Yarn 2+ prereleases use the same CLI hash basis as stable Berry releases.
24612451
assert!(PackageManagerType::Yarn.uses_cli_binary_hash("2.0.0-rc.1"));
24622452
assert!(PackageManagerType::Yarn.uses_cli_binary_hash("4.0.0-rc.53"));
24632453
}
@@ -3572,7 +3562,7 @@ mod tests {
35723562
mismatch.actual,
35733563
"sha512.ca75da26c00327d26267ce33536e5790f18ebd53266796fbb664d2a4a5116308042dd8ee7003b276a20eace7d3c5561c3577bdd71bcb67071187af124779620a"
35743564
);
3575-
// Yarn Classic ships the CLI inside the tarball that Corepack pins.
3565+
// Yarn Classic integrity pins cover the tarball, not the extracted CLI.
35763566
assert_eq!(mismatch.basis, "the npm package tarball");
35773567
}
35783568
other => panic!("Expected PackageManagerHashMismatch error, got {other:?}"),

0 commit comments

Comments
 (0)