-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[release/9.0] Emscripten version reference for RTM build #106881
base: release/9.0
Are you sure you want to change the base?
Conversation
eng/Versions.props
Outdated
@@ -240,7 +240,8 @@ | |||
like - DarcDependenciesChanged.Microsoft_NET_Workload_Emscripten_Current_Manifest-9_0_100_Transport | |||
--> | |||
<MicrosoftNETWorkloadEmscriptenCurrentManifest90100TransportVersion>9.0.0-rc.2.24421.3</MicrosoftNETWorkloadEmscriptenCurrentManifest90100TransportVersion> | |||
<MicrosoftNETRuntimeEmscriptenVersion>$(MicrosoftNETWorkloadEmscriptenCurrentManifest90100TransportVersion)</MicrosoftNETRuntimeEmscriptenVersion> | |||
<MicrosoftNETWorkloadEmscriptenCurrentManifest90100Version>9.0.0-rc.2.24421.3</MicrosoftNETWorkloadEmscriptenCurrentManifest90100Version> | |||
<MicrosoftNETRuntimeEmscriptenVersion>$(MicrosoftNETWorkloadEmscriptenCurrentManifest90100Version)</MicrosoftNETRuntimeEmscriptenVersion> |
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 is because there are references to the packages via stable versions?
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.
TransportVersion is coming from non-shipping dependency and that did not work when used for references with version defined by MicrosoftNETRuntimeEmscriptenVersion. I have added another dependency with shipping versioning (MicrosoftNETWorkloadEmscriptenCurrentManifest90100Version) and use that version as for further references of Emscripten.
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.
Was the source build version not sufficient?
Tagging subscribers to this area: @dotnet/runtime-infrastructure |
Today's the RC2 snap. There's a merge conflict and an open conversation. @pavel-purma can you please unblock the PR so we can get it merged? |
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 should no longer be needed (for rtm at least). See the discussion here dotnet/sdk#43554
The manifests change name when they go to rtm now, not when the versions and as a result the same name is calculated from the stable version and transport package version in rtm.
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.
@pavel-purma - Please retarget this PR to the release/9.0-staging branch.
You can retarget to that branch by clicking on the Edit button on the top right (next to the PR title) and choosing the release/9.0-staging branch from the dropdown. Important: Please make sure you don't bring any unrelated changes from the release/9.0 branch when retargeting to release/9.0-staging.
Added reference to Microsoft.NET.Workload.Emscripten.Current.Manifest and use that version fro Emscripten packages for RTM build