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

Two workloads using the same pack cause MSBuildWorkloadSdkResolver to fail #16700

Closed
jonathanpeppers opened this issue Apr 1, 2021 · 11 comments
Assignees
Milestone

Comments

@jonathanpeppers
Copy link
Member

Example: xamarin/xamarin-android#5539

I added this to the Android workload:

"Microsoft.NET.Runtime.MonoAOTCompiler.Task": {
  "kind": "sdk",
  "version": "6.0.0-preview.4.21177.4"
},

This is also used by dotnet\sdk-manifests\6.0.100\Microsoft.NET.Workload.BlazorWebAssembly\WorkloadManifest.json.

With both workloads using the same pack, dotnet restore fails with:

C:\Users\jopepper\android-toolchain\dotnet\sdk\6.0.100-preview.4.21179.4\MSBuild.dll -nologo -bl -distributedlogger:Microsoft.DotNet.Tools.MSBuild.MSBuildLogger,C:\Users\jopepper\android-toolchain\dotnet\sdk\6.0.100-preview.4.21179.4\dotnet.dll*Microsoft.DotNet.Tools.MSBuild.MSBuildForwardingLogger,C:\Users\jopepper\android-toolchain\dotnet\sdk\6.0.100-preview.4.21179.4\dotnet.dll -maxcpucount -target:Restore -verbosity:m HelloAndroid\HelloAndroid.csproj
C:\src\net5-samples\HelloAndroid\HelloAndroid.csproj : warning MSB4242: The SDK resolver "Microsoft.DotNet.MSBuildWorkloadSdkResolver" failed to run. An item with the same key has already been added. Key: Microsoft.NET.Runtime.MonoAOTCompiler.Task
C:\Users\jopepper\android-toolchain\dotnet\sdk\6.0.100-preview.4.21179.4\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.ImportWorkloads.props(14,3): warning MSB4242: The SDK resolver "Microsoft.DotNet.MSBuildWorkloadSdkResolver" failed to run. An item with the same key has already been added. Key: Microsoft.NET.Runtime.MonoAOTCompiler.Task
C:\Users\jopepper\android-toolchain\dotnet\sdk\6.0.100-preview.4.21179.4\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.ILLink.props(14,3): warning MSB4242: The SDK resolver "Microsoft.DotNet.MSBuildWorkloadSdkResolver" failed to run. An item with the same key has already been added. Key: Microsoft.NET.Runtime.MonoAOTCompiler.Task
C:\Users\jopepper\android-toolchain\dotnet\sdk\6.0.100-preview.4.21179.4\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.ImportWorkloads.targets(16,3): warning MSB4242: The SDK resolver "Microsoft.DotNet.MSBuildWorkloadSdkResolver" failed to run. An item with the same key has already been added. Key: Microsoft.NET.Runtime.MonoAOTCompiler.Task
  Determining projects to restore...
C:\Users\jopepper\android-toolchain\dotnet\sdk\6.0.100-preview.4.21179.4\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.TargetFrameworkInference.targets(109,5): error NETSDK1139: The target platform identifier android was not recognized. [C:\src\net5-samples\HelloAndroid\HelloAndroid.csproj]

Logs: msbuild.zip

I hit this using .NET 6.0.100-preview.4.21179.4.

To workaround I can completely delete this folder and dotnet restore succeeds:

dotnet\sdk-manifests\6.0.100\Microsoft.NET.Workload.BlazorWebAssembly

/cc @dsplaisted @wli3

@dsplaisted
Copy link
Member

Different workload manifests can't declare the same workload pack. Per the spec:

Workloads and packs may not be duplicated in multiple manifests within a band, though manifests may reference workloads and packs defined in other manifests that are expected to be present.

Basically you need to choose which manifest declares the version for the pack.

We should probably improve the error message.

@wli3
Copy link

wli3 commented Apr 2, 2021

2 different workload can reference the same pack right? just cannot be separate manifest file right?

@jonathanpeppers
Copy link
Member Author

Let's see if I understand what needs to happen:

  • BlazorWebAssembly and Android would not include Microsoft.NET.Runtime.MonoAOTCompiler.Task in packs in their WorkloadManifest.json.
  • dotnet/runtime would produce a new Microsoft.NET.Workload.MonoAOTCompiler workload (or some other name).
  • BlazorWebAssembly, Android, iOS, etc. would all use extends for the new MonoAOTCompiler workload?

If that is correct, who is the right person that should own the new workload pack for MonoAOTCompiler?

/cc @pranavkm @steveisok

@steveisok
Copy link
Member

steveisok commented Apr 2, 2021

@jonathanpeppers That's something my team will put together.

@wli3 in our case, the workload + pack will be 1-1 because of each team distributing their own. Assuming I'm reading what the spec says correctly.

@dsplaisted
Copy link
Member

There doesn't necessarily have to be a separate MonoAOTCompiler workload manifest. It depends on what makes sense for shipping the workload packs.

Basically, all of the workload manifests are combined together, and in the combination there should not be any workload pack or workload defined twice. However, a workload can include a workload pack defined in a different workload manifest. So you could use extends on a MonoAOTCompiler workload, but if there's no scenario to install MonoAOTCompiler by itself, then I wouldn't do that. Just define the MonoAOTCompiler workload pack wherever it makes sense and have the workloads in whatever manifest refer to it.

Hopefully that makes sense. Basically there is no fixed relationship between workloads and workload manifests.

/cc @mhutch

@marcpopMSFT marcpopMSFT added this to the 6.0.1xx milestone Apr 6, 2021
steveisok pushed a commit to steveisok/runtime that referenced this issue Apr 7, 2021
We were originally publishing the MonoAOTCompiler nuget and having Blazor, iOS, and Android include it as part of their workload.  Unfortunately, the workload spec does not allow multiple pack references from different manifests.  To get around this 1-1 relationship, we can supply a containing workload that can be consumed via the extends key.

Resolves dotnet/sdk#16700
@CoffeeFlux
Copy link

Hit this locally when running dotnet new blazorwasm with a fresh P5 SDK: https://gist.github.com/CoffeeFlux/ee4d2a255d593095933cca9f01c7e85e

@dsplaisted
Copy link
Member

@jonathanpeppers @steveisok Has this been fixed?

@jonathanpeppers
Copy link
Member Author

Yes, we extend the pack now:

https://github.com/xamarin/xamarin-android/blob/d644609b1bd3614529b1fc110fc468c925e8228c/src/Xamarin.Android.Build.Tasks/Microsoft.NET.Sdk.Android/WorkloadManifest.in.json#L12

You can probably close this, unless you think you need to improve the error message if this happens.

@chucker
Copy link

chucker commented Jun 17, 2021

Hit this locally when running dotnet new blazorwasm with a fresh P5 SDK: https://gist.github.com/CoffeeFlux/ee4d2a255d593095933cca9f01c7e85e

@CoffeeFlux

If you don't need to use MAUI, see if you have a folder /usr/local/share/dotnet/sdk-manifests/6.0.100 and try deleting that (leaving sdk-manifests to be empty).

@mhutch
Copy link
Member

mhutch commented Jun 17, 2021

5cdb5be in PR #18166 improves the message to report the manifests containing the conflicting items. At some point we should probably add a generic message when encountering any WorkloadManifest*Exception that suggests running workload repair, and if workload repair fails to compose the manifests, emit a message that suggests removing and reinstalling the SDK.

@jonathanpeppers
Copy link
Member Author

The use of TryAdd() and messages in 5cdb5be, seems great to me. I think we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants