fix(nuget): move global.json aside during dotnet setversion#820
Merged
Conversation
Consumer repos often pin SDK/workload versions in global.json with rollForward: disable for deterministic builds. The automatic version bump path added in getsentry#707 invokes `dotnet setversion` with cwd=rootDir, which causes the dotnet host to read global.json and refuse to launch if the pinned SDK isn't installed on the release runner. Other dotnet invocations in this target avoid this by spawning with cwd: '/' (see getsentry#601, getsentry#614). That's not viable for `dotnet-setversion` because it operates on files in cwd. Instead, temporarily rename global.json out of the way for the duration of the call and restore it in a finally block. Fixes getsentry#819 Refs getsentry/sentry-dotnet#5250
jamescrosswell
commented
May 21, 2026
The Lint workflow only runs on pull_request, so this pre-existing violation in nuget.ts never failed CI on master. Surfacing it via this PR — applying `pnpm format:check` so the workflow passes.
Contributor
Author
|
The Change log Preview fails, but I suspect that's because I'm making the PR from a fork (I'm a contractor so don't have permission to create a PR on the repo directly). |
BYK
approved these changes
May 22, 2026
Member
BYK
left a comment
There was a problem hiding this comment.
Thanks so much and sorry for the trouble!
| // which breaks if the pinned SDK isn't installed on the craft runner. | ||
| // See: https://github.com/getsentry/craft/issues/819 | ||
| const globalJsonPath = join(rootDir, 'global.json'); | ||
| const globalJsonBackup = `${globalJsonPath}.craft-bak`; |
Member
There was a problem hiding this comment.
We should probably use a unique name per instance to avoid any potential clashes but since the probabiliy is very low, I'll merge it as it. Would appreciate a follow up to fix this tho.
Contributor
Author
There was a problem hiding this comment.
No worries - follow up PR:
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.
Summary
Fixes #819. Refs getsentry/sentry-dotnet#5250.
The automatic version bumping added in #707 invokes
dotnet setversionwithcwd: rootDir, so the dotnet host reads anyglobal.jsonin the consumer repo. The release runner won't always have that exact SDK installed —dotnetthen refuses to launch andcraft prepareaborts.Other
dotnetinvocations in this target dodge this by spawning withcwd: '/'(see #601, #614). That isn't viable here becausedotnet-setversionoperates on files in cwd.Instead, this PR temporarily renames
global.jsontoglobal.json.craft-bakfor the duration of the call and restores it in afinally. The XML-fallback path is unchanged and unaffected.Notes for reviewers
dotnet-setversionaltogether and rely on the xml based version update logic that is currently a fallback. Hard to say which workaround is better.nuget.ts(src/targets/__tests__/has nonuget.test.ts), so this PR doesn't add one. Happy to add one if you'd like — let me know the preferred shape.