-
Notifications
You must be signed in to change notification settings - Fork 447
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
[main] Fix MonoToolchain.Current naming and Update dependencies from dotnet/emsdk and dotnet/sdk #15366
Conversation
Microsoft.DotNet.Common.ItemTemplates , Microsoft.DotNet.MSBuildSdkResolver , Microsoft.NET.Sdk , Microsoft.TemplateEngine.Cli From Version 8.0.100-alpha.1.23077.4 -> To Version 8.0.100-alpha.1.23077.5
Microsoft.DotNet.Common.ItemTemplates , Microsoft.DotNet.MSBuildSdkResolver , Microsoft.NET.Sdk , Microsoft.TemplateEngine.Cli From Version 8.0.100-alpha.1.23077.4 -> To Version 8.0.100-alpha.1.23077.7
46d8b95
to
cde3ded
Compare
waiting for dotnet/runtime#80401 |
0a88868
to
ba33343
Compare
We don't need to wait for dotnet/runtime#80401 the current runtime workload dependency is on the net7 pack that does exist in both builds. |
c308885
to
8981bd9
Compare
@dsplaisted for dotnet/darc packages is there some reason we can't just assume the feature band from the package version instead of the package name, setup a fallback order where release/rtm > rc > preview > alpha/beta and do the right thing automatically? Getting the darc flow corrent and renaming all the variables for each branding change seems unmaintainable and we still end explicitly setting the band even when it is in the name. |
or we could flip the naming convention and use the nuget version unless there is a cc @steveisok |
opened #15367 |
@@ -1,5 +1,6 @@ | |||
<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | |||
<PropertyGroup> | |||
<NetFeatureBandPrerelease>8.0.100-$(PreReleaseVersionLabel).$(PreReleaseVersionIteration)</NetFeatureBandPrerelease> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't 100% correct since it is encoded in the package name and could be different from the sdk PreReleaseVersion* setting but I can't use the package version for the same reason and using the package name here would break intent of the prerelease setting. #15367
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least this way it will fail if there isn't a package that satisfies the conditions required to make it mostly work
No idea why this pr could be causing a wpf failure, but those are the only failing tests. |
You could probably keep have package name shift, but construct the package ref in here based on this repo's feature band/major version. This would work as long as the version of the package always remains the same as another that does have a stable name. |
<BundledManifests Include="Microsoft.NET.Workload.Emscripten.net7" FeatureBand="$(NetFeatureBand)" Version="$(EmscriptenWorkloadManifestVersion)" /> | ||
<BundledManifests Include="Microsoft.NET.Workload.Mono.ToolChain.net6" FeatureBand="$(NetFeatureBand)" Version="$(MonoWorkloadManifestVersion)" /> | ||
<BundledManifests Include="Microsoft.NET.Workload.Mono.ToolChain.net7" FeatureBand="$(NetFeatureBand)" Version="$(MonoWorkloadManifestVersion)" /> | ||
<BundledManifests Include="Microsoft.NET.Workload.Mono.ToolChain.Current" FeatureBand="$(NetFeatureBandPrerelease)" Version="$(MonoWorkloadManifestVersion)" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to remember to update the VS authoring once an SDK with this change is inserted.
Because runtime as a dependency on emsdk, flowing them directly doesn't result in a coherent product until runtime gets the same update and flows through. So coherency tying them is the right thing to do. Note that you need to add the dependency to sdk for the coherency tie to work predictably. |
#15370 test failure is unrelated to the workload changes |
@joeloff we're hitting signing issues on hashed msi names now |
Updated the signing exclusions, hopefully that is the last thing |
/backport to release/8.0.1xx |
Awww, isn't backport in arcade now? |
Update dotnet/emsdk dependency with new workload prerelease naming (and add a comment about the incoming mono name change)