Skip to content

Commit

Permalink
fix(resolve-dependencies): not update git protocol dependency when a…
Browse files Browse the repository at this point in the history
…dd unrelated dependency (#7054)

close #7008
  • Loading branch information
await-ovo authored and nachoaldamav committed Sep 26, 2023
1 parent 025befe commit dd52be4
Show file tree
Hide file tree
Showing 14 changed files with 511 additions and 366 deletions.
9 changes: 9 additions & 0 deletions .changeset/serious-ghosts-taste.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@pnpm/resolve-dependencies": patch
"@pnpm/core": patch
"@pnpm/lockfile-utils": patch
"@pnpm/pick-fetcher": patch
"pnpm": patch
---

Don't update git-hosted dependencies when adding unrelated dependency [#7008](https://github.com/pnpm/pnpm/issues/7008).
6 changes: 6 additions & 0 deletions __fixtures__/with-git-protocol-dep/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name":"with-git-protocol-dep",
"dependencies": {
"is-negative": "github:kevva/is-negative#master"
}
}
26 changes: 26 additions & 0 deletions __fixtures__/with-git-protocol-dep/pnpm-lock.yaml

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

2 changes: 1 addition & 1 deletion fetching/pick-fetcher/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export function pickFetcher (fetcherByHostingType: Partial<Fetchers>, resolution
return fetch
}

function isGitHostedPkgUrl (url: string) {
export function isGitHostedPkgUrl (url: string) {
return (
url.startsWith('https://codeload.github.com/') ||
url.startsWith('https://bitbucket.org/') ||
Expand Down
1 change: 1 addition & 0 deletions lockfile/lockfile-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"dependencies": {
"@pnpm/dependency-path": "workspace:*",
"@pnpm/lockfile-types": "workspace:*",
"@pnpm/pick-fetcher": "workspace:*",
"@pnpm/resolver-base": "workspace:*",
"@pnpm/types": "workspace:*",
"get-npm-tarball-url": "^2.0.3",
Expand Down
4 changes: 3 additions & 1 deletion lockfile/lockfile-utils/src/pkgSnapshotToResolution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { type Resolution } from '@pnpm/resolver-base'
import { type Registries } from '@pnpm/types'
import * as dp from '@pnpm/dependency-path'
import getNpmTarballUrl from 'get-npm-tarball-url'
import { isGitHostedPkgUrl } from '@pnpm/pick-fetcher'
import { nameVerFromPkgSnapshot } from './nameVerFromPkgSnapshot'

export function pkgSnapshotToResolution (
Expand All @@ -13,7 +14,8 @@ export function pkgSnapshotToResolution (
): Resolution {
if (
Boolean((pkgSnapshot.resolution as TarballResolution).type) ||
(pkgSnapshot.resolution as TarballResolution).tarball?.startsWith('file:')
(pkgSnapshot.resolution as TarballResolution).tarball?.startsWith('file:') ||
isGitHostedPkgUrl((pkgSnapshot.resolution as TarballResolution).tarball ?? '')
) {
return pkgSnapshot.resolution as Resolution
}
Expand Down
3 changes: 3 additions & 0 deletions lockfile/lockfile-utils/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
"../../__typings__/**/*.d.ts"
],
"references": [
{
"path": "../../fetching/pick-fetcher"
},
{
"path": "../../packages/dependency-path"
},
Expand Down
36 changes: 36 additions & 0 deletions pkg-manager/core/test/install/fromRepo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,17 @@ import {
addDependenciesToPackage,
install,
} from '@pnpm/core'
import { fixtures } from '@pnpm/test-fixtures'
import { assertProject } from '@pnpm/assert-project'
import rimraf from '@zkochan/rimraf'
import { isCI } from 'ci-info'
import exists from 'path-exists'
import sinon from 'sinon'
import { testDefaults } from '../utils'

const f = fixtures(__dirname)
const withGitProtocolDepFixture = f.find('with-git-protocol-dep')

test('from a github repo', async () => {
const project = prepareEmpty()

Expand Down Expand Up @@ -278,3 +283,34 @@ test('re-adding a git repo with a different tag', async () => {
}
)
})

test('should not update when adding unrelated dependency', async () => {
process.chdir(withGitProtocolDepFixture)
fs.rmSync('./node_modules', {
recursive: true,
force: true,
})
let manifest = JSON.parse(fs.readFileSync('./package.json', 'utf8'))
await install(manifest, await testDefaults({ preferFrozenLockfile: false, dir: withGitProtocolDepFixture, lockfileDir: withGitProtocolDepFixture }))

expect(fs.existsSync('./node_modules/.pnpm/github.com+kevva+is-negative@219c424611ff4a2af15f7deeff4f93c62558c43d')).toBe(true)

manifest = await addDependenciesToPackage(manifest, ['is-number'], await testDefaults({ preferFrozenLockfile: false }))

expect(manifest.dependencies).toHaveProperty('is-number')
expect(manifest.dependencies['is-negative']).toBe('github:kevva/is-negative#master')

const project = assertProject(withGitProtocolDepFixture)
await project.has('is-number')
expect(fs.existsSync('./node_modules/.pnpm/github.com+kevva+is-negative@219c424611ff4a2af15f7deeff4f93c62558c43d')).toBe(true)
expect((await project.readLockfile()).dependencies).toEqual({
'is-negative': {
specifier: 'github:kevva/is-negative#master',
version: 'github.com/kevva/is-negative/219c424611ff4a2af15f7deeff4f93c62558c43d',
},
'is-number': {
specifier: '^7.0.0',
version: '7.0.0',
},
})
})
2 changes: 1 addition & 1 deletion pkg-manager/core/test/install/updatingPkgJson.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ test("don't override existing spec in package.json on named installation", async
expect(manifest.dependencies).toStrictEqual({
'is-negative': '^1.0.1',
'is-positive': '^2.0.0',
sec: 'github:sindresorhus/sec#main',
sec: 'sindresorhus/sec#main',
})
})

Expand Down
1 change: 1 addition & 0 deletions pkg-manager/resolve-dependencies/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"@pnpm/manifest-utils": "workspace:*",
"@pnpm/npm-resolver": "workspace:*",
"@pnpm/pick-registry-for-package": "workspace:*",
"@pnpm/pick-fetcher": "workspace:*",
"@pnpm/prune-lockfile": "workspace:*",
"@pnpm/read-package-json": "workspace:*",
"@pnpm/resolver-base": "workspace:*",
Expand Down
3 changes: 3 additions & 0 deletions pkg-manager/resolve-dependencies/src/resolveDependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,9 @@ function referenceSatisfiesWantedSpec (
return false
}
const { version } = nameVerFromPkgSnapshot(depPath, pkgSnapshot)
if (!semver.validRange(wantedDep.pref) && Object.values(opts.lockfile.importers).filter(importer => importer.specifiers[wantedDep.alias] === wantedDep.pref).length) {
return true
}
return semver.satisfies(version, wantedDep.pref, true)
}

Expand Down
13 changes: 12 additions & 1 deletion pkg-manager/resolve-dependencies/src/updateProjectManifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
} from '@pnpm/manifest-utils'
import versionSelectorType from 'version-selector-type'
import semver from 'semver'
import { isGitHostedPkgUrl } from '@pnpm/pick-fetcher'
import { type TarballResolution } from '@pnpm/resolver-base'
import { type ResolvedDirectDependency } from './resolveDependencyTree'
import { type ImporterToResolve } from '.'

Expand All @@ -25,7 +27,16 @@ export async function updateProjectManifest (
.filter((rdd, index) => importer.wantedDependencies[index]?.updateSpec)
.map((rdd, index) => {
const wantedDep = importer.wantedDependencies[index]!
return resolvedDirectDepToSpecObject({ ...rdd, isNew: wantedDep.isNew, specRaw: wantedDep.raw, preserveNonSemverVersionSpec: wantedDep.preserveNonSemverVersionSpec }, importer, {
return resolvedDirectDepToSpecObject({
...rdd,
isNew:
wantedDep.isNew,
specRaw: wantedDep.raw,
preserveNonSemverVersionSpec: wantedDep.preserveNonSemverVersionSpec,
// For git-protocol dependencies that are already installed locally, there is no normalizedPref unless do force resolve,
// so we use pref in wantedDependency here.
normalizedPref: rdd.normalizedPref ?? (isGitHostedPkgUrl((rdd.resolution as TarballResolution).tarball ?? '') ? wantedDep.pref : undefined),
}, importer, {
nodeExecPath: wantedDep.nodeExecPath,
pinnedVersion: wantedDep.pinnedVersion ?? importer.pinnedVersion ?? 'major',
preserveWorkspaceProtocol: opts.preserveWorkspaceProtocol,
Expand Down
3 changes: 3 additions & 0 deletions pkg-manager/resolve-dependencies/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
{
"path": "../../config/pick-registry-for-package"
},
{
"path": "../../fetching/pick-fetcher"
},
{
"path": "../../lockfile/lockfile-types"
},
Expand Down

0 comments on commit dd52be4

Please sign in to comment.