Restore NUGET_PACKAGES export at CI initialization#16899
Merged
ViktorHofer merged 1 commit intoMay 29, 2026
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restores CI initialization of NUGET_PACKAGES for Arcade entry points that source eng/common/tools.{sh,ps1} without going through build.{sh,ps1}, addressing the restore/build package-root mismatch described in #16898.
Changes:
- Adds CI-gated
GetNuGetPackageCachePathinitialization in Bash tooling. - Adds the equivalent CI-gated initialization in PowerShell tooling.
- Documents the regression source and why the initialization is needed.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
eng/common/tools.sh |
Exports NUGET_PACKAGES during CI initialization for Bash-based entry points. |
eng/common/tools.ps1 |
Exports NUGET_PACKAGES during CI initialization for PowerShell-based entry points. |
Member
|
I would restore the previous behavior by adding the InitializeToolset calls back to tools.ps1 and tools.sh in that exact place where it got removed. |
When the Arcade msbuild logger was removed in dotnet#16814, the InitializeToolset call in the MSBuild function (tools.{sh,ps1}) was removed alongside it. That call had a load-bearing side effect: InitializeToolset invokes GetNuGetPackageCachePath, which exports NUGET_PACKAGES. Without NUGET_PACKAGES exported, NuGet restore defaults to the user profile while RepoLayout.props sets MSBuild's $(NuGetPackageRoot) to $(RepoRoot)/.packages/ under ContinuousIntegrationBuild=true. Generated .nuget.g.props imports guarded by Exists($(NuGetPackageRoot)...) are silently skipped, and properties contributed by them (e.g. XunitConsoleNetCoreAppPath) end up undefined. Restore the InitializeToolset call in the MSBuild function gated on $ci, matching the pre-dotnet#16814 behavior. Placing it in the MSBuild function rather than at tools.{sh,ps1} load time ensures any configure-toolset overrides have already been imported. Fixes dotnet#16898 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
8c69a21 to
6de2cf2
Compare
Member
Author
|
@ViktorHofer done |
ViktorHofer
approved these changes
May 28, 2026
This was referenced May 28, 2026
Member
|
Wouldn't it be great to have force merge permissions for such cases ;) |
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.
Note
This PR was authored with assistance from GitHub Copilot.
Fixes #16898.
Problem
#16814 ("Remove the Arcade msbuild logger") removed the
InitializeToolsetcall from theMSBuildfunction ineng/common/tools.{sh,ps1}along with the ArcadeLogging.dll wiring.InitializeToolsethad a load-bearing side effect: it callsGetNuGetPackageCachePath, which exportsNUGET_PACKAGES.Repos that enter Arcade via
eng/common/build.{sh,ps1}are unaffected becausebuild.{sh,ps1}callsInitializeToolsetdirectly. Repos that enter viaeng/common/msbuild.{sh,ps1}(e.g. dotnet/runtime'ssrc/tests/build.sh) no longer getNUGET_PACKAGESexported.The downstream effect:
NuGet restore falls back to
~/.nuget/packages/.RepoLayout.propsstill sets MSBuild's$(NuGetPackageRoot)to$(RepoRoot)/.packages/whenContinuousIntegrationBuild=true.Generated
.nuget.g.propsimports guarded byExists($(NuGetPackageRoot)…)are silently skipped.Properties contributed by those imports (e.g.
XunitConsoleNetCoreAppPath) end up undefined, and downstream targets fail. Example failure observed in dotnet/runtime CI:Fix
Restore the
InitializeToolsetcall in theMSBuildfunction (gated on$ci), matching the location and gating semantics from before #16814.InitializeToolsetis idempotent (caches via_InitializeToolset/$global:_InitializeToolset).InitializeBuildToolis not re-added:MSBuild-Corealready calls it on its own code path, andInitializeToolsetinvokesDotNet/InitializeDotNetCliitself on the cold path when it needs to restore the Arcade SDK.MSBuildfunction (rather than attools.{sh,ps1}load time) ensures any repo-specificconfigure-toolset.{sh,ps1}overrides — includinguse_global_nuget_cache/$useGlobalNuGetCache— have already been imported beforeGetNuGetPackageCachePathruns.Validation
bash -n eng/common/tools.shpasses.eng/common/tools.ps1.