Skip to content

Commit

Permalink
perf: remove unnecessary realpath on --cwd (#5603)
Browse files Browse the repository at this point in the history
**What's the problem this PR addresses?**
<!-- Describe the rationale of your PR. -->
<!-- Link all issues that it closes. (Closes/Resolves #xxxx.) -->

The `--cwd` argument is currently resolved using `realpathSync`.
This behavior was introduced in
#373 as a workaround for a bug
caused by symlinks and the recursive `--cwd` logic.

**How did you fix it?**
<!-- A detailed description of your implementation. -->

Removed the unnecessary `realpath` calls. They are no longer needed due
to the removal of the recursive logic in
#5600.

I've also added a few tests, I'll add the rest from
#5595 in a future PR.

**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 <[email protected]>
  • Loading branch information
paul-soporan and arcanis authored Jul 28, 2023
1 parent 1a29836 commit d9c0747
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 5 deletions.
23 changes: 23 additions & 0 deletions .yarn/versions/10553996.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
releases:
"@yarnpkg/cli": patch

declined:
- "@yarnpkg/plugin-compat"
- "@yarnpkg/plugin-constraints"
- "@yarnpkg/plugin-dlx"
- "@yarnpkg/plugin-essentials"
- "@yarnpkg/plugin-init"
- "@yarnpkg/plugin-interactive-tools"
- "@yarnpkg/plugin-nm"
- "@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"
- "@yarnpkg/builder"
- "@yarnpkg/core"
- "@yarnpkg/doctor"
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export {};
import {PortablePath, npath, xfs} from '@yarnpkg/fslib';

describe(`Entry`, () => {
describe(`version option`, () => {
describe(`--version`, () => {
test(
`it should print the version from the package.json when given --version`,
makeTemporaryEnv({}, async ({path, run, source}) => {
Expand All @@ -18,4 +18,82 @@ describe(`Entry`, () => {
}),
);
});

describe(`--cwd`, () => {
test(`should use the specified --cwd (relative path)`, makeTemporaryEnv({}, async ({path, run, source}) => {
await run(`install`);

await xfs.mkdirPromise(`${path}/packages` as PortablePath);

await expect(run(`--cwd`, `packages`, `exec`, `pwd`)).resolves.toMatchObject({
stdout: `${npath.fromPortablePath(`${path}/packages`)}\n`,
});
}));

test(`should use the specified --cwd (absolute path)`, makeTemporaryEnv({}, async ({path, run, source}) => {
await run(`install`);

await xfs.mkdirPromise(`${path}/packages` as PortablePath);

await expect(run(`--cwd`, npath.fromPortablePath(`${path}/packages`), `exec`, `pwd`)).resolves.toMatchObject({
stdout: `${npath.fromPortablePath(`${path}/packages`)}\n`,
});
}));

test(`should use the specified --cwd (relative symlink)`, makeTemporaryEnv({}, async ({path, run, source}) => {
await run(`install`);

await xfs.mkdirPromise(`${path}/packages` as PortablePath);
await xfs.symlinkPromise(`${path}/packages` as PortablePath, `${path}/modules` as PortablePath);

await expect(run(`--cwd`, `modules`, `exec`, `pwd`)).resolves.toMatchObject({
stdout: `${npath.fromPortablePath(`${path}/modules`)}\n`,
});
}));

test(`should use the specified --cwd (absolute symlink)`, makeTemporaryEnv({}, async ({path, run, source}) => {
await run(`install`);

await xfs.mkdirPromise(`${path}/packages` as PortablePath);
await xfs.symlinkPromise(`${path}/packages` as PortablePath, `${path}/modules` as PortablePath);

await expect(run(`--cwd`, npath.fromPortablePath(`${path}/modules`), `exec`, `pwd`)).resolves.toMatchObject({
stdout: `${npath.fromPortablePath(`${path}/modules`)}\n`,
});
}));

test(`should use the specified --cwd (bound relative path)`, makeTemporaryEnv({}, async ({path, run, source}) => {
await run(`install`);

await xfs.mkdirPromise(`${path}/packages` as PortablePath);

await expect(run(`--cwd=packages`, `exec`, `pwd`)).resolves.toMatchObject({
stdout: `${npath.fromPortablePath(`${path}/packages`)}\n`,
});
}));

test(`should use the specified --cwd (bound absolute path)`, makeTemporaryEnv({}, async ({path, run, source}) => {
await run(`install`);

await xfs.mkdirPromise(`${path}/packages` as PortablePath);

await expect(run(`--cwd=${npath.fromPortablePath(`${path}/packages`)}`, `exec`, `pwd`)).resolves.toMatchObject({
stdout: `${npath.fromPortablePath(`${path}/packages`)}\n`,
});
}));

test(`should use the specified --cwd (inside script)`, makeTemporaryEnv({
scripts: {
foo: `yarn --cwd=packages exec pwd`,
},
}, async ({path, run, source}) => {
await run(`install`);

await xfs.mkdirPromise(`${path}/packages` as PortablePath);

await expect(run(`run`, `foo`)).resolves.toMatchObject({
stdout: `${npath.fromPortablePath(`${path}/packages`)}\n`,
});
}));
});
});
6 changes: 3 additions & 3 deletions packages/yarnpkg-cli/sources/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,14 @@ function checkCwd(cli: YarnCli, argv: Array<string>) {

let postCwdArgv = argv;
if (argv.length >= 2 && argv[0] === `--cwd`) {
cwd = xfs.realpathSync(npath.toPortablePath(argv[1]));
cwd = npath.toPortablePath(argv[1]);
postCwdArgv = argv.slice(2);
} else if (argv.length >= 1 && argv[0].startsWith(`--cwd=`)) {
cwd = xfs.realpathSync(npath.toPortablePath(argv[0].slice(6)));
cwd = npath.toPortablePath(argv[0].slice(6));
postCwdArgv = argv.slice(1);
}

cli.defaultContext.cwd = cwd ?? ppath.cwd();
cli.defaultContext.cwd = ppath.resolve(cwd ?? ppath.cwd());
return postCwdArgv;
}

Expand Down

0 comments on commit d9c0747

Please sign in to comment.