Skip to content

Commit 3d99d6b

Browse files
Fix #5165 - missing executable perms when installing with pnpm linker (#6066)
**What's the problem this PR addresses?** This is the fix for missing executable perms when installing dependencies like turbo and esbuild with pnpm linker (#5165, #5991) Resolves #5165 <!-- Describe the rationale of your PR. --> <!-- Link all issues that it closes. (Closes/Resolves #xxxx.) --> **How did you fix it?** <!-- A detailed description of your implementation. --> Looks like the code was missing for setting permissions for the file copied by pnpm linker into content-addressable store I added `destinationFs.chmodPromise` to `copyFileViaIndex` in `fslib` if the file is not in store (`if (!indexStat)` branch) **Checklist** <!--- Don't worry if you miss something, chores are automatically tested. --> <!--- This checklist exists to help you remember doing the chores when you submit a PR. --> <!--- Put an `x` in all the boxes that apply. --> - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing). <!-- See https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released for more details. --> <!-- Check with `yarn version check` and fix with `yarn version check -i` --> - [x] I have set the packages that need to be released for my changes to be effective. <!-- The "Testing chores" workflow validates that your PR follows our guidelines. --> <!-- If it doesn't pass, click on it to see details as to what your PR might be missing. --> - [x] I will check that all automated PR checks pass before the PR gets reviewed. --------- Co-authored-by: Maël Nison <nison.mael@gmail.com>
1 parent 7aa2359 commit 3d99d6b

File tree

3 files changed

+79
-15
lines changed

3 files changed

+79
-15
lines changed

.yarn/versions/a5c55beb.yml

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
releases:
2+
"@yarnpkg/cli": patch
3+
"@yarnpkg/fslib": patch
4+
5+
declined:
6+
- "@yarnpkg/plugin-compat"
7+
- "@yarnpkg/plugin-constraints"
8+
- "@yarnpkg/plugin-dlx"
9+
- "@yarnpkg/plugin-essentials"
10+
- "@yarnpkg/plugin-exec"
11+
- "@yarnpkg/plugin-file"
12+
- "@yarnpkg/plugin-git"
13+
- "@yarnpkg/plugin-github"
14+
- "@yarnpkg/plugin-init"
15+
- "@yarnpkg/plugin-interactive-tools"
16+
- "@yarnpkg/plugin-link"
17+
- "@yarnpkg/plugin-nm"
18+
- "@yarnpkg/plugin-npm"
19+
- "@yarnpkg/plugin-npm-cli"
20+
- "@yarnpkg/plugin-pack"
21+
- "@yarnpkg/plugin-patch"
22+
- "@yarnpkg/plugin-pnp"
23+
- "@yarnpkg/plugin-pnpm"
24+
- "@yarnpkg/plugin-stage"
25+
- "@yarnpkg/plugin-typescript"
26+
- "@yarnpkg/plugin-version"
27+
- "@yarnpkg/plugin-workspace-tools"
28+
- vscode-zipfs
29+
- "@yarnpkg/builder"
30+
- "@yarnpkg/core"
31+
- "@yarnpkg/doctor"
32+
- "@yarnpkg/libzip"
33+
- "@yarnpkg/nm"
34+
- "@yarnpkg/pnp"
35+
- "@yarnpkg/pnpify"
36+
- "@yarnpkg/sdks"
37+
- "@yarnpkg/shell"

packages/acceptance-tests/pkg-tests-specs/sources/features/contentAddressedIndex.test.ts

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,25 @@ import {Filename, ppath, xfs} from '@yarnpkg/fslib';
22

33
describe(`Features`, () => {
44
describe(`Content-Addressed Index`, () => {
5+
if (process.platform !== `win32`) {
6+
test(
7+
`it should preserve executable mode when installing`,
8+
makeTemporaryEnv({
9+
dependencies: {
10+
[`has-bin-entries`]: `1.0.0`,
11+
},
12+
}, {
13+
nodeLinker: `pnpm`,
14+
}, async ({path, run, source}) => {
15+
await run(`install`);
16+
17+
const stat = await xfs.statPromise(ppath.join(path, `node_modules/has-bin-entries/bin-with-exit-code.js`));
18+
const executableBits = 0o111;
19+
expect(stat.mode & executableBits).toEqual(executableBits);
20+
}),
21+
);
22+
}
23+
524
test(
625
`it should use the exact same device/inode for the same file from the same package`,
726
makeTemporaryEnv({
@@ -23,7 +42,7 @@ describe(`Features`, () => {
2342
await run(`install`, {cwd: path2});
2443

2544
const statA = await xfs.statPromise(ppath.join(path, `node_modules/no-deps/package.json`));
26-
const statB = await xfs.statPromise(ppath.join(path, `node_modules/no-deps/package.json`));
45+
const statB = await xfs.statPromise(ppath.join(path2, `node_modules/no-deps/package.json`));
2746

2847
expect({
2948
dev: statA.dev,
@@ -57,7 +76,7 @@ describe(`Features`, () => {
5776
await run(`install`, {cwd: path2});
5877

5978
const statA = await xfs.statPromise(ppath.join(path, `node_modules/no-deps/index.js`));
60-
const statB = await xfs.statPromise(ppath.join(path, `node_modules/no-deps/index.js`));
79+
const statB = await xfs.statPromise(ppath.join(path2, `node_modules/no-deps/index.js`));
6180

6281
expect({
6382
dev: statA.dev,
@@ -79,20 +98,18 @@ describe(`Features`, () => {
7998
}, {
8099
nodeLinker: `pnpm`,
81100
}, async ({path, run, source}) => {
82-
await xfs.mktempPromise(async path2 => {
83-
await run(`install`);
101+
await run(`install`);
84102

85-
const referenceFile = ppath.join(path, `node_modules/no-deps/index.js`);
103+
const referenceFile = ppath.join(path, `node_modules/no-deps/index.js`);
86104

87-
const originalContent = await xfs.readFilePromise(referenceFile, `utf8`);
88-
const newContent = `${originalContent}// oh no, modified\n`;
105+
const originalContent = await xfs.readFilePromise(referenceFile, `utf8`);
106+
const newContent = `${originalContent}// oh no, modified\n`;
89107

90-
await xfs.writeFilePromise(referenceFile, newContent);
108+
await xfs.writeFilePromise(referenceFile, newContent);
91109

92-
await run(`install`);
110+
await run(`install`);
93111

94-
await expect(xfs.readFilePromise(referenceFile, `utf8`)).resolves.toEqual(originalContent);
95-
});
112+
await expect(xfs.readFilePromise(referenceFile, `utf8`)).resolves.toEqual(originalContent);
96113
}),
97114
);
98115

@@ -117,7 +134,7 @@ describe(`Features`, () => {
117134
await run(`install`, {cwd: path2});
118135

119136
const referenceFileA = ppath.join(path, `node_modules/no-deps/index.js`);
120-
const referenceFileB = ppath.join(path, `node_modules/no-deps/index.js`);
137+
const referenceFileB = ppath.join(path2, `node_modules/no-deps/index.js`);
121138

122139
const originalContent = await xfs.readFilePromise(referenceFileA, `utf8`);
123140
const newContent = `${originalContent}// oh no, modified\n`;

packages/yarnpkg-fslib/sources/algorithms/copyPromise.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ async function copyImpl<P1 extends Path, P2 extends Path>(prelayout: Operations,
9595

9696
default: {
9797
throw new Error(`Unsupported file type (${sourceStat.mode})`);
98-
} break;
98+
}
9999
}
100100

101101
// We aren't allowed to modify the destination if we work with the index,
@@ -173,7 +173,13 @@ async function copyFolder<P1 extends Path, P2 extends Path>(prelayout: Operation
173173

174174
async function copyFileViaIndex<P1 extends Path, P2 extends Path>(prelayout: Operations, postlayout: Operations, destinationFs: FakeFS<P1>, destination: P1, destinationStat: Stats | null, sourceFs: FakeFS<P2>, source: P2, sourceStat: Stats, opts: CopyOptions<P1>, linkStrategy: HardlinkFromIndexStrategy<P1>) {
175175
const sourceHash = await sourceFs.checksumFilePromise(source, {algorithm: `sha1`});
176-
const indexPath = destinationFs.pathUtils.join(linkStrategy.indexPath, sourceHash.slice(0, 2) as P1, `${sourceHash}.dat` as P1);
176+
177+
const defaultMode = 0o644;
178+
const sourceMode = sourceStat.mode & 0o777;
179+
180+
// add mode to the index file name if it's not the default b/c different packages could have the file with same content, but different modes
181+
const indexFileName = `${sourceHash}${sourceMode !== defaultMode ? sourceMode.toString(8) : ``}`;
182+
const indexPath = destinationFs.pathUtils.join(linkStrategy.indexPath, sourceHash.slice(0, 2) as P1, `${indexFileName}.dat` as P1);
177183

178184
enum AtomicBehavior {
179185
Lock,
@@ -263,8 +269,12 @@ async function copyFileViaIndex<P1 extends Path, P2 extends Path>(prelayout: Ope
263269
});
264270

265271
postlayout.push(async () => {
266-
if (!indexStat)
272+
if (!indexStat) {
267273
await destinationFs.lutimesPromise(indexPath, defaultTime, defaultTime);
274+
if (sourceMode !== defaultMode) {
275+
await destinationFs.chmodPromise(indexPath, sourceMode);
276+
}
277+
}
268278

269279
if (tempPath && !tempPathCleaned) {
270280
await destinationFs.unlinkPromise(tempPath);

0 commit comments

Comments
 (0)