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

First class support for overriding the arcade SDK version #3785

Closed
mmitche opened this issue Dec 13, 2023 · 4 comments
Closed

First class support for overriding the arcade SDK version #3785

mmitche opened this issue Dec 13, 2023 · 4 comments
Labels
area-infra Source-build infrastructure and reporting

Comments

@mmitche
Copy link
Member

mmitche commented Dec 13, 2023

We need better support for overriding the Arcade SDK version. There are at least two places where the arcade SDK version is used:

  • In the SDK resolvers (read from global.json) - We currently override this using a custom sdk resolver we install in the SDK. When certain envvars are set, a different SDK is used. This is awkward. Covered by Design and implement Project SDK resolution that does not involve source-edits or SDK mutation. #3781.
  • For restoring and finding the arcade build project that is the entry points to arcade based builds (Tools.proj and Build.proj). This is read from the global.json, restored, and then set in a variable for use later.

The issue here is that you want repos downstream of arcade to use the correct SDK when it shows up in a project file and the correct Tools/Build.proj. Otherwise you only used the arcade SDK partially. The way this works on Linux right now is that we unpack the arcade SDK to an explicit dir, and then set an environment variable _InitializeToolset to the Build.proj that is the build entry point. This appears to be because the bash behavior variable scoping will pick up environment variables ambiently. In powershell though, we scope most of these variables to global. This means that setting an outside envvar won't work. It will restore the wrong arcade and use the wrong entry point.

This brings up a couple problems overall:

  • Explicitly setting the build proj variables is pretty hacky overall. It relies on Arcade's scripting 'optimizations' not doing any kind of reinitialization to stick with the desired build project.
  • You need to rewrite the common scripting on windows to take into account env vars.
  • The variable names may not be consistent across powershell and bash anyway.

I think ideally, we would cover the build project case by offering a standard environment variable to control the arcade version (overriding global.json) alone. That would then cause the correct restore to happen and the project to launch.

Ideally this could be combined with a real solution for overriding the msbuild SDKs from msbuild's standpoint so that we could also avoid manually extracting msbuild SDK packages in SB/VB mode.

@mmitche mmitche converted this from a draft issue Dec 13, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added area-build Improvements in source-build's own build process untriaged labels Dec 13, 2023
mmitche added a commit to mmitche/arcade that referenced this issue Dec 13, 2023
In  the VMR PoC it was discovered that the SDK kept getting installed even when DOTNET_INSTALL_DIR was already specified. This was because SB was using _InitializeToolset to short circuit arcade and SDK initialization. This is a wider issue discussed in dotnet/source-build#3785. For now, align the behavior of non-Windows with Windows by allowing setting of the _InitializeToolset environment variable (using this to set the global). In addition, align the global variable name with the Nix variants.
@MichaelSimons MichaelSimons added area-infra Source-build infrastructure and reporting and removed area-build Improvements in source-build's own build process untriaged labels Dec 14, 2023
@MichaelSimons MichaelSimons moved this from Backlog to 9.0 in .NET Source Build Dec 14, 2023
@mmitche
Copy link
Member Author

mmitche commented Dec 15, 2023

MSBUILDADDITIONALSDKRESOLVERSFOLDER, MSBUILDADDITIONALSDKRESOLVERSFOLDER_NET, and MSBUILDADDITIONALSDKRESOLVERSFOLDER_NETFRAMEWORK may be some options here.

@ericstj
Copy link
Member

ericstj commented Dec 15, 2023

Looks like these might be interesting:
https://github.com/dotnet/sdk/blob/5ff0a5981ef9409b4b382a4b7045739a3e703d74/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs#L142-L146

// These are overrides that are used to force the resolved SDK tasks and targets to come from a given
// base directory and report a given version to msbuild (which may be null if unknown. One key use case
// for this is to test SDK tasks and targets without deploying them inside the .NET Core SDK.
var msbuildSdksDirFromEnv = _getEnvironmentVariable(EnvironmentVariableNames.DOTNET_MSBUILD_SDK_RESOLVER_SDKS_DIR);
var netcoreSdkVersionFromEnv = _getEnvironmentVariable(EnvironmentVariableNames.DOTNET_MSBUILD_SDK_RESOLVER_SDKS_VER);

Looks like DOTNET_MSBUILD_SDK_RESOLVER_SDKS_DIR can specify a directory where this resolver probes for SdkName\Sdk without any version and uses it. So if we staged such a directory that would be preferred. I noticed though, this resolver isn't used by default -- maybe it's only used when loading the .NET SDK?

Looking at the NuGet resolver, I'm not seeing any hooks, other than to disable it:
https://github.com/NuGet/NuGet.Client/blob/2a234707a663f731e4de93cba4014ed1a8259def/src/NuGet.Core/Microsoft.Build.NuGetSdkResolver/NuGetSdkResolver.cs#L132
Seems like it will read the version from global.json and that's the only version it will use.

Probably adding some resolver of our own that just probes a path might be the easiest thing to do. Ensure that resolver has a higher Priority than the built in ones to make sure it can take precedence.

What might be better is if there was some resolver that had this behavior built in, so we could just specify an additional path to probe for SDKs and it would be preferred.

I wonder what @rainersigwald or @dsplaisted might recommend

@rainersigwald
Copy link
Member

DOTNET_MSBUILD_SDK_RESOLVER_SDKS_DIR applies only when that resolver runs, which it doesn't in dotnet build scenarios, unfortunately.

@mmitche
Copy link
Member Author

mmitche commented Jan 3, 2024

This is already opened as part of #3781 closing here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infra Source-build infrastructure and reporting
Projects
Archived in project
Status: Done
Development

No branches or pull requests

4 participants