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

fix: handle parallel installs #84

Merged
merged 2 commits into from
Feb 2, 2022

Conversation

merceyz
Copy link
Member

@merceyz merceyz commented Jan 23, 2022

What's the problem this PR addresses?

When multiple instances of Corepack try to install the same version of a package manager one of them will fail with an ENOTEMPTY error.

How did you fix it?

Catch the error and assume another instance of Corepack installed the version first and clean up the temp folder

@merceyz merceyz force-pushed the merceyz/fix/parallel-installs branch from 9880770 to 497089b Compare January 23, 2022 23:07
@merceyz merceyz force-pushed the merceyz/fix/parallel-installs branch from 497089b to 26b41b0 Compare January 23, 2022 23:09
Comment on lines +13 to +22
const rimraf = await import(`rimraf`);
return new Promise<void>((resolve, reject) => {
rimraf.default(path, err => {
if (err) {
reject(err);
} else {
resolve();
}
});
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this code in April

@arcanis arcanis merged commit 5cfc6c9 into nodejs:main Feb 2, 2022
@merceyz merceyz deleted the merceyz/fix/parallel-installs branch February 2, 2022 22:34
sounisi5011 added a commit to sounisi5011/package-version-git-tag that referenced this pull request Mar 13, 2023
Old Corepack throws ENOTEMPTY errors (or EPERM errors on Windows) when running package managers in parallel.
To fix this, update the old Corepack when it is detected.
see nodejs/corepack#110
see nodejs/corepack#84

---------

* 💚 [Debug] Try to switch package managers using Corepack in all Node.js

* 💚 [Debug] Run the package manager multiple times

* 💚 [Debug] Using the Package Manager in Test

* 💚 [Debug] Run package managers in parallel

* 💚 Update old Corepack that throws ENOTEMPTY error

    Old Corepack throws ENOTEMPTY error when running package managers in parallel.
    This has been fixed in Corepack v0.11, so if the old Corepack is detected, this will be updated.

* 💚 Include "XXXXXXXXXXXXX" in the argument of the `mktemp` command

    The `mktemp` command on GitHub Actions seems to require the specification of the X's.

* 💚 Also detects EPERM errors thrown by Corepack

    On Windows, the old Corepack seems to throw EPERM errors instead of ENOTEMPTY errors.

* 💚 Modify comments in shell scripts

* 💚 Allow updated Corepack to be used on Windows

    > ```yaml
    > # Windows installs global packages to a directory that has lower priority than
    > # the default node install so we also need to edit $PATH
    > ```
    > Source: https://github.com/vercel/turbo/pull/1632/files#diff-b92a3120126a9ffe46d7d5ec3a8496ef1eac951db09e1972fac7c78438e36c42R62-R63

    > ```bash
    > npm config get prefix >> $GITHUB_PATH
    > ```
    > Source: https://github.com/vercel/turbo/pull/1632/files#diff-b92a3120126a9ffe46d7d5ec3a8496ef1eac951db09e1972fac7c78438e36c42R69

* 💚 Update npm without using npm-windows-upgrade

    This step was added with #185.
    However, perhaps if we run `npm config get prefix >> "${GITHUB_PATH}"`, we can update npm with the `npm install --global npm` command.

* 💚 [Debug] Try updating npm on Windows without updating the environment variable `$PATH`

* ⏪ Revert "💚 [Debug] Try to switch package managers using Corepack in all Node.js"

    This reverts commit 5b4ec7c, 3a62968, 977b430, and 281b88a.

* ✅ [Debug] Verify npm version

* ⏪ Revert "✅ [Debug] Verify npm version"

    This reverts commit c9ddd9d.

* ✅ [Debug] Verify npm version

* ⏪ Revert "💚 [Debug] Try updating npm on Windows without updating the environment variable `$PATH`"

    This reverts commit 919029c.

* 💚 Modify the display content of "Enable Corepack" job

* 💚 Use the prefix "[command]" together with the command `sleep 1`

* 💚 Use the `node -e 'setTimeout(()=>0,10)'` command instead of the `sleep 1` command

    Waiting two seconds just to correct a display shift is a waste of time.
    Note: The POSIX-compliant `sleep` command has a minimum of one second, so we use the `node` command instead.

* 💚 [Debug] Try updating npm on Windows without updating the environment variable `$PATH`

* ⏪ Revert "💚 [Debug] Try updating npm on Windows without updating the environment variable `$PATH`"

    This reverts commit 96914e1.

* ⏪ Revert "✅ [Debug] Verify npm version"

    This reverts commit 70250e2.

* 💚 Modify the display content of "Enable Corepack" job

* 💚 Rewrite `exec_with_debug` function from Bash to Node.js

* 💚 Fix the bash here-document

* 💚 [Debug] Try updating npm on Windows without updating the environment variable `$PATH`

* ⏪ Revert "💚 [Debug] Try updating npm on Windows without updating the environment variable `$PATH`"

    This reverts commit 481b3a0.

* 💚 Wrap complex code that detects old Corepack in a function

* 💚 [Debug] Try to switch package managers using Corepack in all Node.js

* 💚 [Debug] Try without updating old Corepack

* ⏪ Revert "💚 [Debug] Try without updating old Corepack"

    This reverts commit 1631259.

* ⏪ Revert "💚 [Debug] Try to switch package managers using Corepack in all Node.js"

    This reverts commit 6fd6ece.

* 📝 Update CHANGELOG
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.

2 participants