Uh oh!
There was an error while loading. Please reload this page.
Mono toolchain workload - #51327
Conversation
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. Resolvesdotnet/sdk#16700
lewing
left a comment
There was a problem hiding this comment.
Added some thoughts. More to come
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
b273dbd to
13827a7CompareUh oh!
There was an error while loading. Please reload this page.
13827a7 to
d1e0623Compare| <Import Project="$(NuGetPackageRoot)\microsoft.dotnet.build.tasks.packaging\$(MicrosoftDotNetBuildTasksPackagingVersion)\build\Microsoft.DotNet.Build.Tasks.Packaging.props" /> | ||
| <PropertyGroup> | ||
| <WorkloadTasksAssemblyPath>$([MSBuild]::NormalizeDirectory('$(ArtifactsBinDir)', 'WorkloadBuildTasks', 'Debug', '$(NetCoreAppToolCurrent)'))WorkloadBuildTasks.dll</WorkloadTasksAssemblyPath> |
There was a problem hiding this comment.
@akoeplinger Do you recall why it's always debug?
There was a problem hiding this comment.
We always build all tasks in debug. I believe the reason for that was that one of the subsets modified the configuration property which caused the tasks to not build correctly in mixed configurations. Also hardcoding to one configuration avoids a rebuild when you switch configurations when building the repo.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
SamMonoRT
commented
Apr 19, 2021
cc @grendello from Android pov |
grendello
commented
Apr 19, 2021
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
mhutch
commented
Apr 29, 2021
Regarding b40c49b, are you assuming that the installer/resolver will fall back to |
mhutch
commented
Apr 29, 2021
I would recommend marking all these workloads as abstract so they're only able to be installed via the concrete platform SDK workloads that extend them. |
587c552 to
2b7d364Compare2b7d364 to
921f982Comparesteveisok
commented
Apr 29, 2021
After a bunch of back and forth, we'll be explicit about what happens on osx-arm64. |
lewing
left a comment
There was a problem hiding this comment.
Looks good enough to start producing packs.
I tested the aliasing we're using now with an arm64 runtime on an m1 and produced functional AOT builds |
ViktorHofer
commented
May 4, 2021
This change broke our official builds: |
akoeplinger
commented
May 4, 2021
| --> | ||
| <NETStandardPatchVersion>0</NETStandardPatchVersion> | ||
| <MicrosoftNETRuntimeEmscripten2012Nodewinx64Version>6.0.0-preview.4.21212.1</MicrosoftNETRuntimeEmscripten2012Nodewinx64Version> | ||
| <MicrosoftNETRuntimeEmscriptenVersion>$(MicrosoftNETRuntimeEmscripten2012Nodewinx64Version)</MicrosoftNETRuntimeEmscriptenVersion> |
There was a problem hiding this comment.
Can you please move these two properties down to the other property group and add a comment to indicate which repo these properties are targeting? Similar to all the other properties in this file (see below).
| <Project Sdk="Microsoft.NET.Sdk"> | ||
| <PropertyGroup> | ||
| <TargetFramework>$(NetCoreAppToolCurrent)</TargetFramework> | ||
| <OutputType>Library</OutputType> |
There was a problem hiding this comment.
nit: you can remove this property. It's the default.
| <Target Name="PublishBuilder" | ||
| AfterTargets="Build" | ||
| DependsOnTargets="Publish" /> |
There was a problem hiding this comment.
Why do you need to publish the task?
| @@ -0,0 +1,46 @@ | |||
| <Project> | |||
There was a problem hiding this comment.
If this is a shipping package then it needs the copyright section: https://github.com/dotnet/arcade/blob/cc512d28c9368336a2893172cbd45dc27960e304/src/Microsoft.DotNet.Build.Tasks.TargetFramework.Sdk/src/build/Microsoft.DotNet.Build.Tasks.TargetFramework.Sdk.props#L1
| <Import Project="$([MSBuild]::GetPathOfFileAbove(Directory.Build.props))" /> | ||
| <PropertyGroup> | ||
| <PackageDescription>Internal toolchain package not meant for direct consumption. Please do not reference directly.</PackageDescription> |
There was a problem hiding this comment.
Consider setting <UseRuntimePackageDisclaimer>true</UseRuntimePackageDisclaimer> which adds the standard description for internal packages that we use everywhere else:
runtime/Directory.Build.targets
Line 54 in cd271c1
| <Import Project="$(NuGetPackageRoot)\microsoft.dotnet.build.tasks.packaging\$(MicrosoftDotNetBuildTasksPackagingVersion)\build\Microsoft.DotNet.Build.Tasks.Packaging.props" /> | ||
| <PropertyGroup> | ||
| <WorkloadTasksAssemblyPath>$([MSBuild]::NormalizeDirectory('$(ArtifactsBinDir)', 'WorkloadBuildTasks', 'Debug', '$(NetCoreAppToolCurrent)'))WorkloadBuildTasks.dll</WorkloadTasksAssemblyPath> |
There was a problem hiding this comment.
Use NormalizePath instead. NormalizeDirectory is intended to be used for paths to directories where-as NormalizePath is for file paths.
| <Import Project="Sdk.props" Sdk="Microsoft.NET.Runtime.Emscripten.Node" /> | ||
| <Import Project="Sdk.props" Sdk="Microsoft.NET.Runtime.Emscripten.Sdk" /> | ||
| <Import Project="Sdk.props" Sdk="Microsoft.NETcore.App.Runtime.AOT.Cross.browser-wasm" /> | ||
| </ImportGroup> |
There was a problem hiding this comment.
nit: indentations in this file are off. We use two spaces in all our msbuild files in the repo.
First pass at a more comprehensive mono workload that includes wasm, the aot compilers, and the runtime config task.