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

Ensure shared framework consumes source-built RID graph #53550

Closed
ericstj opened this issue Jun 1, 2021 · 11 comments · Fixed by #76068
Closed

Ensure shared framework consumes source-built RID graph #53550

ericstj opened this issue Jun 1, 2021 · 11 comments · Fixed by #76068

Comments

@ericstj
Copy link
Member

ericstj commented Jun 1, 2021

Related to #50818

I believe the shared framework build process is still consuming the statically defined, checked-in, runtime.json.

We need to make sure it uses the generated one.

I suggest we do this by making the shared framework always consume it from an output location (along with the assemblies, etc). Then make the Microsoft.NETCore.Platforms project responsible for putting it there, whether it builds it or not.

Also we should consider scenarios like #52907 where folks want to add RIDs without necessarily doing a source build.

Perhaps there is a way to use the RID generation process in more conditions while still ensuring the checked-in copy is up-to-date?

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jun 1, 2021
@dotnet-issue-labeler
Copy link

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.

@NikolaMilosavljevic NikolaMilosavljevic added this to the 6.0.0 milestone Jun 2, 2021
@NikolaMilosavljevic NikolaMilosavljevic removed the untriaged New issue has not been triaged by the area owner label Jun 2, 2021
@ericstj
Copy link
Member Author

ericstj commented Jun 2, 2021

Note that the fix here will likely require changes in installer and libraries. It's likely going to be me fixing this if I can find the time to get to it. Would love help identifying what to change in installer and how to test this sort of thing (EG: hypothetical new linux or new arch). cc @ViktorHofer @crummel

@ViktorHofer
Copy link
Member

@NikolaMilosavljevic this seems important enough to keep in 6.0.

@ViktorHofer ViktorHofer modified the milestones: 7.0.0, 6.0.0 Aug 16, 2021
@ViktorHofer
Copy link
Member

@ericstj as this issue still targets 6.0, do you have someone to work on it?

@ericstj
Copy link
Member Author

ericstj commented Sep 3, 2021

I'll leave it to the owners of the area to make the call and fix it if they deem it necessary. I don't see it as must fix for 6.0 unless there's a source build scenario that requires it.

@ViktorHofer
Copy link
Member

@NikolaMilosavljevic should we keep this in 6.0 or move it out?

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 17, 2022
@tmds
Copy link
Member

tmds commented Aug 30, 2022

I'll leave it to the owners of the area to make the call and fix it if they deem it necessary. I don't see it as must fix for 6.0 unless there's a source build scenario that requires it.

This is needed to make .NET 6.0 and .NET 7.0 source-build on new versions of a distro.

Currently maintainers need to patch the runtime graph before the build works.

If this is fixed on main, we can see if the patch works for .NET 6 and .NET 7. If it does, maintainers can include it, and no longer need to worry about updating the runtime graph.

Would love help identifying what to change in installer and how to test this sort of thing (EG: hypothetical new linux or new arch).

I have a long open issue for this: dotnet/source-build#297.

We need to identify when we're using the build host rid and when we're using the target rid.

The bootstrap SDK won't know about the unknown distro, so we may need to force it to be compatible. That's already possible by setting DOTNET_RUNTIME_ID. This controls the build host rid. For example: if we're using Microsoft's portable build, it can be set to linux-x64.

The target RID should be treated as the name of the thing we're building. Source-build is already passing this name as TargetRid to the runtime repo. SourceBuild.props then maps it to PackageRid. In #74504 I'm exposing it as an arg on the top-level build script.

@smasher164 smasher164 modified the milestones: 6.0.x, 7.0.0 Aug 31, 2022
@tmds
Copy link
Member

tmds commented Sep 7, 2022

@ericstj I'm source-building .NET 7 with #74504 on an unknown distro (I changed my /etc/os-release to Fedora 39). Besides the PR, I made a change to source-build so it sets RuntimeOS to linux during the runtime build to make it use the linux-x64 prebuilts/sdk.

The build passes.

The shared/Microsoft.NETCore.App/7.0.0-rc.2.22451.11/Microsoft.NETCore.App.deps.json doesn't have a runtimes section like:

  "runtimes": {
    "fedora.39-x64": [
      "fedora.39",
      "fedora-x64",
      "fedora",
      "linux-x64",
      "linux",
      "unix-x64",
      "unix",
      "any",
      "base"
    ]
  }

That is tracked by this issue, right?

If we fix it, we should have all the bits we need to make .NET 7 source-build work on unknown distros.

@ericstj
Copy link
Member Author

ericstj commented Sep 7, 2022

doesn't have a runtimes section
That is tracked by this issue, right?

It should be. You do see that runtime listed in the runtime.json in the built Microsoft.NETCore.Platforms?

If so I believe all that's needed to fix this is to plumb the generated runtime.json into the shared framework build.
If not - then there might be more needed to plumb the new runtime into the RID graph generation process.

@NikolaMilosavljevic
Copy link
Member

@MichaelSimons

@tmds
Copy link
Member

tmds commented Sep 8, 2022

It should be. You do see that runtime listed in the runtime.json in the built Microsoft.NETCore.Platforms?

Yes, it's in the package runtime.json file.

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Sep 21, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Sep 23, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Sep 27, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Oct 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.