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

Lengthy RuntimeIdentifierGraph update roundtrip with SDK #37790

Closed
am11 opened this issue Jun 12, 2020 · 10 comments · Fixed by #37824
Closed

Lengthy RuntimeIdentifierGraph update roundtrip with SDK #37790

am11 opened this issue Jun 12, 2020 · 10 comments · Fixed by #37824
Labels
area-Infrastructure-coreclr untriaged New issue has not been triaged by the area owner

Comments

@am11
Copy link
Member

am11 commented Jun 12, 2020

A change to src/libraries/pkg/Microsoft.NETCore.Platforms/runtime.json requires a full update cycle, which includes:

  1. a PR updating runtime.json merged in runtime repo
  2. SDK repo picks up runtime change
  3. runtime picks up the updated SDK

This blocks building CLR tests on a new platform, until all of the above has happened (or we patch SDK after the restore). For example, my current workaround is:

# workaround: overwrite file containing updated RIDs from libraries
/runtime $ cp src/libraries/pkg/Microsoft.NETCore.Platforms/runtime.json \
    .dotnet/sdk/5.0.100-preview.6.20310.4/RuntimeIdentifierGraph.json

# then cross-compile CLR tests without getting restore errors
/runtime $ ROOTFS_DIR=$(pwd)/.tools/rootfs/x64 ./src/coreclr/build-test.sh \
    -x64 -cross -os illumos -gcc

In the age of live-live builds, is it possible to ensure LiveRuntimeIdentifierGraphPath from eng/liveBuilds.targets is properly picked up during the SDK resolution for test build? Currently it seem to have no effect (despite being imported in src/coreclr/tests/src/Common/test_dependencies/test_dependencies.csproj) and SDK ends up using .dotnet/sdk/<version>/RuntimeIdentifierGraph.json, instead of the runtime.json file pointed by LiveRuntimeIdentifierGraphPath.

cc @jashook, @jkotas

@ghost
Copy link

ghost commented Jun 12, 2020

Tagging subscribers to this area: @safern, @ViktorHofer
Notify danmosemsft if you want to be subscribed.

@am11
Copy link
Member Author

am11 commented Jun 12, 2020

I think the previous tag was correct; this was only encountered during the build of CLR tests.

@jkotas
Copy link
Member

jkotas commented Jun 12, 2020

The fundamental problem is the process required to add new RIDs that falls under libraries though.

@ViktorHofer
Copy link
Member

In the age of live-live builds, is it possible to ensure LiveRuntimeIdentifierGraphPath from eng/liveBuilds.targets is properly picked up during the SDK resolution for test build?

I don't know how flowing of the RID graph into the SDK currently works. I assumed this is handled by the Microsoft.NetCore.Platforms OOB package so I'm wondering how the SDK comes into play here. @ericstj might know.

@jkoritzinsky
Copy link
Member

To my knowledge dotnet/installer pulls down the Microsoft.NETCore.Platforms package during build and embeds the file in the final product.

@ericstj
Copy link
Member

ericstj commented Jun 12, 2020

Seems like the SDK just needs a place to define this. Let's see if they have it. /cc @dsplaisted

@dsplaisted
Copy link
Member

You can set the BundledRuntimeIdentifierGraphFile property to the path to the updated runtime identifier graph json file. Is that what you're looking for?

@am11
Copy link
Member Author

am11 commented Jun 12, 2020

I think RuntimeIdentifierGraphPath is the way to define custom path, which SDK checks here, and liveBuilds.targets defines at:

<RuntimeIdentifierGraphPath Condition="'$(UseLiveRuntimeIdentifierGraph)' == 'true'">$(LiveRuntimeIdentifierGraphPath)</RuntimeIdentifierGraphPath>

For some reason it is not taking effect when this targets is imported by Directory.Build.targets at repo root. (it is already too late by then?)

Before the cp workaround mentioned above, I was getting:

/runtime/.dotnet/sdk/5.0.100-preview.6.20310.4/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(59,5): error NETSDK1083: The specified RuntimeIdentifier 'illumos-x64' is not recognized. [/runtime/src/coreclr/tests/src/Common/test_dependencies/test_dependencies.csproj] [/runtime/src/coreclr/tests/build.proj]
/runtime/src/coreclr/tests/build.proj(52,5): error MSB3073: The command ""/runtime/.dotnet/dotnet" restore -r illumos-x64 /runtime/src/coreclr/tests/src/Common/test_dependencies/test_dependencies.csproj  /p:SetTFMForRestore=true /p:TargetOS=illumos /p:TargetArchitecture=x64 /p:Configuration=Debug " exited with code 1.

@ericstj
Copy link
Member

ericstj commented Jun 12, 2020

@am11 it looks like you need to set the property @dsplaisted mentions (BundledRuntimeIdentifierGraphFile) it seems to get used in a few more places than RuntimeIdentifierGraphPath: https://github.com/dotnet/sdk/search?q=BundledRuntimeIdentifierGraphFile&unscoped_q=BundledRuntimeIdentifierGraphFile

@am11
Copy link
Member Author

am11 commented Jun 12, 2020

Replacing RuntimeIdentifierGraphPath with BundledRuntimeIdentifierGraphFile in liveBuilds.targets fixed the issue. I also had to import this targets in external.csproj and ilasm.ilproj. Will prepare a PR shortly. Thank you!

@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-coreclr untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants