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

Use generated runtime.json when building shared framework packages. #76068

Merged
merged 6 commits into from
Sep 27, 2022

Conversation

tmds
Copy link
Member

@tmds tmds commented Sep 23, 2022

Fixes #53550
Based on #69455

With this change, when I build:

./build.sh /p:DotnetBuildFromSource=true /p:AdditionalRuntimeIdentifiers=fedora.45-x64

The new rid shows up in the packages:

artifacts/obj/Microsoft.NETCore.App.Bundle/Debug/net7.0/linux-x64/output/shared/Microsoft.NETCore.App/8.0.0-dev/Microsoft.NETCore.App.deps.json:    "fedora.45-x64": [
artifacts/obj/Microsoft.NETCore.App.Composite.Bundle/Debug/net7.0/linux-x64/output/shared/Microsoft.NETCore.App/8.0.0-dev/Microsoft.NETCore.App.deps.json:    "fedora.45-x64": [
artifacts/obj/Microsoft.NETCore.App.Runtime.Composite/Debug/net7.0/linux-x64/Microsoft.NETCore.App.deps.json:    "fedora.45-x64": [
artifacts/obj/Microsoft.NETCore.App.Runtime.Composite/Debug/net7.0/linux-x64/output/shared/Microsoft.NETCore.App/8.0.0-dev/Microsoft.NETCore.App.deps.json:    "fedora.45-x64": [
artifacts/obj/Microsoft.NETCore.App.Runtime/Debug/net7.0/linux-x64/Microsoft.NETCore.App.deps.json:    "fedora.45-x64": [
artifacts/obj/Microsoft.NETCore.App.Runtime/Debug/net7.0/linux-x64/output/shared/Microsoft.NETCore.App/8.0.0-dev/Microsoft.NETCore.App.deps.json:    "fedora.45-x64": [
artifacts/bin/Microsoft.NETCore.Platforms/Debug/runtime.json:    "fedora.45-x64": {

@ericstj @ViktorHofer ptal.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Sep 23, 2022
@ghost
Copy link

ghost commented Sep 23, 2022

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #53550
Based on #69455

With this change, when I build:

./build.sh /p:DotnetBuildFromSource=true /p:AdditionalRuntimeIdentifiers=fedora.45-x64

The new rid shows up in the packages:

artifacts/obj/Microsoft.NETCore.App.Bundle/Debug/net7.0/linux-x64/output/shared/Microsoft.NETCore.App/8.0.0-dev/Microsoft.NETCore.App.deps.json:    "fedora.45-x64": [
artifacts/obj/Microsoft.NETCore.App.Composite.Bundle/Debug/net7.0/linux-x64/output/shared/Microsoft.NETCore.App/8.0.0-dev/Microsoft.NETCore.App.deps.json:    "fedora.45-x64": [
artifacts/obj/Microsoft.NETCore.App.Runtime.Composite/Debug/net7.0/linux-x64/Microsoft.NETCore.App.deps.json:    "fedora.45-x64": [
artifacts/obj/Microsoft.NETCore.App.Runtime.Composite/Debug/net7.0/linux-x64/output/shared/Microsoft.NETCore.App/8.0.0-dev/Microsoft.NETCore.App.deps.json:    "fedora.45-x64": [
artifacts/obj/Microsoft.NETCore.App.Runtime/Debug/net7.0/linux-x64/Microsoft.NETCore.App.deps.json:    "fedora.45-x64": [
artifacts/obj/Microsoft.NETCore.App.Runtime/Debug/net7.0/linux-x64/output/shared/Microsoft.NETCore.App/8.0.0-dev/Microsoft.NETCore.App.deps.json:    "fedora.45-x64": [
artifacts/bin/Microsoft.NETCore.Platforms/Debug/runtime.json:    "fedora.45-x64": {

@ericstj @ViktorHofer ptal.

Author: tmds
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

Directory.Build.props Outdated Show resolved Hide resolved
Directory.Build.props Outdated Show resolved Hide resolved
eng/liveBuilds.targets Outdated Show resolved Hide resolved
@@ -223,6 +223,8 @@
ResolveLibrariesRuntimeFilesFromLocalBuild" />

<PropertyGroup>
<BundledRuntimeIdentifierGraphFile>$(RuntimeIdGraphDefinitionFile)</BundledRuntimeIdentifierGraphFile>
<!-- Keep in sync with outputs defined in Microsoft.NETCore.Platforms.csproj. -->
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be in the .targets file otherwise Microsoft.NET.Sdk overwrites it.

Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good now. We just need to make sure that the package is correctly produced in the all configurations leg. Can you verify that offline as we don't upload packages in PRs afaik because of size constraints.

You would first do a build.sh libs and check that Microsoft.NetCore.Platforms package doesn't get produced but that the library is built and then do a build.sh libs -allconfigurations and check that the Microsoft.NetCore.Platforms package exists under artifacts/packages/...

EDIT: Doing this kind of validation myself right now, just to be sure.
EDIT2: Works as expected. Nice!

@ViktorHofer
Copy link
Member

My mistake. I mentioned that the <GeneratePackageOnBuild Condition="'$(BuildAllConfigurations)' != 'true'">false</GeneratePackageOnBuild> statement is required in the Platforms.csproj project to avoid package clashes but we only ever build packages in the allconfigurations leg. So that statement was redundant and I just removed it again. Sorry for the confusion here.

@ViktorHofer ViktorHofer merged commit cd2f0ff into dotnet:main Sep 27, 2022
@tmds
Copy link
Member Author

tmds commented Sep 27, 2022

@ViktorHofer thanks for the responsive reviewing!

ayakael added a commit to ayakael/runtime that referenced this pull request Oct 26, 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.
Labels
area-Infrastructure-libraries community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure shared framework consumes source-built RID graph
3 participants