Sitelet https://github.com/dotnet/runtime/pull/133115
Skip to content

Use project references for shared framework pack inputs - #133115

Open
jkoritzinsky wants to merge 11 commits into
dotnet:mainfrom
jkoritzinsky:jkoritzinsky-sfxproj-project-references
Open

jkoritzinsky wants to merge 11 commits into
dotnet:mainfrom
jkoritzinsky:jkoritzinsky-sfxproj-project-references

Conversation

@jkoritzinsky

Copy link
Copy Markdown
Member

Summary

  • derive Microsoft.NETCore.App managed library and generator inputs from NetCoreAppLibrary.props through project references
  • preserve shared-framework shims such as mscorlib and the existing analyzer package layout
  • rebuild libs.native through build-native.proj before liveBuilds.targets harvests native files, while deduplicating combined libs.native+packs builds

Validation

  • built CoreCLR, Mono, and NativeAOT product packs
  • built libs.native+packs.product and confirmed one build-native.proj execution
  • generated the runtime zip and tar.gz archives through the bundle archive target
  • confirmed reference packages are byte-identical, runtime package inventories are unchanged, and zip/tar payloads match

Note

This PR description was generated by GitHub Copilot.

Derive managed shared framework and generator inputs from NetCoreAppLibrary.props, and rebuild native library inputs through build-native.proj before packaging.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a02d8ce4-72ef-40df-b3d2-8bea5d31d7a4
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

It changes core packaging/build-graph mechanics across multiple MSBuild entry points, so a maintainer should validate the end-to-end pack composition and build ordering implications.

Review tier: Lite
Findings: 1 Medium severity

New issues introduced by this change (1)
Severity Finding
Medium severity src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Ref.sfxproj — The generator projects are multi-targeted (e.g., netstandard2.0 +…
What changed in this PR

This PR refactors how the shared-framework (SFX) packs discover their managed-library and generator inputs by deriving them from NetCoreAppLibrary.props via project references, and adjusts the build graph so libs.native is built before native files are harvested for packs.

Changes:

  • Replace ad-hoc project globbing in sfx-src.proj / sfx-gen.proj with a shared NetCoreAppLibraryProjectReferences.props import that produces the project reference item lists.
  • Update Microsoft.NETCore.App runtime/ref pack projects to consume managed library outputs (and generator analyzer files) via project references/MSBuild harvesting instead of local-build artifact globbing.
  • Split eng/liveBuilds.targets runtime file harvesting into managed vs native targets and tweak subset build properties to ensure correct runtime-flavor propagation and avoid duplicate build-native.proj executions.
File Description
src/​libraries/​sfx-src.proj Switch traversal inputs to @(NetCoreAppLibrarySourceProject) from the shared props import.
src/​libraries/​sfx-gen.proj Switch traversal inputs to @(NetCoreAppLibraryGeneratorProject) from the shared props import.
src/​libraries/​NetCoreAppLibraryProjectReferences.props New shared logic to compute library/generator ProjectReference item lists (incl. shim validation).
src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Runtime.props Add project references to build-native.proj and sfx-src.proj; derive runtime assemblies from resolved project outputs.
src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Runtime.CoreCLR.sfxproj Use resolved project outputs for WASM framework R2R inputs and move -r closure to a response file.
src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Ref.sfxproj Derive reference/doc inputs from resolved project outputs and harvest analyzer files via GetAnalyzerPackFiles.
eng/​Subsets.props Pass runtime-flavor properties through subset build items (including libs.native and pack builds).
eng/​liveBuilds.targets Separate managed vs native library runtime harvesting and recompose ResolveLibrariesRuntimeFilesFromLocalBuild.

Comment thread src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Ref.sfxproj Outdated
Avoid building every target framework for generator project references while preserving the existing ref pack analyzer layout.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a02d8ce4-72ef-40df-b3d2-8bea5d31d7a4
Copilot AI review requested due to automatic review settings September 2, 2026 20:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The new native-artifact existence enforcement in ResolveLibrariesRuntimeFilesFromLocalBuild risks breaking existing managed-only consumers unless they’re updated to use the new managed-only target (or the enforcement is made opt-in).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 1 High severity

New issues introduced by this change (1)
Severity Finding
High severity eng/​liveBuilds.targets — EnsureLocalLibrariesNativeArtifactsExist now hard-errors when $(LibrariesNativeArtifactsPath)…
Issues resolved since last review (1)
Severity Finding
Medium severity src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Ref.sfxproj — The generator projects are multi-targeted (e.g., netstandard2.0 +… View resolved comment

Comment thread eng/liveBuilds.targets
Use the generator project references to collect analyzer files and preserve no-build and Visual Studio packing from existing outputs.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a02d8ce4-72ef-40df-b3d2-8bea5d31d7a4
Copilot AI review requested due to automatic review settings September 2, 2026 22:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The ref-pack analyzer packaging currently hardcodes the analyzer target path and can break the existing analyzer folder layout (e.g., Roslyn-versioned paths) by ignoring PackagePath.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 1 High severity · 1 Medium severity

New issues introduced by this change (1)
Severity Finding
Medium severity src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Ref.sfxproj — The analyzer packaging path is currently hardcoded to analyzers/dotnet/cs/%(...Culture) for…
Pre-existing issues (1)
Severity Finding
High severity eng/​liveBuilds.targets — EnsureLocalLibrariesNativeArtifactsExist now hard-errors when $(LibrariesNativeArtifactsPath)… View comment

Comment thread src/installer/pkg/sfx/Microsoft.NETCore.App/Microsoft.NETCore.App.Ref.sfxproj Outdated
Derive generator assemblies, symbols, and resources uniformly from project reference target paths.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a02d8ce4-72ef-40df-b3d2-8bea5d31d7a4
Copilot AI review requested due to automatic review settings September 3, 2026 17:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The ref pack now hardcodes analyzer TargetPath placement, which risks diverging from the existing GeneratorProjectBaseTargetPath-driven analyzer layout used elsewhere.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 1 High severity · 1 Medium severity

New issues introduced by this change (1)
Severity Finding
Medium severity src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Ref.sfxproj — The analyzer/generator files are being packed to a hardcoded analyzers/dotnet/cs/ path. Elsewhere…
Pre-existing issues (1)
Severity Finding
High severity eng/​liveBuilds.targets — EnsureLocalLibrariesNativeArtifactsExist now hard-errors when $(LibrariesNativeArtifactsPath)… View comment
Issues resolved since last review (1)
Severity Finding
Medium severity src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Ref.sfxproj — The analyzer packaging path is currently hardcoded to analyzers/dotnet/cs/%(...Culture) for… View resolved comment

Avoid requiring libs.native artifacts for NativeAOT tracing references and WebAssembly P/Invoke table generation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a02d8ce4-72ef-40df-b3d2-8bea5d31d7a4
Copilot AI review requested due to automatic review settings September 3, 2026 19:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

It changes core build/packaging dependency flow across multiple MSBuild entrypoints (packs, live builds, wasm/NativeAOT test hooks), so it should be validated by a maintainer with CI results before approval.

Review tier: Lite
Findings: 1 Medium severity

Pre-existing issues (1)
Severity Finding
Medium severity src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Ref.sfxproj — The analyzer/generator files are being packed to a hardcoded analyzers/dotnet/cs/ path. Elsewhere… View comment
Issues resolved since last review (1)
Severity Finding
High severity eng/​liveBuilds.targets — EnsureLocalLibrariesNativeArtifactsExist now hard-errors when $(LibrariesNativeArtifactsPath)… View resolved comment

Build CoreLib and NativeAOT managed assemblies through project references, extract MIBc merging into a dedicated producer, and let shared framework R2R process CoreLib consistently.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a02d8ce4-72ef-40df-b3d2-8bea5d31d7a4
Copilot AI review requested due to automatic review settings September 9, 2026 01:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

It makes broad, cross-cutting changes to build/pack orchestration (project references, artifact resolution, and MIBc production) that warrant final human review for packaging/layout and build graph regressions.

Review tier: Lite
Findings: 1 Medium severity · 2 Low severity

New issues introduced by this change (2)
Severity Finding
Low severity src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Runtime.CoreCLR.sfxproj — Remove the empty <ItemGroup> block; it doesn't carry any items and makes the project file harder to…
Low severity src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Runtime.Mono.sfxproj — Remove the empty <ItemGroup> block; it adds noise and can mislead readers into thinking project…
Pre-existing issues (1)
Severity Finding
Medium severity src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Ref.sfxproj — The satellite resource assembly glob uses *\... which does not populate %(RecursiveDir). As a… View comment

Use a recursive generator resource glob and remove empty runtime pack item groups.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a02d8ce4-72ef-40df-b3d2-8bea5d31d7a4
Copilot AI review requested due to automatic review settings September 9, 2026 04:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

eng/codeOptimization.targets can enable PGO embedding while producing no .mibc input when UseMibcProjectReference is disabled, which risks invalid/ineffective R2R compilation settings.

Review tier: Lite
Findings: None

Issues resolved since last review (3)
Severity Finding
Low severity src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Runtime.Mono.sfxproj — Remove the empty <ItemGroup> block; it adds noise and can mislead readers into thinking project… View resolved comment
Low severity src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Runtime.CoreCLR.sfxproj — Remove the empty <ItemGroup> block; it doesn't carry any items and makes the project file harder to… View resolved comment
Medium severity src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Ref.sfxproj — The satellite resource assembly glob uses *\... which does not populate %(RecursiveDir). As a… View resolved comment
Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

eng/codeOptimization.targets:55

  • In AddReadyToRunPgoOptions, PublishReadyToRunPgoFiles is now sourced only from @(MergedMibcFile). If someone disables UseMibcProjectReference (or it's false in a given build), IncludeMibcFilesInReadyToRun can still become true, which will enable PGO embedding flags but provide no .mibc input file. Consider adding a fallback to the traditional StandardOptimizationData.mibc path when UseMibcProjectReference != true, or otherwise gating PGO inputs on that switch.

Let all runtime packs rely on the condition in codeOptimization.targets instead of forcing the project reference locally.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a02d8ce4-72ef-40df-b3d2-8bea5d31d7a4
Copilot AI review requested due to automatic review settings September 14, 2026 18:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Three unresolved findings remain, including one critical packaging issue.

Get a fresh assessment by requesting another Copilot review.

Review tier: Lite
Findings: 1 High severity · 1 Medium severity

Open findings (2)
Previously missed findings (1)

In code that hasn't changed since last review

Medium severity CoreCLR runtime pack drops CoreLib symbol files

src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Runtime.CoreCLR.sfxproj:62

When UseRuntimeManagedProjectReferences is true, eng/liveBuilds.targets removes the old CoreCLRArtifactsPdbDir entries for System.Private.CoreLib.pdb and System.Private.CoreLib.ni.pdb, so this target must add them. _RuntimeCoreLibFile is only the project output and does not include those separately located PDBs; as written, CoreCLR runtime packs lose these symbols compared with the artifact-based path. Include the existing CoreCLR PDB files alongside the project output before packaging.

Treat project-produced CoreLib as a native runtime-pack asset on every platform and include the merged MIBc project output in regular NativeAOT packs while keeping source-build excluded.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a02d8ce4-72ef-40df-b3d2-8bea5d31d7a4
Copilot AI review requested due to automatic review settings September 17, 2026 18:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Artifact override handling and CoreCLR symbol/MIBC packaging gaps remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Preserve CoreLib PDBs in project-reference runtime packs

src/​installer/​pkg/​sfx/​Microsoft.NETCore.App/​Microsoft.NETCore.App.Runtime.CoreCLR.sfxproj:63

With UseRuntimeManagedProjectReferences=true, this is now the only place that adds CoreCLR's managed CoreLib to RuntimeFiles, but it adds only the DLL. src/coreclr/Directory.Build.targets:6-15 moves the CoreLib PDB to $(RuntimeBinDir)PDB, and the old eng/liveBuilds.targets:123-128 included both System.Private.CoreLib.pdb and System.Private.CoreLib.ni.pdb; symbol-enabled CoreCLR runtime packs therefore omit those files. Add the existing PDB paths, when present, as native RuntimeFiles alongside the DLL.

Comment thread eng/liveBuilds.targets
@@ -1,19 +1,7 @@
<Project>
<!-- Accept override paths for live artifacts. -->
<!-- Set up live asset paths. -->
Resolve the crossgen CoreLib MIBc conflict by keeping the project-produced MIBc item while preserving main's WebAssembly PGO embedding behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a02d8ce4-72ef-40df-b3d2-8bea5d31d7a4
DependsOnTargets="ResolveProjectReferences">
<ItemGroup>
<RuntimeFiles Include="@(_RuntimeCoreLibFile)"
IsNative="true" />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this change the package layout for corelib? I think corelib isn't supposed to go in the native dir right?

Comment thread eng/liveBuilds.targets
Condition="Exists('$(CoreCLRArtifactsPath)System.Private.CoreLib.dll')" />
<_systemPrivateCoreLib Include="$(CoreCLRArtifactsPath)IL/System.Private.CoreLib.dll"
Condition="Exists('$(CoreCLRArtifactsPath)IL/System.Private.CoreLib.dll') and '@(_systemPrivateCoreLib)' == ''" />
<RuntimeFiles Include="@(_systemPrivateCoreLib)" />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it true that we now crossgen corelib twice, once during crossgen-corelib, and once during packaging? And we test the crossgen-corelib output but not the one that ends up in the package? I wonder if there's a way we can keep sharing the crossgen step for corelib.

<PublishReadyToRunContainerFormat>wasm</PublishReadyToRunContainerFormat>
<PublishReadyToRunEmitSymbols>false</PublishReadyToRunEmitSymbols>
<_PublishReadyToRunCrossgen2ExtraArgs>--opt-cross-module:*;--codegenopt:JitWasmNyiToR2RUnsupported=1;--codegenopt:JitWasmSimdNyiToR2RUnsupported=1</_PublishReadyToRunCrossgen2ExtraArgs>
<_PublishReadyToRunCrossgen2ExtraArgs>$(_PublishReadyToRunCrossgen2ExtraArgs);--opt-cross-module:*;--codegenopt:JitWasmNyiToR2RUnsupported=1;--codegenopt:JitWasmSimdNyiToR2RUnsupported=1</_PublishReadyToRunCrossgen2ExtraArgs>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copilot thinks this could end up using different args for crossgen in debug/checked builds, specifically it seems to omit --verify-type-and-field-layout, --eonable-cached-interface-dispatch-support, and for browser, --verify-gc-mode-transitions.

<ItemGroup>
<ProjectReference Include="$(LibrariesProjectRoot)sfx-src.proj"
AdditionalProperties="RuntimeFlavor=$(_NetCoreAppLibrariesRuntimeFlavor)"
GlobalPropertiesToRemove="RuntimeIdentifier;TargetFramework"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did you validate that this doesn't cause duplicate builds of sfx-src.proj due to global props? I think the reference through sfx.proj has a different set.

SkipGetTargetFrameworkProperties="true"
OutputItemType="_NetCoreAppLibrarySourceAssembly" />
<ProjectReference Include="@(NetCoreAppLibraryGeneratorProject)"
GlobalPropertiesToRemove="RuntimeIdentifier"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same question for this one

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants