-
Notifications
You must be signed in to change notification settings - Fork 132
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
PortableRuntimeIdentifierGraph.json not found during runtime repo build #3592
Comments
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
This is not really related to dotnet/installer#17185. With dotnet/sdk#34279, the PortableRuntimeIdentifierGraph.json is assumed to be next to
I believe that comes from: @ViktorHofer @dsplaisted thoughts on what the right thing to do here is? Should runtime stop setting that property? Or should sdk allow overriding portable vs non-portable RID graph separately? |
Since the full RID graph isn't supposed to be updated any more, it's probably OK for the runtime to stop setting the You could probably also copy the portable RID graph from the SDK next to the full RID graph, or we could update the SDK to allow overriding the one path without the other. But the first thing I'd try would be to stop setting the property. |
@mthalman how do we go about testing the stage 2 bootstrapping scenario? I'm not sure how to repro the failure locally to check just doing:
|
The steps are a bit involved to do manually. I kicked off an internal CI build to test this scenario with the removal of that property. I'll report back on the results. Takes a few hours to run. |
+1 |
@elinor-fung - I've verified the stage 2 build passes with this removal. Go ahead with that change. We'll need it in all the relevant branches: rc1, 8.0, main. |
Rc1 automatically flows into release/8.0 so we just need two PRs. We usually start in main and then just backport via the backport command. |
cc @tmds |
During the runtime build I'd be surprised if we can just remove it, as otherwise we wouldn't have need to set it in the first place.
This seems to suggest it is not necessary. 🤞 To verify you need to have all the recent changes both in the bootstrap SDK and the sources being built. |
@tmds - I have verified that the removal of this property resolves the issue we were having in source-build. It allows us to successfully run through the SDK bootstrapping scenario, and the smoke tests are passing as well. |
I wonder how code like this can work: https://github.com/dotnet/sdk/blob/05dbd2efce67e49ca6f32224e2761c8766d8cd1e/src/Tasks/Microsoft.NET.Build.Tasks/ResolveReadyToRunCompilers.cs#L183-L189. but anyway, if it works ... it works. |
I'm going to take a guess. This task uses This is the legacy, non-trimmed runtime graph. @mthalman did you verify it on an OS that is in this graph? Then the runtime build would not need a patched version of it. We may be back in a situation where .NET won't source-build unless the distro is already added to this graph... If you update the |
@tmds - I changed the ID to |
I'm surprised it works, but happy we didn't break it. Thanks for checking! |
The following error occurs when attempting to build the VMR (during the runtime repo build) with a source-built version of the SDK (stage 2 bootstrapping scenario):
This occurred after the merge of dotnet/installer#17118.
Binlog: runtime.zip
@elinor-fung - Is this related to dotnet/installer#17185?
The text was updated successfully, but these errors were encountered: