-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fix(arborist): omit failed optional dependencies from installed deps #8184
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
Conversation
@zkat I did what I could here. Had a couple of comments/lack of my own clarity in things I changed - see above. |
Other than the comment we really really should re-use the exceptional description from #8177, either by copying it into this issue now or when we merge. Probably easier to do now. I'll do it now. |
const set = optionalSet(node) | ||
for (const node of set) { | ||
node.parent = null | ||
node.ideallyInert = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been thinking about this, and maybe it's better to just call it inert
instead of ideallyInert
. I think putting the ideally
in there is probably a bit too much clever inside baseball and it's more verbose to boot. :)
Some other name would be good, too, as long as it communicates that this is in the tree, but we don't physically install it.
Thanks for pulling this work forward, much appreciated @zkat @owlstronaut @wraithgar. |
… but keep them 'in the tree' Fixes: #4828 Fixes: #7961 Replaces: #8127 Replaces: #8077 When optional dependencies fail, we don't want to install them, for whatever reason. The way this "uninstallation" is done right now is by simply removing the dependencies from the ideal tree during reification. Unfortunately, this means that what gets saved to the "hidden lockfile" is the edited ideal tree as well, and when NPM runs next, it'll use that when regenerating the "real" package-lock. This PR tags failed optional deps such that they're still added to the "trash list" (and thus have their directories cleaned up by the reifier), but prevents Arborist from removing them altogether from the ideal tree (which is usually done by setting their parent to `null`, making them unreachable, and letting them get GC'd).
f635c97
to
68a8d05
Compare
Amazing, I'm so excited for this to be fixed. Thank you @zkat @owlstronaut @wraithgar 🎉 |
@jakebailey Can this be back ported into old versions? |
This is not going to be backported. |
npm version 11.3.0 includes a fix (see: npm/cli#8184) that was pointed out in tailwindlabs/tailwindcss#15806 (comment) where it should resolve properly. See this thread for my own investigation parcel-bundler/lightningcss#956 (comment)
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. closes #7147 Task: 0 Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. Task: 0 X-original-commit: 2612a12
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. Task: 0 X-original-commit: 2612a12
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. closes #7148 Task: 0 X-original-commit: 2612a12 Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Pierre Rousseau (pro) <[email protected]>
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. Task: 0 X-original-commit: 6cc703f
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. Task: 0 X-original-commit: 6cc703f
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. Task: 0 X-original-commit: 6cc703f
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. closes #7151 Task: 0 X-original-commit: 6cc703f Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Pierre Rousseau (pro) <[email protected]>
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. Task: 0 X-original-commit: 724f762
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. Task: 0 X-original-commit: 724f762
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. closes #7153 Task: 0 X-original-commit: 724f762 Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Pierre Rousseau (pro) <[email protected]>
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. Task: 0 X-original-commit: b1dc674
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. Task: 0 X-original-commit: b1dc674
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. Task: 0 X-original-commit: b1dc674
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. Task: 0 X-original-commit: b1dc674
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. closes #7157 Task: 0 X-original-commit: b1dc674 Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Pierre Rousseau (pro) <[email protected]>
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. closes #7156 Task: 0 X-original-commit: b1dc674 Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Pierre Rousseau (pro) <[email protected]>
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. closes #7155 Task: 0 X-original-commit: b1dc674 Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Pierre Rousseau (pro) <[email protected]>
Release commit 8c68f3d has been done with Node.js v22.19.0 and NPM v10.9.3. But in the meantime, NPM had an issue that affected the package-lock.json by pruning OS optional dependencies (See npm/cli#7961). The issue has been fixed (npm/cli#8184) in NPM v11.3 but not backported. During the release process, we ran `npm install`, and so the package-lock.json was updated to remove the optional dependencies for swc binaries. Which caused issues for users each time they run `npm install` as they were not present anymore in the package-lock.json. This commit adds the swc binaries to the optional dependencies of this project to avoid them being pruned again in the future. closes #7154 Task: 0 X-original-commit: b1dc674 Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Pierre Rousseau (pro) <[email protected]>
Discovered while investigating #8535 (comment) Similar to #8566, relates to #8184 Moves `inert` (uninstallable optional) calculation into `buildIdealTree` so it can be used in diff. Also removes most of #8184 in favor of a [simpler fix](https://github.com/npm/cli/pull/8602/files#diff-02626074e1a4a170693607e4a3a69dfc08ee52067734717833b22cf162923e07R354), see PR comments for the journey. Improvements: * we don't see uninstallable packages as "installed" in CLI output * `createSparseTree` no longer creates dirs that will only be cleaned later For the example in the linked issue, it changes output from `added 156 packages` to `added 12 packages` and combined with #8537 it changes to `added 6 packages`, the expected result.
… but keep them 'in the tree'
This PR was authored by @zkat
Fixes: #4828
Fixes: #7961
Replaces: #8127
Replaces: #8077
When optional dependencies fail, we don't want to install them, for whatever reason. The way this "uninstallation" is done right now is by simply removing the dependencies from the ideal tree during reification.
Unfortunately, this means that what gets saved to the "hidden lockfile" is the edited ideal tree as well, and when npm runs next, it'll use that when regenerating the "real" package-lock.
This PR tags failed optional deps such that they're still added to the "trash list" (and thus have their directories cleaned up by the reifier), but prevents Arborist from removing them altogether from the ideal tree (which is usually done by setting their parent to
null
, making them unreachable, and letting them get GC'd).PS: It's Friday, this is what I managed to get done together. I'm making this a draft PR for now so folks can look at it, but I want to make sure both reifiers work well, fix some messaging issues, and clean stuff up (I'm pretty sure I'm guarding
ideallyInert
in more places than I need to, because I was trying to find the right spot). Still, feel free to talk about the approach. I'll get back to this on Monday.PPS: also hi