Skip to content

Commit be82499

Browse files
LPegasusoctogonz
andauthored
[package-extractor]: fix issue #5719, preventing duplicate-copy conflicts across npm-packlist versions (#5720)
* fix(package-extractor): preventing duplicate-copy conflicts across npm-packlist versions * docs(README.md): update readme * Fix different behavior on Windows that was breaking the unit test * Prepare to publish a PATCH version of Rush --------- Co-authored-by: LPegasus <lpegasus@users.noreply.github.com> Co-authored-by: Pete Gonzalez <4673363+octogonz@users.noreply.github.com>
1 parent 07d8024 commit be82499

10 files changed

Lines changed: 77 additions & 2 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ These GitHub repositories provide supplementary resources for Rush Stack:
217217
| [/build-tests/package-extractor-test-02](./build-tests/package-extractor-test-02/) | This project is used by tests in the @rushstack/package-extractor package. |
218218
| [/build-tests/package-extractor-test-03](./build-tests/package-extractor-test-03/) | This project is used by tests in the @rushstack/package-extractor package. |
219219
| [/build-tests/package-extractor-test-04](./build-tests/package-extractor-test-04/) | This project is used by tests in the @rushstack/package-extractor package. |
220+
| [/build-tests/package-extractor-test-05](./build-tests/package-extractor-test-05/) | This project is used by tests in the @rushstack/package-extractor package. |
220221
| [/build-tests/run-scenarios-helpers](./build-tests/run-scenarios-helpers/) | Helpers for the *-scenarios test projects. |
221222
| [/build-tests/rush-amazon-s3-build-cache-plugin-integration-test](./build-tests/rush-amazon-s3-build-cache-plugin-integration-test/) | Tests connecting to an amazon S3 endpoint |
222223
| [/build-tests/rush-lib-declaration-paths-test](./build-tests/rush-lib-declaration-paths-test/) | This project ensures all of the paths in rush-lib/lib/... have imports that resolve correctly. If this project builds, all `lib/**/*.d.ts` files in the `@microsoft/rush-lib` package are valid. |
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"name": "package-extractor-test-05",
3+
"description": "This project is used by tests in the @rushstack/package-extractor package.",
4+
"version": "1.0.0",
5+
"private": true,
6+
"main": "./dist/index.js",
7+
"files": [
8+
"dist"
9+
],
10+
"scripts": {
11+
"_phase:build": ""
12+
},
13+
"dependencies": {
14+
"@rushstack/node-core-library": "workspace:*"
15+
}
16+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export {};
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/rush",
5+
"comment": "Fix an issue where \"rush deploy\" could fail with EEXIST due to a \"npm-packlist\" regression (GitHub #5720)",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@microsoft/rush"
10+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@rushstack/package-extractor",
5+
"comment": "preventing duplicate-copy conflicts across npm-packlist versions",
6+
"type": "patch"
7+
}
8+
],
9+
"packageName": "@rushstack/package-extractor"
10+
}

common/config/rush/version-policies.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@
103103
"policyName": "rush",
104104
"definitionName": "lockStepVersion",
105105
"version": "5.172.0",
106-
"nextBump": "minor",
106+
"nextBump": "patch",
107107
"mainProject": "@microsoft/rush"
108108
}
109109
]

common/config/subspaces/default/pnpm-lock.yaml

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

libraries/package-extractor/src/PackageExtractor.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,20 @@ export class PackageExtractor {
334334
}
335335
);
336336
const npmPackFiles: string[] = await walkerPromise;
337-
return npmPackFiles;
337+
338+
// npm-packlist@v5 (uses glob@v8) returns "./dist/index.js" for pattern: "./dist/index.js",
339+
// whereas npm-packlist@v2 (uses glob@v7) returns an empty array.
340+
// This may cause duplicate files in the result list, leading to copying conflicts
341+
// in the AssetHandler.ts includeAssetAsync method.
342+
//
343+
// Temporary fix: normalize the file paths to be relative to the package root, and remove duplicates.
344+
//
345+
// TODO: A better long-term fix is to replace "npm-packlist" with "@pnpm/fs.packlist", notes here:
346+
// https://github.com/microsoft/rushstack/pull/5720/changes#r2984873283
347+
const normalizedFiles: string[] = Array.from(
348+
new Set(npmPackFiles.map((file) => path.posix.normalize(file)))
349+
);
350+
return normalizedFiles;
338351
}
339352

340353
/**

libraries/package-extractor/src/test/PackageExtractor.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,17 @@ const project1PackageName: string = 'package-extractor-test-01';
1919
const project2PackageName: string = 'package-extractor-test-02';
2020
const project3PackageName: string = 'package-extractor-test-03';
2121
const project4PackageName: string = 'package-extractor-test-04';
22+
const project5PackageName: string = 'package-extractor-test-05';
2223
const project1RelativePath: string = path.join('build-tests', project1PackageName);
2324
const project2RelativePath: string = path.join('build-tests', project2PackageName);
2425
const project3RelativePath: string = path.join('build-tests', project3PackageName);
2526
const project4RelativePath: string = path.join('build-tests', project4PackageName);
27+
const project5RelativePath: string = path.join('build-tests', project5PackageName);
2628
const project1Path: string = path.join(repoRoot, project1RelativePath);
2729
const project2Path: string = path.resolve(repoRoot, project2RelativePath);
2830
const project3Path: string = path.resolve(repoRoot, project3RelativePath);
2931
const project4Path: string = path.resolve(repoRoot, project4RelativePath);
32+
const project5Path: string = path.resolve(repoRoot, project5RelativePath);
3033

3134
function getDefaultProjectConfigurations(): IExtractorProjectConfiguration[] {
3235
return [
@@ -619,4 +622,13 @@ describe(PackageExtractor.name, () => {
619622
Sort.sortBy(metadata.projects, (x) => x.path);
620623
expect(metadata).toMatchSnapshot();
621624
});
625+
626+
it('should normalize and remove duplicate file paths', async () => {
627+
await FileSystem.writeFileAsync(path.join(project5Path, 'dist', 'index.js'), '', {
628+
ensureFolderExists: true
629+
});
630+
const result = await PackageExtractor.getPackageIncludedFilesAsync(project5Path);
631+
// To make the test work on both Windows and *nix, need to normalize the paths to posix style
632+
expect(result.map(path.posix.normalize)).toEqual(['dist/index.js', 'package.json']);
633+
});
622634
});

rush.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,12 @@
690690
"reviewCategory": "tests",
691691
"shouldPublish": false
692692
},
693+
{
694+
"packageName": "package-extractor-test-05",
695+
"projectFolder": "build-tests/package-extractor-test-05",
696+
"reviewCategory": "tests",
697+
"shouldPublish": false
698+
},
693699
{
694700
"packageName": "rush-mcp-example-plugin",
695701
"projectFolder": "build-tests/rush-mcp-example-plugin",

0 commit comments

Comments
 (0)