From 609ef013670fbb12b0702f1638e6bc75d50217b1 Mon Sep 17 00:00:00 2001 From: David Michon Date: Thu, 11 Dec 2025 23:18:36 +0000 Subject: [PATCH 1/3] [package-deps-hash] Migrate to git ls-files --- .../rush/git-ls-files_2025-12-11-23-00.json | 10 +++++ .../git-ls-files_2025-12-11-23-00.json | 10 +++++ common/reviews/api/package-deps-hash.api.md | 1 + .../package-deps-hash/src/getRepoState.ts | 45 ++++++++++++------- .../src/test/getRepoDeps.test.ts | 29 +++++++++--- .../cli/test/RushCommandLineParser.test.ts | 3 +- .../RushCommandLineParserFailureCases.test.ts | 3 +- .../src/logic/ProjectChangeAnalyzer.ts | 11 ++++- .../logic/test/ProjectChangeAnalyzer.test.ts | 3 +- 9 files changed, 90 insertions(+), 25 deletions(-) create mode 100644 common/changes/@microsoft/rush/git-ls-files_2025-12-11-23-00.json create mode 100644 common/changes/@rushstack/package-deps-hash/git-ls-files_2025-12-11-23-00.json diff --git a/common/changes/@microsoft/rush/git-ls-files_2025-12-11-23-00.json b/common/changes/@microsoft/rush/git-ls-files_2025-12-11-23-00.json new file mode 100644 index 00000000000..2690a17dd75 --- /dev/null +++ b/common/changes/@microsoft/rush/git-ls-files_2025-12-11-23-00.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Log a warning if Git-tracked symbolic links are encountered during repo state analysis.", + "type": "none" + } + ], + "packageName": "@microsoft/rush" +} \ No newline at end of file diff --git a/common/changes/@rushstack/package-deps-hash/git-ls-files_2025-12-11-23-00.json b/common/changes/@rushstack/package-deps-hash/git-ls-files_2025-12-11-23-00.json new file mode 100644 index 00000000000..8cbea805475 --- /dev/null +++ b/common/changes/@rushstack/package-deps-hash/git-ls-files_2025-12-11-23-00.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/package-deps-hash", + "comment": "Replace \"git ls-tree\" with \"git ls-files\" to improve performance. Identify symbolic links and return them separately in \"getDetailedRepoStateAsync\". Symbolic links will be omitted from the result returned by \"getRepoStateAsync\", as they are not \"files\".", + "type": "minor" + } + ], + "packageName": "@rushstack/package-deps-hash" +} \ No newline at end of file diff --git a/common/reviews/api/package-deps-hash.api.md b/common/reviews/api/package-deps-hash.api.md index 6a18a450903..1de2d1b706f 100644 --- a/common/reviews/api/package-deps-hash.api.md +++ b/common/reviews/api/package-deps-hash.api.md @@ -33,6 +33,7 @@ export interface IDetailedRepoState { files: Map; hasSubmodules: boolean; hasUncommittedChanges: boolean; + symlinks: Map; } // @beta diff --git a/libraries/package-deps-hash/src/getRepoState.ts b/libraries/package-deps-hash/src/getRepoState.ts index db21877b78d..3867ac7af6b 100644 --- a/libraries/package-deps-hash/src/getRepoState.ts +++ b/libraries/package-deps-hash/src/getRepoState.ts @@ -29,15 +29,18 @@ const STANDARD_GIT_OPTIONS: readonly string[] = [ interface IGitTreeState { files: Map; // type "blob" + symlinks: Map; // type "link" submodules: Map; // type "commit" } /** * Parses the output of the "git ls-tree -r -z" command + * or the "git ls-files --cached -z --format='%(objectmode) %(objecttype) %(objectname)%x09%(path)'" command * @internal */ export function parseGitLsTree(output: string): IGitTreeState { const files: Map = new Map(); + const symlinks: Map = new Map(); const submodules: Map = new Map(); // Parse the output @@ -57,16 +60,21 @@ export function parseGitLsTree(output: string): IGitTreeState { // The newHash will be all zeros if the file is deleted, or a hash if it exists const hash: string = item.slice(tabIndex - 40, tabIndex); - const spaceIndex: number = item.lastIndexOf(' ', tabIndex - 42); + const mode: string = item.slice(0, item.indexOf(' ')); - const type: string = item.slice(spaceIndex + 1, tabIndex - 41); - - switch (type) { - case 'commit': { + switch (mode) { + case '160000': { + // This is a submodule submodules.set(filePath, hash); break; } - case 'blob': + case '120000': { + // This is a symbolic link + symlinks.set(filePath, hash); + break; + } + case '100644': + case '100755': default: { files.set(filePath, hash); break; @@ -79,6 +87,7 @@ export function parseGitLsTree(output: string): IGitTreeState { return { files, + symlinks, submodules }; } @@ -386,6 +395,10 @@ export interface IDetailedRepoState { * The Git file hashes for all files in the repository, including uncommitted changes. */ files: Map; + /** + * The Git file hashes for all symbolic links in the repository, including uncommitted changes. + */ + symlinks: Map; /** * A boolean indicating whether the repository has submodules. */ @@ -413,15 +426,15 @@ export async function getDetailedRepoStateAsync( const statePromise: Promise = spawnGitAsync( gitPath, STANDARD_GIT_OPTIONS.concat([ - 'ls-tree', - // Recursively expand trees - '-r', + 'ls-files', + // Read from the index only + '--cached', // Use NUL as the separator '-z', // Specify the full path to files relative to the root '--full-name', - // As of last commit - 'HEAD', + // Match the format of "git ls-tree". The %(objecttype) placeholder requires git 2.51.0+, so not using yet. + '--format=%(objectmode) type %(objectname)%x09%(path)', '--', ...(filterPath ?? []) ]), @@ -454,13 +467,14 @@ export async function getDetailedRepoStateAsync( } } - const [{ files }, locallyModified] = await Promise.all([statePromise, locallyModifiedPromise]); + const [{ files, symlinks }, locallyModified] = await Promise.all([statePromise, locallyModifiedPromise]); for (const [filePath, exists] of locallyModified) { - if (exists) { + if (exists && !symlinks.has(filePath)) { yield filePath; } else { files.delete(filePath); + symlinks.delete(filePath); } } } @@ -471,7 +485,7 @@ export async function getDetailedRepoStateAsync( gitPath ); - const [{ files, submodules }, locallyModifiedFiles] = await Promise.all([ + const [{ files, symlinks, submodules }, locallyModifiedFiles] = await Promise.all([ statePromise, locallyModifiedPromise ]); @@ -502,7 +516,8 @@ export async function getDetailedRepoStateAsync( return { hasSubmodules, hasUncommittedChanges: locallyModifiedFiles.size > 0, - files + files, + symlinks }; } diff --git a/libraries/package-deps-hash/src/test/getRepoDeps.test.ts b/libraries/package-deps-hash/src/test/getRepoDeps.test.ts index 63325761544..b65928e5cdc 100644 --- a/libraries/package-deps-hash/src/test/getRepoDeps.test.ts +++ b/libraries/package-deps-hash/src/test/getRepoDeps.test.ts @@ -51,12 +51,29 @@ describe(parseGitLsTree.name, () => { const hash: string = '3451bccdc831cb43d7a70ed8e628dcf9c7f888c8'; const output: string = `100644 blob ${hash}\t${filename}\x00`; - const { files } = parseGitLsTree(output); + const { files, symlinks, submodules } = parseGitLsTree(output); + + expect(symlinks.size).toEqual(0); // Expect there to be exactly 0 symlinks + expect(submodules.size).toEqual(0); // Expect there to be exactly 0 submodules expect(files.size).toEqual(1); // Expect there to be exactly 1 change expect(files.get(filename)).toEqual(hash); // Expect the hash to be ${hash} }); + it('can handle a symlink', () => { + const filename: string = 'src/symlink'; + const hash: string = '3451bccdc831cb43d7a70ed8e628dcf9c7f888c8'; + + const output: string = `120000 link ${hash}\t${filename}\x00`; + const { files, symlinks, submodules } = parseGitLsTree(output); + + expect(files.size).toEqual(0); // Expect there to be exactly 0 files + expect(submodules.size).toEqual(0); // Expect there to be exactly 0 submodules + + expect(symlinks.size).toEqual(1); // Expect there to be exactly 1 symlink + expect(symlinks.get(filename)).toEqual(hash); // Expect the hash to be ${hash} + }); + it('can handle a submodule', () => { const filename: string = 'rushstack'; const hash: string = 'c5880bf5b0c6c1f2e2c43c95beeb8f0a808e8bac'; @@ -78,14 +95,16 @@ describe(parseGitLsTree.name, () => { const filename3: string = 'submodule/src/index.ts'; const hash3: string = 'fedcba9876543210fedcba9876543210fedcba98'; - const output: string = `100644 blob ${hash1}\t${filename1}\x00100666 blob ${hash2}\t${filename2}\x00106666 commit ${hash3}\t${filename3}\0`; - const { files, submodules } = parseGitLsTree(output); + const output: string = `100644 blob ${hash1}\t${filename1}\x00100666 blob ${hash2}\t${filename2}\x00160000 commit ${hash3}\t${filename3}\0`; + const { files, symlinks, submodules } = parseGitLsTree(output); - expect(files.size).toEqual(2); // Expect there to be exactly 2 changes + expect(files.size).toEqual(2); // Expect there to be exactly 2 files expect(files.get(filename1)).toEqual(hash1); // Expect the hash to be ${hash1} expect(files.get(filename2)).toEqual(hash2); // Expect the hash to be ${hash2} - expect(submodules.size).toEqual(1); // Expect there to be exactly 1 submodule changes + expect(symlinks.size).toEqual(0); // Expect there to be exactly 0 symlink changes + + expect(submodules.size).toEqual(1); // Expect there to be exactly 1 submodule expect(submodules.get(filename3)).toEqual(hash3); // Expect the hash to be ${hash3} }); }); diff --git a/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts b/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts index 02da1daeac7..1ef158b946f 100644 --- a/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts +++ b/libraries/rush-lib/src/cli/test/RushCommandLineParser.test.ts @@ -10,7 +10,8 @@ jest.mock(`@rushstack/package-deps-hash`, () => { return { hasSubmodules: false, hasUncommittedChanges: false, - files: new Map([['common/config/rush/npm-shrinkwrap.json', 'hash']]) + files: new Map([['common/config/rush/npm-shrinkwrap.json', 'hash']]), + symlinks: new Map() }; }, getRepoChangesAsync(): ReadonlyMap { diff --git a/libraries/rush-lib/src/cli/test/RushCommandLineParserFailureCases.test.ts b/libraries/rush-lib/src/cli/test/RushCommandLineParserFailureCases.test.ts index a5a671afef3..e1f0d46ea64 100644 --- a/libraries/rush-lib/src/cli/test/RushCommandLineParserFailureCases.test.ts +++ b/libraries/rush-lib/src/cli/test/RushCommandLineParserFailureCases.test.ts @@ -13,7 +13,8 @@ jest.mock(`@rushstack/package-deps-hash`, () => { return { hasSubmodules: false, hasUncommittedChanges: false, - files: new Map() + files: new Map(), + symlinks: new Map() }; }, getRepoChangesAsync(): ReadonlyMap { diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index ecf09749f5a..a423bd0cec3 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -305,7 +305,7 @@ export class ProjectChangeAnalyzer { return async function tryGetSnapshotAsync(): Promise { try { - const [{ files: hashes, hasUncommittedChanges }, additionalFiles] = await Promise.all([ + const [{ files: hashes, symlinks, hasUncommittedChanges }, additionalFiles] = await Promise.all([ getDetailedRepoStateAsync(rootDirectory, additionalRelativePathsToHash, gitPath, filterPath), getAdditionalFilesFromRushProjectConfigurationAsync( additionalGlobs, @@ -315,8 +315,15 @@ export class ProjectChangeAnalyzer { ) ]); + if (symlinks.size > 0) { + terminal.writeWarningLine( + `Warning: Detected ${symlinks.size} Git-tracked symlinks in the repository. ` + + `These will be ignored by the change detection engine.` + ); + } + for (const file of additionalFiles) { - if (hashes.has(file)) { + if (hashes.has(file) || symlinks.has(file)) { additionalFiles.delete(file); } } diff --git a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts index 9ab196ea8a8..30c281da13c 100644 --- a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts +++ b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts @@ -30,7 +30,8 @@ jest.mock(`@rushstack/package-deps-hash`, () => { return { hasSubmodules: false, hasUncommittedChanges: false, - files: mockHashes + files: mockHashes, + symlinks: new Map() }; }, getRepoChangesAsync(): ReadonlyMap { From cd7cd602e9dcb5b05fb1356783eea8a6858888aa Mon Sep 17 00:00:00 2001 From: David Michon Date: Thu, 11 Dec 2025 23:34:17 +0000 Subject: [PATCH 2/3] Use constants --- libraries/package-deps-hash/src/getRepoState.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/libraries/package-deps-hash/src/getRepoState.ts b/libraries/package-deps-hash/src/getRepoState.ts index 3867ac7af6b..4debfd6d820 100644 --- a/libraries/package-deps-hash/src/getRepoState.ts +++ b/libraries/package-deps-hash/src/getRepoState.ts @@ -27,6 +27,11 @@ const STANDARD_GIT_OPTIONS: readonly string[] = [ 'maintenance.auto=false' ]; +const OBJECTMODE_SUBMODULE: '160000' = '160000'; +const OBJECTMODE_SYMLINK: '120000' = '120000'; +const OBJECTMODE_FILE_NONEXECUTABLE: '100644' = '100644'; +const OBJECTMODE_FILE_EXECUTABLE: '100755' = '100755'; + interface IGitTreeState { files: Map; // type "blob" symlinks: Map; // type "link" @@ -34,8 +39,7 @@ interface IGitTreeState { } /** - * Parses the output of the "git ls-tree -r -z" command - * or the "git ls-files --cached -z --format='%(objectmode) %(objecttype) %(objectname)%x09%(path)'" command + * Parses the output of the "git ls-tree -r -z" command or of other commands that have been coerced to match its format. * @internal */ export function parseGitLsTree(output: string): IGitTreeState { @@ -63,18 +67,18 @@ export function parseGitLsTree(output: string): IGitTreeState { const mode: string = item.slice(0, item.indexOf(' ')); switch (mode) { - case '160000': { + case OBJECTMODE_SUBMODULE: { // This is a submodule submodules.set(filePath, hash); break; } - case '120000': { + case OBJECTMODE_SYMLINK: { // This is a symbolic link symlinks.set(filePath, hash); break; } - case '100644': - case '100755': + case OBJECTMODE_FILE_NONEXECUTABLE: + case OBJECTMODE_FILE_EXECUTABLE: default: { files.set(filePath, hash); break; From 79319157f848e8f2e31699ff1e6b69ff5bc80f7e Mon Sep 17 00:00:00 2001 From: David Michon Date: Thu, 11 Dec 2025 23:38:41 +0000 Subject: [PATCH 3/3] constant-ify --- libraries/package-deps-hash/src/getRepoState.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libraries/package-deps-hash/src/getRepoState.ts b/libraries/package-deps-hash/src/getRepoState.ts index 4debfd6d820..556c39bf177 100644 --- a/libraries/package-deps-hash/src/getRepoState.ts +++ b/libraries/package-deps-hash/src/getRepoState.ts @@ -32,6 +32,10 @@ const OBJECTMODE_SYMLINK: '120000' = '120000'; const OBJECTMODE_FILE_NONEXECUTABLE: '100644' = '100644'; const OBJECTMODE_FILE_EXECUTABLE: '100755' = '100755'; +// Note that `type` is a stub that is being ignored by the parser in favor of using `mode` to infer, since `%(objecttype)` requires git 2.51.0+ +// e.g. 10644 blob \t +const GIT_LSTREE_FORMAT: string = '%(objectmode) type %(objectname)%x09%(path)'; + interface IGitTreeState { files: Map; // type "blob" symlinks: Map; // type "link" @@ -438,7 +442,7 @@ export async function getDetailedRepoStateAsync( // Specify the full path to files relative to the root '--full-name', // Match the format of "git ls-tree". The %(objecttype) placeholder requires git 2.51.0+, so not using yet. - '--format=%(objectmode) type %(objectname)%x09%(path)', + `--format=${GIT_LSTREE_FORMAT}`, '--', ...(filterPath ?? []) ]),