diff --git a/.changeset/git-tarball-path-in-lockfile.md b/.changeset/git-tarball-path-in-lockfile.md new file mode 100644 index 0000000000..fa20450747 --- /dev/null +++ b/.changeset/git-tarball-path-in-lockfile.md @@ -0,0 +1,6 @@ +--- +"@pnpm/lockfile.utils": patch +"pnpm": patch +--- + +Git dependencies that point to a subdirectory of a repository (`repo#commit&path:/sub/dir`) keep their `path` in the lockfile again. Since the integrity of git-hosted tarballs started being pinned in the lockfile, any install that actually downloaded the tarball rebuilt the lockfile resolution as `{ integrity, tarball, gitHosted }` and dropped the `path` field, while installs served from the store kept it — so the field disappeared seemingly at random. Without `path`, later installs from that lockfile silently unpacked the repository root instead of the subdirectory [#12304](https://github.com/pnpm/pnpm/issues/12304). diff --git a/lockfile/utils/src/toLockfileResolution.ts b/lockfile/utils/src/toLockfileResolution.ts index 5363e8aea5..83849a9d04 100644 --- a/lockfile/utils/src/toLockfileResolution.ts +++ b/lockfile/utils/src/toLockfileResolution.ts @@ -26,49 +26,46 @@ export function toLockfileResolution ( // legacy lockfiles read by callers that don't enrich the field). const gitHosted = (resolution as TarballResolution).gitHosted === true || isGitHostedTarballUrl(tarball) - if (lockfileIncludeTarballUrl) { - return preservingGitHosted({ - integrity: resolution['integrity'], - tarball, - }, gitHosted) - } - // Tarball URLs that cannot be reconstructed from the package name, version, - // and registry must always stay in the lockfile, otherwise the package can - // no longer be re-fetched. This covers local `file:` tarballs and tarballs - // served by git providers (GitHub, GitLab, Bitbucket). - if (tarball.startsWith('file:') || gitHosted) { - return preservingGitHosted({ - integrity: resolution['integrity'], - tarball, - }, gitHosted) - } - // Sometimes packages are hosted under non-standard tarball URLs. - // For instance, when they are hosted on npm Enterprise. See https://github.com/pnpm/pnpm/issues/867 - // Or in other weird cases, like https://github.com/pnpm/pnpm/issues/1072. - // Even when the user explicitly sets `lockfileIncludeTarballUrl: false`, we - // must preserve such URLs — otherwise the package cannot be re-fetched on a - // frozen-lockfile install (e.g. GitHub Packages tarballs at - // `https://npm.pkg.github.com/download////`). - // `lockfileIncludeTarballUrl` only controls whether URLs that *can* be - // derived from name+version+registry are written. - const expectedTarball = getNpmTarballUrl(pkg.name, pkg.version, { registry }) - const actualTarball = tarball.replaceAll('%2f', '/') - if (removeProtocol(expectedTarball) !== removeProtocol(actualTarball)) { - return preservingGitHosted({ - integrity: resolution['integrity'], - tarball, - }, gitHosted) + // A standard registry tarball whose URL can be rebuilt from the package name, + // version, and registry is written as just `{ integrity }` — pnpm derives the + // URL on demand. Every other tarball must keep its URL or it can no longer be + // re-fetched on a frozen-lockfile install: `file:` tarballs, git-provider + // tarballs (GitHub/GitLab/Bitbucket), and non-standard registry URLs such as + // npm Enterprise (https://github.com/pnpm/pnpm/issues/867) or GitHub Packages + // `/download/` URLs. `lockfileIncludeTarballUrl` forces the URL to be kept. + if ( + !lockfileIncludeTarballUrl && + !gitHosted && + !tarball.startsWith('file:') && + isCanonicalRegistryTarballUrl(tarball, pkg, registry) + ) { + return { integrity: resolution['integrity'] } } + // The kept-URL form carries the `gitHosted` marker and the subdirectory `path` + // (`repo#commit&path:/sub/dir`, only ever set on git-hosted tarballs) so a + // git-hosted monorepo tarball still unpacks the right subfolder. + // See https://github.com/pnpm/pnpm/issues/12304. + const { path } = resolution as TarballResolution return { integrity: resolution['integrity'], + tarball, + ...(gitHosted ? { gitHosted: true } : {}), + ...(path == null ? {} : { path }), } } -function preservingGitHosted ( - resolution: T, - gitHosted: boolean -): T & { gitHosted?: boolean } { - return gitHosted ? { ...resolution, gitHosted: true } : resolution +// Whether `tarball` is the canonical npm registry URL derived from the package +// name, version, and registry — i.e. it can be dropped from the lockfile and +// rebuilt on demand. The `%2f` unescape matches the URLs npm produces for +// scoped packages. +function isCanonicalRegistryTarballUrl ( + tarball: string, + pkg: { name: string, version: string }, + registry: string +): boolean { + const expectedTarball = getNpmTarballUrl(pkg.name, pkg.version, { registry }) + const actualTarball = tarball.replaceAll('%2f', '/') + return removeProtocol(expectedTarball) === removeProtocol(actualTarball) } // Inlined to avoid pulling @pnpm/fetching.pick-fetcher into the lockfile-utils diff --git a/lockfile/utils/test/toLockfileResolution.ts b/lockfile/utils/test/toLockfileResolution.ts index 03f0faf0e3..07e25b497e 100644 --- a/lockfile/utils/test/toLockfileResolution.ts +++ b/lockfile/utils/test/toLockfileResolution.ts @@ -105,6 +105,37 @@ test('keeps git-hosted tarballs when lockfileIncludeTarballUrl is false', () => }) }) +test('keeps the path of a git-hosted tarball pointing to a subdirectory', () => { + // The path selects the subdirectory to extract from a monorepo tarball + // (`repo#commit&path:/sub/dir`). Dropping it makes later installs silently + // unpack the repository root. See https://github.com/pnpm/pnpm/issues/12304. + expect(toLockfileResolution( + { name: 'foo', version: '1.0.0' }, + { integrity: 'sha512-AAAA', tarball: 'https://codeload.github.com/foo/bar/tar.gz/abcdef', gitHosted: true, path: '/packages/foo' }, + REGISTRY, + false + )).toEqual({ + integrity: 'sha512-AAAA', + tarball: 'https://codeload.github.com/foo/bar/tar.gz/abcdef', + gitHosted: true, + path: '/packages/foo', + }) +}) + +test('keeps the path of a git-hosted tarball when lockfileIncludeTarballUrl is true', () => { + expect(toLockfileResolution( + { name: 'foo', version: '1.0.0' }, + { integrity: 'sha512-AAAA', tarball: 'https://codeload.github.com/foo/bar/tar.gz/abcdef', gitHosted: true, path: '/packages/foo' }, + REGISTRY, + true + )).toEqual({ + integrity: 'sha512-AAAA', + tarball: 'https://codeload.github.com/foo/bar/tar.gz/abcdef', + gitHosted: true, + path: '/packages/foo', + }) +}) + test('records gitHosted on the lockfile entry when set on the resolution', () => { expect(toLockfileResolution( { name: 'foo', version: '1.0.0' }, diff --git a/pacquet/crates/lockfile/src/resolution.rs b/pacquet/crates/lockfile/src/resolution.rs index 39c8a89b7c..19aa4bdf6d 100644 --- a/pacquet/crates/lockfile/src/resolution.rs +++ b/pacquet/crates/lockfile/src/resolution.rs @@ -299,24 +299,31 @@ impl LockfileResolution { let git_hosted = tarball.git_hosted == Some(true) || is_git_hosted_tarball_url(&tarball.tarball); - let keep_url = || { - LockfileResolution::Tarball(TarballResolution { - tarball: tarball.tarball.clone(), - integrity: Some(integrity.clone()), - git_hosted: git_hosted.then_some(true), - path: tarball.path.clone(), - }) - }; - - if include_tarball_url || tarball.tarball.starts_with("file:") || git_hosted { - return keep_url(); + // A standard registry tarball whose URL can be rebuilt from name+version+ + // registry is written as just `{integrity}` — pnpm derives the URL on + // demand. Every other tarball must keep its URL or it can no longer be + // re-fetched on a frozen-lockfile install: `file:` tarballs, git-provider + // tarballs, and non-standard registry URLs (npm Enterprise, GitHub Packages + // `/download/` URLs). `include_tarball_url` forces the URL to be kept. + if !include_tarball_url + && !git_hosted + && !tarball.tarball.starts_with("file:") + && is_canonical_registry_tarball_url(&tarball.tarball, name, version, registry) + { + return LockfileResolution::Registry(RegistryResolution { + integrity: integrity.clone(), + }); } - let expected = npm_tarball_url(name, version, registry); - let actual = tarball.tarball.replace("%2f", "/"); - if remove_protocol(&expected) != remove_protocol(&actual) { - return keep_url(); - } - LockfileResolution::Registry(RegistryResolution { integrity: integrity.clone() }) + // The kept-URL form carries the `git_hosted` marker and the subdirectory + // `path` (`repo#commit&path:/sub/dir`, only ever set on git-hosted tarballs) + // so a git-hosted monorepo tarball still unpacks the right subfolder. + // See . + LockfileResolution::Tarball(TarballResolution { + tarball: tarball.tarball.clone(), + integrity: Some(integrity.clone()), + git_hosted: git_hosted.then_some(true), + path: tarball.path.clone(), + }) } } @@ -335,6 +342,21 @@ pub fn npm_tarball_url(name: &str, version: &str, registry: &str) -> String { format!("{registry}{name}/-/{scopeless}-{version}.tgz") } +/// Whether `tarball` is the canonical npm registry URL derived from `name`, +/// `version`, and `registry` — i.e. it can be dropped from the lockfile and +/// rebuilt on demand. The `%2f` unescape matches the URLs npm produces for +/// scoped packages. +fn is_canonical_registry_tarball_url( + tarball: &str, + name: &str, + version: &str, + registry: &str, +) -> bool { + let expected = npm_tarball_url(name, version, registry); + let actual = tarball.replace("%2f", "/"); + remove_protocol(&expected) == remove_protocol(&actual) +} + /// Default-vs-scope routing for an npm package. Mirrors pnpm's /// [`pickRegistryForPackage`](https://github.com/pnpm/pnpm/blob/main/config/pick-registry-for-package/src/index.ts). /// diff --git a/pacquet/crates/lockfile/src/resolution/tests.rs b/pacquet/crates/lockfile/src/resolution/tests.rs index 420ff106e5..5da1664f9b 100644 --- a/pacquet/crates/lockfile/src/resolution/tests.rs +++ b/pacquet/crates/lockfile/src/resolution/tests.rs @@ -666,3 +666,53 @@ fn libc_matches_truth_table() { assert!(!libc_matches(None, Some("uclibc"))); assert!(!libc_matches(Some("glibc"), Some("uclibc"))); } + +const SHA512: &str = "sha512-gf6ZldcfCDyNXPRiW3lQjEP1Z9rrUM/4Cn7BZbv3SdTA82zxWRP8OmLwvGR974uuENhGCFgFdN11z3n1Ofpprg=="; + +/// A reconstructible registry tarball URL is dropped, leaving only the +/// integrity, so the path-preserving cases below are not just returning the +/// input unchanged. +#[test] +fn to_lockfile_form_drops_reconstructible_registry_tarball() { + let resolution = LockfileResolution::Tarball(TarballResolution { + tarball: "https://registry.npmjs.org/foo/-/foo-1.0.0.tgz".to_string(), + integrity: Some(integrity(SHA512)), + git_hosted: None, + path: None, + }); + let actual = resolution.to_lockfile_form("foo", "1.0.0", "https://registry.npmjs.org/", false); + assert_eq!( + actual, + LockfileResolution::Registry(RegistryResolution { integrity: integrity(SHA512) }), + ); +} + +/// The `path` selects the subdirectory to extract from a monorepo tarball +/// (`repo#commit&path:/sub/dir`). Dropping it makes later installs silently +/// unpack the repository root. See +/// . +#[test] +fn to_lockfile_form_keeps_git_hosted_subdirectory_path() { + let resolution = LockfileResolution::Tarball(TarballResolution { + tarball: "https://codeload.github.com/foo/bar/tar.gz/abc1234".to_string(), + integrity: Some(integrity(SHA512)), + git_hosted: Some(true), + path: Some("/packages/foo".to_string()), + }); + let actual = resolution.to_lockfile_form("foo", "1.0.0", "https://registry.npmjs.org/", false); + assert_eq!(actual, resolution); +} + +/// `include_tarball_url` takes the same kept-URL branch, so it must keep +/// `path` too. +#[test] +fn to_lockfile_form_keeps_git_hosted_subdirectory_path_when_including_tarball_url() { + let resolution = LockfileResolution::Tarball(TarballResolution { + tarball: "https://codeload.github.com/foo/bar/tar.gz/abc1234".to_string(), + integrity: Some(integrity(SHA512)), + git_hosted: Some(true), + path: Some("/packages/foo".to_string()), + }); + let actual = resolution.to_lockfile_form("foo", "1.0.0", "https://registry.npmjs.org/", true); + assert_eq!(actual, resolution); +}