Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test for hard-to-detect recursive CLI calls #996

Merged
merged 14 commits into from May 3, 2024

Conversation

scotttrinh
Copy link
Collaborator

Yarn Berry uses an even different scheme for running bin scripts: it unpacks the script into a temporary directory and adds that temporary directory into the shell's environment.

This mitigate is by far the most expensive, so it's used dead-last to avoid the overhead. Looks like on yarn 4.1.1, the call that succeeds takes an additional 140ms. Updated the call order and caching strategy to cache successful runs of the CLI (if it differs from the current cached value) to speed this up on multiple runs from newer Yarn invocations.

Yarn Berry uses an even different scheme for running bin scripts: it
unpacks the script into a temporary directory and adds that temporary
directory into the shell's environment.

This mitigate is by far the most expensive, so it's used dead-last to
avoid the overhead. Looks like on yarn 4.1.1, the call that succeeds
takes an additional 140ms, so that calling latency is added to every
call of the package bin script here.
In the case of the cache being empty, or it pointing to a wrapper
script, make sure we update the cache after we get a successful run from
the CLI. This will not cache the location if the run failed, which could
be common, but seems safer than always caching.
@scotttrinh scotttrinh enabled auto-merge (squash) May 3, 2024 15:05
@scotttrinh scotttrinh disabled auto-merge May 3, 2024 15:10
@scotttrinh scotttrinh force-pushed the test-for-wrapper-script branch 2 times, most recently from 1ed9d6c to 98d0a6b Compare May 3, 2024 15:17
@scotttrinh scotttrinh enabled auto-merge (squash) May 3, 2024 15:23
@scotttrinh scotttrinh merged commit 9f5431f into master May 3, 2024
8 of 10 checks passed
@scotttrinh scotttrinh deleted the test-for-wrapper-script branch May 3, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant