dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.22k stars 1.35k forks source link

NuGet-based SDK resolver design spec #2803

Closed jeffkl closed 6 years ago

jeffkl commented 6 years ago

Overview

At the moment, there are two SDK resolvers, both of which assume that the SDKs are on disk. It would be very useful to have SDKs resolved as NuGet packages as well which would allow third party SDKs to be acquired.

Related issues:

User Experience

Users would have to opt into the new behavior by specifying a version of the SDK to use. The resolver would never automatically get the latest because this would introduce very non-deterministic builds.

Users could control the version on a per-project basis:

<Project Sdk="My.Custom.Sdk/2.3.4">
  ...
</Project>

In this example, a NuGet package named My.Custom.Sdk version 2.3.4 would be retrieved and imported.

It would scale much better if users could specify a version per SDK for the whole repository. They could do this in their global.json:

{
  "sdk" : {
    "version:" "1.0.0"
  },
  "msbuild-sdks": {
    "My.Custom.Sdk" : "2.3.4"
  }
}

The sdk object controls which version of the .NET CLI to use and the msbuild-sdks object controls which versions of the MSBuild SDKs to use. If the resolver detected that multiple versions of the same SDK are requested for the same build episode, a warning would be logged and the first version specified would be used.

MSB4528: warning: A version of the SDK 'My.Custom.Sdk' was already specified in 'D:\Foo\global.json'.  The version specified at 'C:\Foo\bar.csproj (1.7)' was ignored.

In Visual Studio, users would see projects in a Loading state while SDKs are resolved. Any errors that occur during acquisition would be reported in the Output window just like other package restore or build errors. Projects would fail to load if an SDK could not be resolved. Users could fix the issue and reload the project in Visual Studio.

image

Since the SDKs would be NuGet packages, the standard caching mechanisms and configurations would be in place. Users would control which feeds to use and other NuGet settings in their NuGet.config.

Implementation

If time permits, I'd like to add a command-line argument like /resolver which would allow users to specify custom resolvers to address #2278. Repository owners could control resolvers with Directory.Build.rsp. Resolvers would not be able to be specified in an import because resolvers load before projects are evaluated.

jeffkl commented 6 years ago

FYI @nguerrera

jaredpar commented 6 years ago

. It would be very useful to have SDKs resolved as NuGet packages as well which would allow third party SDKs to be acquired.

Think this design should encompass bringing in the .NET Core SDK via a NuGet package. Today it must be installed SxS with MSBuild and that creates a lot of friction in our initial build experience. Literally everything about the build except the .NET Core SDK can be bootstrapped. This is true even though the .NET Core SDK has supported xcopy distributions.

jp2masa commented 6 years ago

I'm working on 2 project systems, and I'd like to be able to create MSBuild SDKs which can be consumed by the project systems on both Visual Studio and the dotnet CLI. Installing the SDKs to the SDKs folder is not really easy (as it requires admin permission, and for the dotnet CLI it's even harder, as it would have to be installed on all the dotnet SDK installations). As one of the project systems is an extension for managed project systems, I use PackageReference, but it causes some problems on MEF parts import.

I see that the milestone for this feature is MSBuild 15.6, is it planned for the 15.6 release?

jeffkl commented 6 years ago

Yes I'm working to get this out in the next release

jeffkl commented 6 years ago

PR is out for a NuGet-based SDK resolver... #2850

natemcmaster commented 6 years ago

A few questions.

  1. What is the expected location for the Sdk.props/targets files inside a NuGet package?
  2. What takes precedence when setting the version of the NuGet SDK package to download, project or global.json? If version is absent from the <Sdk> element/attribute in a project, will the version in global.json be applied?
  3. /target:Restore supports using MSBuild properties for restore settings, such as adding feeds via RestoreSources, changing the NuGet cache folder with RestorePackagesPath or the location of the NuGet.config file.. This is something we must use because Nuget.config does not support conditionals. (Example in aspnet/Kestrel: build/sources.props). Can these MSBuild properties be used to control how an SDK package is downloaded by the NuGet resolver?
jeffkl commented 6 years ago

@natemcmaster I've been working on documentation and some of the answers are in there: https://github.com/MicrosoftDocs/visualstudio-docs/blob/master/docs/msbuild/how-to-use-project-sdk.md

What is the expected location for the Sdk.props/targets files inside a NuGet package?

Sdk.props and Sdk.targets must be in a root folder in the package named Sdk

MyPackage
└───Sdk
        Sdk.props
        Sdk.targets

What takes precedence when setting the version of the NuGet SDK package to download, project or global.json? If version is absent from the element/attribute in a project, will the version in global.json be applied?

The order is:

  1. Version specified in a project's XML
  2. Version specified in global.json

If no version exists in the project, then the one in global.json applies. We recommend you only put versions in global.json just because its easier to maintain.

/target:Restore supports using MSBuild properties for restore settings, such as adding feeds via RestoreSources, changing the NuGet cache folder with RestorePackagesPath or the location of the NuGet.config file.. This is something we must use because Nuget.config does not support conditionals. (Example in aspnet/Kestrel: build/sources.props). Can these MSBuild properties be used to control how an SDK package is downloaded by the NuGet resolver?

At the moment, no properties are used for the SDK package restores. When resolving an SDK, project properties haven't been read yet. Global properties could be used but only generally come into play with command-line builds. So for now you can only configure SDK restore with a NuGet.config. We'll have to get feedback if that's not going to work and then come up with a solution if needed.

nguerrera commented 6 years ago

Sdk.props and Sdk.targets must be in a root folder in the package named Sdk

nuget package folders are all camelCased. We shouldn't break that convention

jeffkl commented 6 years ago

@nguerrera the folder name comes from what the SDK resolvers use:

https://github.com/dotnet/cli/blob/master/src/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs#L96 https://github.com/Microsoft/msbuild/blob/master/src/NuGetSdkResolver/NuGetSdkResolver.cs#L207 https://github.com/Microsoft/msbuild/blob/master/src/Build/BackEnd/Components/SdkResolution/DefaultSdkResolver.cs#L32

Interestingly, the Sdk folder is the only one that's proper cased:

C:\Program Files\dotnet\sdk\15.5.0-preview-007044\Sdks\Microsoft.NET.Sdk
├───build
├───buildCrossTargeting
├───Sdk
└───tools

So the ship might have already sailed. The NuGetSdkResolver could use lower case names but I'm not sure if we can get that change in? What do you think @AndyGerlicher ? Maybe we should even use a different name in the NuGet packages like just build or msbuildsdk?

nguerrera commented 6 years ago

Yeah, I told everyone the folder should be 'sdk' when we first introduced these, but the feedback did not get applied. :(

build already exists as a nuget concept for PackageName.targets/PackageName.props, but I think build/Sdk.props build/Sdk.targets would work.

nguerrera commented 6 years ago

Maybe we just need to check both on case-sensitive file systems and document 'sdk' as the good way?

natemcmaster commented 6 years ago

@jeffkl thanks for clarifying.

I totally get why, but it's too bad we can't use MSBuild settings to influence the restore inside the NuGet SDK resolver. NuGet.config isn't quite expressive enough, and MSBuild project evaluation has to come after SDKs are resolved. Looks like we'll have to go back to using a console tool to manipulate NuGet.config files before invoking MSBuild.

jeffkl commented 6 years ago

@natemcmaster can you open an issue so I can track the request and investigate a solution?

natemcmaster commented 6 years ago

Ok. https://github.com/Microsoft/msbuild/issues/2914

clairernovotny commented 6 years ago

What is the ordering works w.r.t. multiple SDK's. First one goes outer (first props/last targets?)

One big issue I have with MSBuild.Sdk.Extras is that I want my props to be after the .NET SDK props and my targets before the .NET SDK targets.

I wonder if this would do that: <Project Sdk="Microsoft.NET.Sdk;MSBuild.Sdk.Extras/1.2.1">

Second, it would be really useful if NuGet could add/update these from both the project location and the global.json location. How are people supposed to know these packages exist (and get installed in the "right" way?) I imagine that people would use the NuGet UI or dotnet add gestures just like they do now. Except instead of a PackageReference, it'll add to the Sdk= attribute. Also for updates, how will the user know there's a newer version? Having it display and be updatable in the NuGet UI would be helpful.

Should also show version's/updates for global.json entries since that is the recommended place to put these.

natemcmaster commented 6 years ago

it would be really useful if NuGet could add/update these from both the project location and the global.json location. How are people supposed to know these packages exist (and get installed in the "right" way?)

@onovotny IMO using packageType metadata is a good first step. If we agree to do https://github.com/NuGet/Home/issues/6484, NuGet could use this to add tooling experiences for these types of packages (and by tooling I mean VS, nuget.org search, command-line experiences, etc). I don't think any of those experiences are planned yet, though.

kzu commented 6 years ago

so is this shipping already in current 15.6 previews?

jeffkl commented 6 years ago

@kzu yes it went out in 15.6 Preview 3 and we have some minor improvements coming to it in Preview 4. I've started working on SDKs: https://github.com/Microsoft/MSBuildSdks

clairernovotny commented 6 years ago

@jeffkl What's the right compat story to support both usages? Just add an Sdks directory and the sdk props/targets?

I'd like to migrate my MSBuild.Sdk.Extras to it, but I can't break existing users using as a PackageReference.

jeffkl commented 6 years ago

@onovotny yes at the moment you can just add add an Sdk folder with Sdk.props and Sdk.targets which just imports ..\build\MSBuild.Sdk.Extras.props and ..\build\MSBuild.Sdk.Extras.targets (unless you need do so something more fancy).

SDK packages don't honor dependencies yet and MSBuild considers them to be pretty much standalone when it adds the implicit imports. We don't currently support an SDK depending on an SDK and having multiple imports in dependency order. But I might work on that if the need arises.

dasMulli commented 6 years ago

What happens if a sdk project file also has an sdk import? #inception

clairernovotny commented 6 years ago

How about ordering? Like if mine needs to be after the .net sdk ones, so that I get their props and my targets run before the .net sdk targets?

This is partially a tooling issue; if/when NuGet installs it, how does it know to prepend/append. I need to be able to specify the .net sdk goes before mine.

dasMulli commented 6 years ago

if that works (sdk import in sdk), @onovotny you could in theory make the extras package the single sdk reference and then add an <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" /> in the middle of your targets just where you need it..

jeffkl commented 6 years ago

@onovotny I think this would work best:

<Project Sdk="Microsoft.NET.Sdk">
  <Sdk Name="MSBuild.Extras.Sdk" Version="1.0.0" />
  <PropertyGroup>
      <TargetFramework>net46</TargetFramework>
  </PropertyGroup>
</Project>

In that case, the implicit imports should be like:

<Project Sdk="Microsoft.NET.Sdk">
  <Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />
  <Import Project="Sdk.props" Sdk="MSBuild.Extras.Sdk" Version="1.0.0" />
  <PropertyGroup>
      <TargetFramework>net46</TargetFramework>
  </PropertyGroup>
  <Import Project="Sdk.targets" Sdk="MSBuild.Extras.Sdk" Version="1.0.0" />
  <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />
</Project>

If you need to be imported after Microsoft.NET.Sdk, you probably just want to stick with <PackageReference />. SDK packages are really meant to be the top-level set of imports or if you need to modify how Restore works (which can't be done via a PackageReference).

clairernovotny commented 6 years ago

@jeffkl I don't want to be a PackageReference, as I do want to control how restore works (and include additional metapackages, like UWP).

Also, PackageReference does not have the right extensibility points for the build imports, it pulls me in too late for the targets. I have to tell people to add this to the end of their project file to get in earlier:

<Import Project="$(MSBuildSDKExtrasTargets)" Condition="Exists('$(MSBuildSDKExtrasTargets)')" />

Does it have to be <Sdk Name="MSBuild.Extras.Sdk" Version="1.0.0" /> as a separate element for this? Why can't I say <Project Sdk="Microsoft.NET.Sdk;MSBuild.Extras.Sdk"> where the global.json defines the version? That seems much cleaner, I just need ordering guarantees, and it'd be nice if the NuGet tooling could know that's what should happen.

clairernovotny commented 6 years ago

Better yet would be <Project Sdk="MSBuild.Extras.Sdk"> where my SDK can do the right thing and declare that Microsoft.NET.Sdk has to be "first".

rainersigwald commented 6 years ago

As long as what you're referencing is not another NuGet-delivered SDK, @dasMulli's suggestion https://github.com/Microsoft/msbuild/issues/2803#issuecomment-362004691 should enable <Project Sdk="MSBuild.Extras.Sdk">.

clairernovotny commented 6 years ago

@rainersigwald Neat, that looks like it could work then, assuming that Microsoft.NET.Sdk will always be in-box.

What happens if I do that but the user still has <Project Sdk="Microsoft.NET.Sdk;MSBuild.Extras.Sdk">? What wins the import battle? Are there warnings of multiple imports? Just thinking though cases where people do the wrong thing.

jeffkl commented 6 years ago

@onovotny you do not have to declare a version in the project, global.json is definitely the better way to go. I just keep mentioning it to remind people that if no version is specified in the project or global.json, the NuGet SDK resolver won't do anything.

Your suggestion of doing both SDKs as a list is probably what you want. I tend to use the <Sdk /> element if I want to reference and SDK in an import.

So this:

<Project Sdk="MSBuild.Extras.Sdk;Microsoft.NET.Sdk">
  <PropertyGroup>
      <TargetFramework>net46</TargetFramework>
  </PropertyGroup>
</Project>

Should expand to this:

<Project>
  <Import Project="Sdk.props" Sdk="MSBuild.Extras.Sdk" />
  <Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />
  <PropertyGroup>
      <TargetFramework>net46</TargetFramework>
  </PropertyGroup>
  <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />
  <Import Project="Sdk.targets" Sdk="MSBuild.Extras.Sdk" />
</Project>

You will be able to confirm the ordering in the preprocessor in 15.6 preview 4 (I had to fix a bug)

clairernovotny commented 6 years ago

I just keep mentioning it to remind people that if no version is specified in the project or global.json, the NuGet SDK resolver won't do anything.

Is an error raised then, since no matching SDK would be found? A silent error seems bad...?

jeffkl commented 6 years ago

You get an error that the SDK could not be found. The NuGet SDK resolver notices that no version is specified so MSBuild continues on in the list of resolvers:

  1. NuGet SDK resolver finds nothing because no version is specified
  2. .NET CLI resolver finds nothing because the SDK is not installed with .NET CLI
  3. MSBuild default resolver finds nothing because the SDK is not installed with MSBuild
clairernovotny commented 6 years ago

So what that leaves is what the best practice/recommendation should be?

I would need <Project Sdk="Microsoft.NET.Sdk;MSBuild.Extras.Sdk"> as the ordering to work properly.

Do I rely on that, or as @dasMulli suggested, put the <Import Sdk="Microsoft.NET.Sdk" ... in my Sdk props/targets? That would enable <Project Sdk="MSBuild.Extras.Sdk">, but I question what happens if people do <Project Sdk="Microsoft.NET.Sdk;MSBuild.Extras.Sdk"> anyway in that case (double import warning/error?)

Finally, it's about the tooling doing the right thing with the package on install (correct order or replacement of the existing sdk item).

jeffkl commented 6 years ago

In my opinion, having MSBuild.Extras.Sdk include an SDK reference to Microsoft.NET.Sdk would be a bad choice. It limits what the project owner can do and having them specify <Project Sdk="MSBuild.Extras.Sdk;Microsoft.NET.Sdk" /> doesn't seem like too much overhead. That said, if MSBuild.Extras.Sdk is designed to only extend Microsoft.NET.Sdk and you really want it to be a replacement, then having it include an SDK reference might make sense.

I imagine you'll want to play around with it a little bit and decide what gives your users the best experience when consuming MSBuild.Extras.Sdk. And of course please circle back and let me know if you have any feedback. You should install 15.6 Preview 3 and try it out if you haven't already.

jeffkl commented 6 years ago

Closing this since we've completed all work on our end and remaining items have associated in other repos.