Fix incorrect merge suppressing my change to publish CG2 framework#49666
Fix incorrect merge suppressing my change to publish CG2 framework#49666trylek merged 11 commits intodotnet:mainfrom
Conversation
As David discovered, I apparently made a merge bug in rebasing my CG2 framework switch-over change prior to checking in that actually suppressed CG2 publishing due to losing the property PublishReadyToRunUseCrossgen2. This change fixes that deficiency. Thanks Tomas
davidwrighton
left a comment
davidwrighton
left a comment
There was a problem hiding this comment.
Correct calc of this needs to use the computed sdk path, not assume its in .dotnet. This will break developers which install a global build of the sdk which has a sufficiently matching dotnet version. (Most of the library developers do this.)
| <PropertyGroup> | ||
| <OriginalDotnetRootValue>$(DOTNET_ROOT)</OriginalDotnetRootValue> | ||
| <DOTNET_ROOT>$(RepoRoot)</DOTNET_ROOT> | ||
| <DOTNET_ROOT>$([System.IO.Path]::Combine('$(RepoRoot)', '.dotnet'))</DOTNET_ROOT> |
There was a problem hiding this comment.
This isn't reliable. The dotnet.cmd tool will not always point at the .dotnet directory. Instead you need to read the directory from artifacts/toolset/sdk.txt.
There was a problem hiding this comment.
Fixed in 4th commit. I have also added a temporary instrumentation showing the DOTNET_ROOT location as the installer builds for some reason still fail to find the correct SDK even though I believe lab machines use the default .dotnet location.
|
@davidwrighton - I believe I have addressed your PR feedback and in fact, thanks to Jeremy's advice it's ultimately super simple. I have also analyzed the binlogs on Windows (there are no binlogs produced for Linux / OSX runs, I'll create a tracking issue for that) and I confirmed that |
ghost
locked as resolved and limited conversation to collaborators
3 participants
As David discovered, I apparently made a merge bug in rebasing my
CG2 framework switch-over change prior to checking in that actually
suppressed CG2 publishing due to losing the property
PublishReadyToRunUseCrossgen2. This change fixes that deficiency.
Thanks
Tomas
/cc @dotnet/crossgen-contrib