Skip to content

Use project references for shared framework pack inputs - #133115

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

jkoritzinsky wants to merge 10 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. -->

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.

2 participants