diff --git a/.changeset/heavy-dragons-start.md b/.changeset/heavy-dragons-start.md new file mode 100644 index 0000000000..6bd5b20975 --- /dev/null +++ b/.changeset/heavy-dragons-start.md @@ -0,0 +1,7 @@ +--- +"pnpm": patch +"@pnpm/git-fetcher": patch +"@pnpm/git-resolver": patch +--- + +Always resolve git references to full commits and ensure `HEAD` points to the commit after checkout [#10310](https://github.com/pnpm/pnpm/pull/10310). diff --git a/fetching/git-fetcher/package.json b/fetching/git-fetcher/package.json index 1f110488ce..86a760b6b8 100644 --- a/fetching/git-fetcher/package.json +++ b/fetching/git-fetcher/package.json @@ -32,6 +32,7 @@ "compile": "tsc --build && pnpm run lint --fix" }, "dependencies": { + "@pnpm/error": "workspace:*", "@pnpm/fetcher-base": "workspace:*", "@pnpm/fs.packlist": "catalog:", "@pnpm/prepare-package": "workspace:*", diff --git a/fetching/git-fetcher/src/index.ts b/fetching/git-fetcher/src/index.ts index 89b994ef5b..7928362e30 100644 --- a/fetching/git-fetcher/src/index.ts +++ b/fetching/git-fetcher/src/index.ts @@ -6,6 +6,7 @@ import { packlist } from '@pnpm/fs.packlist' import { globalWarn } from '@pnpm/logger' import { preparePackage } from '@pnpm/prepare-package' import { addFilesFromDir } from '@pnpm/worker' +import { PnpmError } from '@pnpm/error' import rimraf from '@zkochan/rimraf' import execa from 'execa' import { URL } from 'url' @@ -31,6 +32,10 @@ export function createGitFetcher (createOpts: CreateGitFetcherOptions): { git: G await execGit(['clone', resolution.repo, tempLocation]) } await execGit(['checkout', resolution.commit], { cwd: tempLocation }) + const receivedCommit = await execGit(['rev-parse', 'HEAD'], { cwd: tempLocation }) + if (receivedCommit.trim() !== resolution.commit) { + throw new PnpmError('GIT_CHECKOUT_FAILED', `received commit ${receivedCommit.trim()} does not match expected value ${resolution.commit}`) + } let pkgDir: string try { const prepareResult = await preparePackage({ @@ -85,7 +90,8 @@ function prefixGitArgs (): string[] { return process.platform === 'win32' ? ['-c', 'core.longpaths=true'] : [] } -async function execGit (args: string[], opts?: object): Promise { +async function execGit (args: string[], opts?: object): Promise { const fullArgs = prefixGitArgs().concat(args || []) - await execa('git', fullArgs, opts) + const { stdout } = await execa('git', fullArgs, opts) + return stdout } diff --git a/fetching/git-fetcher/test/index.ts b/fetching/git-fetcher/test/index.ts index 223154ead0..baaf544e82 100644 --- a/fetching/git-fetcher/test/index.ts +++ b/fetching/git-fetcher/test/index.ts @@ -205,6 +205,23 @@ test('fail when preparing a git-hosted package', async () => { ).rejects.toThrow('Failed to prepare git-hosted package fetched from "https://github.com/pnpm-e2e/prepare-script-fails.git": @pnpm.e2e/prepare-script-fails@1.0.0 npm-install: `npm install`') }) +test('fail when preparing a git-hosted package with a partial commit', async () => { + const storeDir = temporaryDirectory() + const fetch = createGitFetcher({ + rawConfig: {}, + }).git + await expect( + fetch(createCafsStore(storeDir), + { + commit: 'deadbeef', + repo: 'https://github.com/pnpm-e2e/simple-pkg.git', + type: 'git', + }, { + filesIndexFile: path.join(storeDir, 'index.json'), + }) + ).rejects.toThrow(/received commit [0-9a-f]{40} does not match expected value deadbeef/) +}) + test('do not build the package when scripts are ignored', async () => { const storeDir = tempy.directory() const fetch = createGitFetcher({ ignoreScripts: true, rawConfig: {} }).git diff --git a/fetching/git-fetcher/tsconfig.json b/fetching/git-fetcher/tsconfig.json index de869c45b0..eb88410628 100644 --- a/fetching/git-fetcher/tsconfig.json +++ b/fetching/git-fetcher/tsconfig.json @@ -12,6 +12,9 @@ { "path": "../../exec/prepare-package" }, + { + "path": "../../packages/error" + }, { "path": "../../packages/logger" }, diff --git a/pkg-manager/plugin-commands-installation/test/saveCatalog.ts b/pkg-manager/plugin-commands-installation/test/saveCatalog.ts index 3feef7f4d2..1195862cc3 100644 --- a/pkg-manager/plugin-commands-installation/test/saveCatalog.ts +++ b/pkg-manager/plugin-commands-installation/test/saveCatalog.ts @@ -120,7 +120,7 @@ test('saveCatalogName works with different protocols', async () => { }, 'is-positive': { specifier: 'catalog:', - version: 'https://codeload.github.com/kevva/is-positive/tar.gz/97edff6', + version: 'https://codeload.github.com/kevva/is-positive/tar.gz/97edff6f525f192a3f83cea1944765f769ae2678', }, }, }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ce559cd007..d1295f67ac 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3152,6 +3152,9 @@ importers: fetching/git-fetcher: dependencies: + '@pnpm/error': + specifier: workspace:* + version: link:../../packages/error '@pnpm/fetcher-base': specifier: workspace:* version: link:../fetcher-base @@ -7207,6 +7210,9 @@ importers: resolving/git-resolver: dependencies: + '@pnpm/error': + specifier: workspace:* + version: link:../../packages/error '@pnpm/fetch': specifier: workspace:* version: link:../../network/fetch diff --git a/resolving/git-resolver/package.json b/resolving/git-resolver/package.json index d385dc649f..e968c0cbaa 100644 --- a/resolving/git-resolver/package.json +++ b/resolving/git-resolver/package.json @@ -34,6 +34,7 @@ "compile": "tsc --build && pnpm run lint --fix" }, "dependencies": { + "@pnpm/error": "workspace:*", "@pnpm/fetch": "workspace:*", "@pnpm/resolver-base": "workspace:*", "graceful-git": "catalog:", diff --git a/resolving/git-resolver/src/index.ts b/resolving/git-resolver/src/index.ts index 10b82b1bf1..f76bce5a9f 100644 --- a/resolving/git-resolver/src/index.ts +++ b/resolving/git-resolver/src/index.ts @@ -4,6 +4,7 @@ import semver from 'semver' import { parseBareSpecifier, type HostedPackageSpec } from './parseBareSpecifier.js' import { createGitHostedPkgId } from './createGitHostedPkgId.js' import { type AgentOptions } from '@pnpm/network.agent' +import { PnpmError } from '@pnpm/error' export { createGitHostedPkgId } @@ -99,16 +100,21 @@ async function getRepoRefs (repo: string, ref: string | null): Promise { - if (ref.match(/^[0-9a-f]{7,40}$/) != null) { + const committish = ref.match(/^[0-9a-f]{7,40}$/) !== null + if (committish && ref.length === 40) { return ref } - const refs = await getRepoRefs(repo, range ? null : ref) - return resolveRefFromRefs(refs, repo, ref, range) + const refs = await getRepoRefs(repo, (range ?? committish) ? null : ref) + const result = resolveRefFromRefs(refs, repo, ref, committish, range) + if (committish && !result.startsWith(ref)) { + throw new PnpmError('GIT_AMBIGUOUS_REF', `resolved commit ${result} from commit-ish reference ${ref}`) + } + return result } -function resolveRefFromRefs (refs: { [ref: string]: string }, repo: string, ref: string, range?: string): string { +function resolveRefFromRefs (refs: { [ref: string]: string }, repo: string, ref: string, committish: boolean, range?: string): string { if (!range) { - const commitId = + let commitId = refs[ref] || refs[`refs/${ref}`] || refs[`refs/tags/${ref}^{}`] || // prefer annotated tags @@ -116,7 +122,13 @@ function resolveRefFromRefs (refs: { [ref: string]: string }, repo: string, ref: refs[`refs/heads/${ref}`] if (!commitId) { - throw new Error(`Could not resolve ${ref} to a commit of ${repo}.`) + // check for a partial commit + const commits = committish ? Object.values(refs).filter((value: string) => value.startsWith(ref)) : [] + if (commits.length === 1) { + commitId = commits[0] + } else { + throw new Error(`Could not resolve ${ref} to a commit of ${repo}.`) + } } return commitId diff --git a/resolving/git-resolver/test/index.ts b/resolving/git-resolver/test/index.ts index 6a6e9749ac..2d86cd4045 100644 --- a/resolving/git-resolver/test/index.ts +++ b/resolving/git-resolver/test/index.ts @@ -60,15 +60,21 @@ test('resolveFromGit() with no commit, when main branch is not master', async () test('resolveFromGit() with partial commit', async () => { const resolveResult = await resolveFromGit({ bareSpecifier: 'zoli-forks/cmd-shim#a00a83a' }) expect(resolveResult).toStrictEqual({ - id: 'https://codeload.github.com/zoli-forks/cmd-shim/tar.gz/a00a83a', + id: 'https://codeload.github.com/zoli-forks/cmd-shim/tar.gz/a00a83a1593edb6e395d3ce41f2ef70edf7e2cf5', normalizedBareSpecifier: 'github:zoli-forks/cmd-shim#a00a83a', resolution: { - tarball: 'https://codeload.github.com/zoli-forks/cmd-shim/tar.gz/a00a83a', + tarball: 'https://codeload.github.com/zoli-forks/cmd-shim/tar.gz/a00a83a1593edb6e395d3ce41f2ef70edf7e2cf5', }, resolvedVia: 'git-repository', }) }) +test('resolveFromGit() with partial commit that is a branch name', async () => { + await expect( + resolveFromGit({ bareSpecifier: 'pnpm-e2e/simple-pkg#deadbeef' }) + ).rejects.toThrow(/resolved commit [0-9a-f]{40} from commit-ish reference deadbeef/) +}) + test('resolveFromGit() with branch', async () => { const resolveResult = await resolveFromGit({ bareSpecifier: 'zkochan/is-negative#canary' }) expect(resolveResult).toStrictEqual({ @@ -421,6 +427,10 @@ test('resolveFromGit() normalizes full url (alternative form 2)', async () => { // This test relies on implementation detail. // current implementation does not try git ls-remote --refs on bareSpecifier with full commit hash, this fake repo url will pass. test('resolveFromGit() private repo with commit hash', async () => { + // parseBareSpecifier will try to access the repository with --exit-code + git.mockImplementation(() => { + throw new Error('private') + }) mockFetchAsPrivate() const resolveResult = await resolveFromGit({ bareSpecifier: 'fake/private-repo#2fa0531ab04e300a24ef4fd7fb3a280eccb7ccc5' }) expect(resolveResult).toStrictEqual({ diff --git a/resolving/git-resolver/tsconfig.json b/resolving/git-resolver/tsconfig.json index aa41c8b8b2..6d7d381a6b 100644 --- a/resolving/git-resolver/tsconfig.json +++ b/resolving/git-resolver/tsconfig.json @@ -12,6 +12,9 @@ { "path": "../../network/fetch" }, + { + "path": "../../packages/error" + }, { "path": "../resolver-base" }