Skip to content

Commit

Permalink
fix(fslib): handle symlinks to zip files (#6603)
Browse files Browse the repository at this point in the history
This is the same as #5474 but
allows edits by maintainer

**What's the problem this PR addresses?**
<!-- Describe the rationale of your PR. -->
Yarn shouldn't care if zips in the cache are symlinks or not. The only
reason it doesn't work is that finding the mount point skips zips if
they're not regular files. This used to be supported until it was
removed for "performance" reasons in
#1474. The bulk of the logic in the
linked PR involves resolving the real path of the symlink but I don't
think that's needed for this?

<!-- Link all issues that it closes. (Closes/Resolves #xxxx.) -->
Resolves #3514
Closes #5474 (supersedes it)

**How did you fix it?**
I switched the check to use `stat` from `lstat`.

**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: merceyz <[email protected]>
  • Loading branch information
thatsmydoing and merceyz authored Nov 18, 2024
1 parent 9155359 commit fe7c10a
Show file tree
Hide file tree
Showing 10 changed files with 111 additions and 4 deletions.
2 changes: 1 addition & 1 deletion .pnp.cjs

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

39 changes: 39 additions & 0 deletions .yarn/versions/757578fa.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
releases:
"@yarnpkg/cli": patch
"@yarnpkg/core": patch
"@yarnpkg/fslib": patch
"@yarnpkg/pnp": patch

declined:
- "@yarnpkg/plugin-compat"
- "@yarnpkg/plugin-constraints"
- "@yarnpkg/plugin-dlx"
- "@yarnpkg/plugin-essentials"
- "@yarnpkg/plugin-exec"
- "@yarnpkg/plugin-file"
- "@yarnpkg/plugin-git"
- "@yarnpkg/plugin-github"
- "@yarnpkg/plugin-http"
- "@yarnpkg/plugin-init"
- "@yarnpkg/plugin-interactive-tools"
- "@yarnpkg/plugin-link"
- "@yarnpkg/plugin-nm"
- "@yarnpkg/plugin-npm"
- "@yarnpkg/plugin-npm-cli"
- "@yarnpkg/plugin-pack"
- "@yarnpkg/plugin-patch"
- "@yarnpkg/plugin-pnp"
- "@yarnpkg/plugin-pnpm"
- "@yarnpkg/plugin-stage"
- "@yarnpkg/plugin-typescript"
- "@yarnpkg/plugin-version"
- "@yarnpkg/plugin-workspace-tools"
- vscode-zipfs
- "@yarnpkg/builder"
- "@yarnpkg/doctor"
- "@yarnpkg/extensions"
- "@yarnpkg/libzip"
- "@yarnpkg/nm"
- "@yarnpkg/pnpify"
- "@yarnpkg/sdks"
- "@yarnpkg/shell"
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,33 @@ describe(`Commands`, () => {
}),
);

tests.testIf(
() => process.platform !== `win32`,
`it should install from zips that are symlinks`,
makeTemporaryEnv({
dependencies: {
[`no-deps`]: `1.0.0`,
},
}, async ({path, run, source}) => {
await run(`install`);

const allFiles = await xfs.readdirPromise(ppath.join(path, `.yarn/cache`));
const zipFiles = allFiles.filter(file => file.endsWith(`.zip`));

await xfs.mkdirPromise(ppath.join(path, `store`));
for (const filename of zipFiles) {
const zipFile = ppath.join(path, `.yarn/cache`, filename);
const storePath = ppath.join(path, `store`, filename);
await xfs.movePromise(zipFile, storePath);
await xfs.symlinkPromise(storePath, zipFile);
}

await xfs.removePromise(ppath.join(path, Filename.pnpCjs));

await run(`install`, `--immutable`);
}),
);

test(
`it should refuse to create a lockfile when using --immutable`,
makeTemporaryEnv({
Expand Down
27 changes: 27 additions & 0 deletions packages/acceptance-tests/pkg-tests-specs/sources/pnp.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2253,4 +2253,31 @@ describe(`Plug'n'Play`, () => {
});
}),
);

testIf(
() => process.platform !== `win32`,
`it can resolve files from zips that are symlinks`,
makeTemporaryEnv({
dependencies: {
[`no-deps`]: `1.0.0`,
},
}, async ({path, run, source}) => {
await run(`install`);

const allFiles = await xfs.readdirPromise(ppath.join(path, `.yarn/cache`));
const zipFiles = allFiles.filter(file => file.endsWith(`.zip`));

await xfs.mkdirPromise(ppath.join(path, `store`));
for (const filename of zipFiles) {
const zipFile = ppath.join(path, `.yarn/cache`, filename);
const storePath = ppath.join(path, `store`, filename);
await xfs.movePromise(zipFile, storePath);
await xfs.symlinkPromise(storePath, zipFile);
}

await expect(
source(`require('no-deps')`),
).resolves.toEqual({name: `no-deps`, version: `1.0.0`});
}),
);
});
2 changes: 1 addition & 1 deletion packages/yarnpkg-core/sources/worker-zip/index.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/yarnpkg-fslib/sources/MountFS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1013,7 +1013,7 @@ export class MountFS<MountedFS extends MountableFS> extends BasePortableFakeFS {
continue;

try {
if (this.typeCheck !== null && (this.baseFs.lstatSync(filePath).mode & constants.S_IFMT) !== this.typeCheck) {
if (this.typeCheck !== null && (this.baseFs.statSync(filePath).mode & constants.S_IFMT) !== this.typeCheck) {
this.notMount.add(filePath);
continue;
}
Expand Down
13 changes: 13 additions & 0 deletions packages/yarnpkg-libzip/tests/ZipOpenFS.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,15 @@ export const ZIP_DIR3 = ppath.join(
npath.toPortablePath(__dirname),
`fixtures/foo.hiddenzip` as Filename,
);
export const ZIP_DIR4 = ppath.join(
npath.toPortablePath(__dirname),
`fixtures/symlink.zip` as Filename,
);

export const ZIP_FILE1 = ppath.join(ZIP_DIR1, `foo.txt`);
export const ZIP_FILE2 = ppath.join(ZIP_DIR2, `foo.txt`);
export const ZIP_FILE3 = ppath.join(ZIP_DIR3, `foo.txt`);
export const ZIP_FILE4 = ppath.join(ZIP_DIR4, `foo.txt`);

afterEach(() => {
jest.useRealTimers();
Expand Down Expand Up @@ -81,6 +86,14 @@ describe(`ZipOpenFS`, () => {
fs.discardAndClose();
});

it(`can read from a zip file that's a symlink`, () => {
const fs = new ZipOpenFS();

expect(fs.readFileSync(ZIP_FILE4, `utf8`)).toEqual(`foo\n`);

fs.discardAndClose();
});

it(`doesn't close a ZipFS instance with open handles`, () => {
const fs = new ZipOpenFS({maxOpenFiles: 1});

Expand Down
Binary file not shown.
1 change: 1 addition & 0 deletions packages/yarnpkg-libzip/tests/fixtures/symlink.zip
2 changes: 1 addition & 1 deletion packages/yarnpkg-pnp/sources/hook.js

Large diffs are not rendered by default.

0 comments on commit fe7c10a

Please sign in to comment.