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

[rush] install-run.js: _cleanInstallFolder() fails on Windows #1878

Closed
1 of 2 tasks
mikeharder opened this issue May 19, 2020 · 0 comments · Fixed by #1880
Closed
1 of 2 tasks

[rush] install-run.js: _cleanInstallFolder() fails on Windows #1878

mikeharder opened this issue May 19, 2020 · 0 comments · Fixed by #1880

Comments

@mikeharder
Copy link
Contributor

mikeharder commented May 19, 2020

Is this a feature or a bug?

  • Feature
  • Bug

Please describe the actual behavior.
On Windows, the install-run-rush.js script fails with the following error if it needs to call the _cleanInstallFolder() function inside install-run.js:

Error: Error cleaning the package install folder 
(C:\js\common\temp\install-run\@[email protected]): 
Error: EPERM: operation not permitted, rename 
'C:\js\common\temp\install-run\@[email protected]\node_modules' -> 
'C:\js\common\temp\rush-recycler\install-run-1589914852715'

Here's an easy way to repro:

> node common\scripts\install-run-rush.js --help
> del "common\temp\install-run\@[email protected]\installed.flag"
> node common\scripts\install-run-rush.js --help

I believe the root cause is these two lines:

const rushRecyclerFolder = _ensureAndJoinPath(rushTempFolder, 'rush-recycler', `install-run-${Date.now().toString()}`);
fs.renameSync(nodeModulesFolder, rushRecyclerFolder);

The first line creates the rushRecyclerFolder, and the second line tries to rename nodeModulesFolder to rushRecyclerFolder. The problem is Windows doesn't allow renaming a folder to an existing folder name.

I see two possible solutions:

  1. Delete rushRecyclerFolder before the rename:
fs.rmdirSync(rushRecyclerFolder);
fs.renameSync(nodeModulesFolder, rushRecyclerFolder);
  1. Rename nodeModulesFolder to a sub-folder under rushRecyclerFolder. Something like:
fs.renameSync(nodeModulesFolder, path.join(rushRecyclerFolder, NODE_MODULES_FOLDER_NAME));

@octogonz, @iclanton: I can submit a PR for this. Do you prefer fix 1 (rmdir) or fix 2 (rename to subdir)? Are there tests in the repo for install-run.js, so I could add a regression test for this scenario?

Versions

  • Tool: Rush
  • Tool Version: 5.19.3
  • Node Version: 10.20.1
    • Is this a LTS version? yes
    • Have you tested on a LTS version? yes
  • OS: Windows 10
mikeharder added a commit to mikeharder/azure-sdk-for-js that referenced this issue May 19, 2020
mikeharder added a commit to Azure/azure-sdk-for-js that referenced this issue May 20, 2020
- Update pnpm to latest (known to fail on Node 8)
- Reinstall packages with native dependencies after swapping node version
- Delete Rush install folder after swapping node versions
  - Workaround for microsoft/rushstack#1878
mikeharder added a commit to mikeharder/rushstack that referenced this issue May 20, 2020
- Windows does not allow renaming a directory to an existing directory
- Fixes microsoft#1878
@iclanton iclanton moved this to Closed in Bug Triage Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant