Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 7 additions & 0 deletions .changeset/heavy-dragons-start.md
Original file line number Diff line number Diff line change
@@ -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).
1 change: 1 addition & 0 deletions fetching/git-fetcher/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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:*",
Expand Down
10 changes: 8 additions & 2 deletions fetching/git-fetcher/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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({
Expand Down Expand Up @@ -85,7 +90,8 @@ function prefixGitArgs (): string[] {
return process.platform === 'win32' ? ['-c', 'core.longpaths=true'] : []
}

async function execGit (args: string[], opts?: object): Promise<void> {
async function execGit (args: string[], opts?: object): Promise<string> {
const fullArgs = prefixGitArgs().concat(args || [])
await execa('git', fullArgs, opts)
const { stdout } = await execa('git', fullArgs, opts)
return stdout
}
17 changes: 17 additions & 0 deletions fetching/git-fetcher/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,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/[email protected] 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 = temporaryDirectory()
const fetch = createGitFetcher({ ignoreScripts: true, rawConfig: {} }).git
Expand Down
3 changes: 3 additions & 0 deletions fetching/git-fetcher/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
{
"path": "../../exec/prepare-package"
},
{
"path": "../../packages/error"
},
{
"path": "../../packages/logger"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
},
},
Expand Down
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions resolving/git-resolver/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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:",
Expand Down
24 changes: 18 additions & 6 deletions resolving/git-resolver/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down Expand Up @@ -99,24 +100,35 @@ async function getRepoRefs (repo: string, ref: string | null): Promise<Record<st
}

async function resolveRef (repo: string, ref: string, range?: string): Promise<string> {
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
refs[`refs/tags/${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
Expand Down
14 changes: 12 additions & 2 deletions resolving/git-resolver/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,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({
Expand Down Expand Up @@ -434,6 +440,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({
Expand Down
3 changes: 3 additions & 0 deletions resolving/git-resolver/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
{
"path": "../../network/fetch"
},
{
"path": "../../packages/error"
},
{
"path": "../resolver-base"
}
Expand Down
Loading