Skip to content

Commit

Permalink
fix: use lexigraphical sorting in places where it matters (#5587)
Browse files Browse the repository at this point in the history
  • Loading branch information
zkochan committed Nov 4, 2022
1 parent e19356c commit 2e97907
Show file tree
Hide file tree
Showing 10 changed files with 37 additions and 7 deletions.
7 changes: 7 additions & 0 deletions .changeset/smart-coats-jam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@pnpm/find-workspace-packages": patch
"find-packages": patch
"@pnpm/hoist": patch
---

Use deterministic sorting.
1 change: 1 addition & 0 deletions packages/find-packages/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"dependencies": {
"@pnpm/read-project-manifest": "workspace:*",
"@pnpm/types": "workspace:*",
"@pnpm/util.lex-comparator": "1.0.0",
"fast-glob": "^3.2.12",
"p-filter": "^2.1.0"
},
Expand Down
3 changes: 2 additions & 1 deletion packages/find-packages/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import path from 'path'
import { readExactProjectManifest } from '@pnpm/read-project-manifest'
import { ProjectManifest } from '@pnpm/types'
import { lexCompare } from '@pnpm/util.lex-comparator'
import fastGlob from 'fast-glob'
import pFilter from 'p-filter'

Expand Down Expand Up @@ -48,7 +49,7 @@ export async function findPackages (root: string, opts?: Options): Promise<Proje
paths
.map(manifestPath => path.join(root, manifestPath))
.sort((path1, path2) =>
path.dirname(path1).localeCompare(path.dirname(path2))
lexCompare(path.dirname(path1), path.dirname(path2))
)
),
async manifestPath => {
Expand Down
1 change: 1 addition & 0 deletions packages/find-workspace-packages/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"@pnpm/cli-utils": "workspace:*",
"@pnpm/constants": "workspace:*",
"@pnpm/types": "workspace:*",
"@pnpm/util.lex-comparator": "1.0.0",
"find-packages": "workspace:*",
"read-yaml-file": "^2.1.0"
},
Expand Down
3 changes: 2 additions & 1 deletion packages/find-workspace-packages/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import path from 'path'
import { packageIsInstallable } from '@pnpm/cli-utils'
import { WORKSPACE_MANIFEST_FILENAME } from '@pnpm/constants'
import { Project } from '@pnpm/types'
import { lexCompare } from '@pnpm/util.lex-comparator'
import { findPackages } from 'find-packages'
import readYamlFile from 'read-yaml-file'

Expand Down Expand Up @@ -37,7 +38,7 @@ export async function findWorkspacePackagesNoCheck (workspaceRoot: string, opts?
includeRoot: true,
patterns,
})
pkgs.sort((pkg1: { dir: string }, pkg2: { dir: string }) => pkg1.dir.localeCompare(pkg2.dir))
pkgs.sort((pkg1: { dir: string }, pkg2: { dir: string }) => lexCompare(pkg1.dir, pkg2.dir))
return pkgs
}

Expand Down
1 change: 1 addition & 0 deletions packages/hoist/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"@pnpm/matcher": "workspace:*",
"@pnpm/symlink-dependency": "workspace:*",
"@pnpm/types": "workspace:*",
"@pnpm/util.lex-comparator": "1.0.0",
"dependency-path": "workspace:*",
"ramda": "npm:@pnpm/ramda@0.28.1"
},
Expand Down
3 changes: 2 additions & 1 deletion packages/hoist/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { logger } from '@pnpm/logger'
import { createMatcher } from '@pnpm/matcher'
import { symlinkDependency } from '@pnpm/symlink-dependency'
import { HoistedDependencies } from '@pnpm/types'
import { lexCompare } from '@pnpm/util.lex-comparator'
import * as dp from 'dependency-path'

const hoistLogger = logger('hoist')
Expand Down Expand Up @@ -171,7 +172,7 @@ async function hoistGraph (
// sort by depth and then alphabetically
.sort((a, b) => {
const depthDiff = a.depth - b.depth
return depthDiff === 0 ? a.depPath.localeCompare(b.depPath) : depthDiff
return depthDiff === 0 ? lexCompare(a.depPath, b.depPath) : depthDiff
})
// build the alias map and the id map
.forEach((depNode) => {
Expand Down
1 change: 1 addition & 0 deletions packages/lockfile-file/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
"@pnpm/lockfile-types": "workspace:*",
"@pnpm/merge-lockfile-changes": "workspace:*",
"@pnpm/types": "workspace:*",
"@pnpm/util.lex-comparator": "1.0.0",
"@zkochan/rimraf": "^2.1.2",
"comver-to-semver": "^1.0.0",
"dependency-path": "workspace:*",
Expand Down
6 changes: 2 additions & 4 deletions packages/lockfile-file/src/sortLockfileKeys.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { lexCompare } from '@pnpm/util.lex-comparator'
import sortKeys from 'sort-keys'
import { LockfileFile } from './write'

Expand Down Expand Up @@ -53,10 +54,7 @@ function compareWithPriority (priority: Record<string, number>, left: string, ri
if (leftPriority && rightPriority) return leftPriority - rightPriority
if (leftPriority) return -1
if (rightPriority) return 1
// We want deterministic sorting, so we can't use .localCompare here.
// comparing strings with < and > will produce the same result on each machine.
// An alternative solution could be to use a specific culture for compare, using Intl.Collator
return left < right ? -1 : (left > right ? 1 : 0)
return lexCompare(left, right)
}

export function sortLockfileKeys (lockfile: LockfileFile) {
Expand Down
18 changes: 18 additions & 0 deletions pnpm-lock.yaml

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

0 comments on commit 2e97907

Please sign in to comment.