Repository navigation
Run npm update -g in a scratch HOME and TMPDIR - #2224
Merged
Merged
Conversation
Install scripts run by the update, opencode's among them, wrote root-owned directories into /root that every branch inherited. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Collaborator
Author
|
infra rerun: upstream-verify: "fatal: couldn't find remote ref refs/pull/2224/head" (https://github.com/Sanne/incus-spawn/actions/runs/43499384786) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2221
What
isx update-allandisx project updaterunnpm update -gwith a scratchHOMEandTMPDIR, keeping root's npm cache and npmrc, so install scripts (opencode's postinstall) no longer leave root-owned directories in/rootthat branches inherit.Design
Chosen: wrap the update in a script (
NpmGlobalInstall.UPDATE_SCRIPT) that sets scratch HOME/TMPDIR and removes it on exit. Alternative: clean the directories afterwards. Why: it covers every package's install script, as the issue suggested, and matches whatOpencodeSetupdoes for the build.Regression test
NpmGlobalInstallTest#updateRunsInstallScriptsInAScratchHomeAndRemovesIt: runs the script against a stub npm and asserts HOME/TMPDIR are a scratch directory, removed afterwards, with cache and npmrc kept. On main it does not compile (noUPDATE_SCRIPT); the old code rannpm update -gdirectly with root's HOME.CHARACTER
nothing
Docs
design/templates.md(opencode paragraph).Verified on
Container box, JVM: full
mvn testgreen (968 tests). Not verified against a real Incus or native: the script is plainsh.Handoff
none
Self-review
Self-review: review medium: 4 findings, applied 3 (doc, trap cleanup, npmrc kept), reverted 0, not applied 1
Review findings not applied:
shpasses its status; not worth a second test.Follow-ups
none (PR #2218 not touching these files was not checked beyond the issue's note)
Risk
low: a reviewer should look hardest at
npm_config_userconfig/cachecarry-over inUPDATE_SCRIPT.Timing
build ~1m, full unit run 9m
Co-Authored-By: Claude Sonnet 5.5 noreply@anthropic.com
🤖 Generated with Claude Code