Add DependencyOnlyTargetFrameworks to eliminate System.ValueTuple from net471 NuGet graph#126773
Add DependencyOnlyTargetFrameworks to eliminate System.ValueTuple from net471 NuGet graph#126773
DependencyOnlyTargetFrameworks to eliminate System.ValueTuple from net471 NuGet graph#126773Conversation
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/3ab46313-c615-4bf8-ab41-c39ac2b60d77 Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
…irect users (Option 2) Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/55b33308-5b6c-4b57-9bdb-81b49c84b6bf Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
…ref assets shipped) Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/55b33308-5b6c-4b57-9bdb-81b49c84b6bf Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
…n packaging.targets Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/474682a7-ecbe-4a65-b384-1477530033eb Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
|
@copilot can you fix the docs, can you also update this issue with the data we gathered by trying both option 1 and option 2. All of these measurements should be part of https://github.com/dotnet/runtime/tasks/1eacb5f4-97cd-4cc7-8fc3-812fe976ef9b?session_id=3ab46313-c615-4bf8-ab41-c39ac2b60d77 |
…ng.md Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/71fa89bc-7d12-4ed7-952e-31943b7bcf62 Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
|
Tagging subscribers to this area: @dotnet/area-infrastructure-libraries |
Added the documentation in commit 02fa65e ( |
There was a problem hiding this comment.
Pull request overview
This PR updates the libraries packaging infrastructure to allow emitting NuGet dependency groups for additional TFMs (specifically net471) without shipping extra lib/ assets, eliminating unnecessary System.ValueTuple dependencies for net471 consumers while keeping package size unchanged.
Changes:
- Add
DependencyOnlyTargetFrameworkssupport ineng/packaging.targetsto emit dependency-only TFM groups (and suppressNU5128) without producinglib/outputs. - Update affected library
.csprojfiles to useDependencyOnlyTargetFrameworksand restrictSystem.ValueTuplePackageReferences to$(NetFrameworkMinimum)only. - Add documentation for
DependencyOnlyTargetFrameworksand introduceNetFrameworkWithValueTuple=net471for reuse.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/packaging.targets | Implements DependencyOnlyTargetFrameworks behavior for packing (append TFMs, suppress build output in package, suppress NU5128). |
| Directory.Build.props | Introduces NetFrameworkWithValueTuple (set to net471) to centralize the chosen dependency-only TFM. |
| docs/coding-guidelines/libraries-packaging.md | Documents DependencyOnlyTargetFrameworks usage and behavior. |
| src/libraries/System.Text.Json/src/System.Text.Json.csproj | Uses DependencyOnlyTargetFrameworks and scopes System.ValueTuple dependency to $(NetFrameworkMinimum). |
| src/libraries/System.Text.Json/ref/System.Text.Json.csproj | Scopes System.ValueTuple dependency to $(NetFrameworkMinimum) in ref project. |
| src/libraries/System.Text.Encoding.CodePages/src/System.Text.Encoding.CodePages.csproj | Uses DependencyOnlyTargetFrameworks and scopes System.ValueTuple dependency to $(NetFrameworkMinimum). |
| src/libraries/System.Linq.AsyncEnumerable/src/System.Linq.AsyncEnumerable.csproj | Uses DependencyOnlyTargetFrameworks and scopes System.ValueTuple dependency to $(NetFrameworkMinimum). |
| src/libraries/System.Linq.AsyncEnumerable/ref/System.Linq.AsyncEnumerable.csproj | Scopes System.ValueTuple dependency to $(NetFrameworkMinimum) in ref project. |
| src/libraries/System.Formats.Nrbf/src/System.Formats.Nrbf.csproj | Uses DependencyOnlyTargetFrameworks and scopes System.ValueTuple dependency to $(NetFrameworkMinimum). |
| src/libraries/System.Formats.Cbor/src/System.Formats.Cbor.csproj | Uses DependencyOnlyTargetFrameworks and scopes System.ValueTuple dependency to $(NetFrameworkMinimum). |
| src/libraries/System.Formats.Asn1/src/System.Formats.Asn1.csproj | Uses DependencyOnlyTargetFrameworks and scopes System.ValueTuple dependency to $(NetFrameworkMinimum). |
| src/libraries/Microsoft.Extensions.Options/src/Microsoft.Extensions.Options.csproj | Uses DependencyOnlyTargetFrameworks and scopes System.ValueTuple dependency to $(NetFrameworkMinimum). |
| src/libraries/Microsoft.Extensions.Logging/src/Microsoft.Extensions.Logging.csproj | Uses DependencyOnlyTargetFrameworks and scopes System.ValueTuple dependency to $(NetFrameworkMinimum). |
| src/libraries/Microsoft.Extensions.Configuration.Abstractions/src/Microsoft.Extensions.Configuration.Abstractions.csproj | Uses DependencyOnlyTargetFrameworks and scopes System.ValueTuple dependency to $(NetFrameworkMinimum). |
| src/libraries/Microsoft.Extensions.Caching.Memory/src/Microsoft.Extensions.Caching.Memory.csproj | Uses DependencyOnlyTargetFrameworks and scopes System.ValueTuple dependency to $(NetFrameworkMinimum). |
| src/libraries/Microsoft.Bcl.TimeProvider/src/Microsoft.Bcl.TimeProvider.csproj | Uses DependencyOnlyTargetFrameworks and scopes System.ValueTuple dependency to $(NetFrameworkMinimum). |
| src/libraries/Microsoft.Bcl.Memory/src/Microsoft.Bcl.Memory.csproj | Uses DependencyOnlyTargetFrameworks and scopes System.ValueTuple dependency to $(NetFrameworkMinimum). |
| src/libraries/System.Security.Cryptography.Cose/ref/System.Security.Cryptography.Cose.csproj | Scopes System.ValueTuple dependency to $(NetFrameworkMinimum) in ref project. |
🤖 Copilot Code Review — PR #126773Note This review was generated by Copilot. Holistic AssessmentMotivation: Well-justified. net471 projects consuming these packages unnecessarily pull in Approach: The chosen approach (Option 3 — Summary: ✅ LGTM. The implementation is correct, the approach is well-motivated, and the documentation is clear. One minor cosmetic nit below. All verification points checked out. Detailed Findings✅ MSBuild Logic — Correctly implementedThe core logic in
✅ No build time impactThe existing Copilot review comment about "increasing build time" is a false positive. Before this PR, ✅ ValueTuple references correctly scopedVerified all 12 affected src projects: every ✅ Completeness — All affected packages convertedAll 12 ✅ Documentation — Clear and accurateThe 💡 Duplicate comment — Minor cosmetic nitIn <!-- Suppress build output for these TFMs so no lib/ entry is added to the package. -->Line 36 applies to ✅ Interaction with existing packaging.targets logic
|
eng/packaging.targets
Outdated
| --> | ||
| <!-- Append DependencyOnlyTargetFrameworks to TargetFrameworks so the inner builds run. --> | ||
| <TargetFrameworks Condition="'$(DependencyOnlyTargetFrameworks)' != '' and '$(TargetFrameworks)' != ''">$(TargetFrameworks);$(DependencyOnlyTargetFrameworks)</TargetFrameworks> | ||
| <!-- Suppress build output for these TFMs so no lib/ entry is added to the package. --> |
There was a problem hiding this comment.
| <!-- Suppress build output for these TFMs so no lib/ entry is added to the package. --> |
…aging.targets Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/76c21081-db5d-4109-ab27-bf0c1fba1ea4 Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
eng/packaging.targets
Outdated
| </ItemGroup> | ||
|
|
||
| <!-- | ||
| Exclude DependencyOnlyTargetFrameworks TFMs from P2P TFM resolution so that test projects or |
There was a problem hiding this comment.
Probably this capability is a better fit in https://github.com/dotnet/arcade/tree/a08169b890573cfd7f949ea9062c86a4db1aab1b/src/Microsoft.DotNet.Build.Tasks.TargetFramework, but we can test it here to start.
| <!-- | ||
| Exclude DependencyOnlyTargetFrameworks TFMs from P2P TFM resolution so that test projects or | ||
| consuming projects don't accidentally resolve the dependency-only TFM instead of the shipping | ||
| NetFrameworkMinimum TFM. The dependency-only TFMs are only meaningful for NuGet packaging | ||
| (to emit an empty dependency group); they should be invisible to project references. | ||
|
|
||
| The save/restore pattern mirrors the approach used in the arcade SDK's buildMultiTargeting | ||
| targets: filter _InnerBuildProjects before GetTargetFrameworksWithPlatformFromInnerBuilds | ||
| (the P2P query target) and restore afterward so DispatchToInnerBuilds still builds them. | ||
| --> | ||
| <Target Name="ExcludeDependencyOnlyTFMsFromP2PQuery" | ||
| Condition="'$(DependencyOnlyTargetFrameworks)' != '' and '$(IsCrossTargetingBuild)' == 'true'" | ||
| BeforeTargets="GetTargetFrameworksWithPlatformFromInnerBuilds"> | ||
| <ItemGroup> | ||
| <!-- Save the full set so it can be restored after the P2P TFM query. --> | ||
| <_InnerBuildProjectsWithDependencyOnlyTFMs Include="@(_InnerBuildProjects)" /> | ||
| <!-- Build one filter item per DependencyOnly TFM, using the same AdditionalProperties | ||
| format that _ComputeTargetFrameworkItems uses for _InnerBuildProjects. --> | ||
| <_DependencyOnlyTFMItems Include="$(DependencyOnlyTargetFrameworks)" /> | ||
| <_DependencyOnlyInnerBuildFilter Include="$(MSBuildProjectFile)"> | ||
| <AdditionalProperties>TargetFramework=%(_DependencyOnlyTFMItems.Identity)</AdditionalProperties> | ||
| </_DependencyOnlyInnerBuildFilter> | ||
| <!-- Remove DependencyOnly TFMs from the inner builds visible to P2P callers. --> | ||
| <_InnerBuildProjects Remove="@(_DependencyOnlyInnerBuildFilter)" | ||
| MatchOnMetadata="AdditionalProperties" | ||
| MatchOnMetadataOptions="CaseInsensitive" /> | ||
| </ItemGroup> | ||
| </Target> | ||
|
|
||
| <Target Name="RestoreInnerBuildProjectsAfterP2PQuery" | ||
| Condition="'@(_InnerBuildProjectsWithDependencyOnlyTFMs)' != ''" | ||
| AfterTargets="GetTargetFrameworksWithPlatformFromInnerBuilds"> | ||
| <ItemGroup> | ||
| <!-- Restore the full set so DispatchToInnerBuilds still builds the DependencyOnly TFMs | ||
| (they need to compile for NuGet to generate the dependency group in the nuspec). --> | ||
| <_InnerBuildProjects Remove="@(_InnerBuildProjects)" /> | ||
| <_InnerBuildProjects Include="@(_InnerBuildProjectsWithDependencyOnlyTFMs)" /> | ||
| </ItemGroup> | ||
| </Target> | ||
|
|
There was a problem hiding this comment.
You know I was thinking we might be able to simplify even more. What if we were able to completely remove these from TargetFrameworks -- so that they are only seen by restore and nothing else. If we can find the right hook, that might be simpler. Then we don't need builds, IncludeBuildOutput, nothing. @copilot can you find the correct place to hook and the right things to modify?
There was a problem hiding this comment.
Done in commits 70549aa and 25aee97. The approach uses two hooks into the NuGet SDK's internal item groups, so dep-only TFMs are never added to TargetFrameworks and require no compilation:
-
AddDependencyOnlyTFMsToRestoreGraph(AfterTargets="_GetRestoreTargetFrameworksAsItems"): adds dep-only TFMs to_RestoreTargetFrameworkItems— the list that_GenerateProjectRestoreGraphAllFrameworksiterates to evaluate the project per-TFM. NuGet re-evaluates the project withTargetFramework=net471, finds noSystem.ValueTuple(thePackageReferenceis conditioned onNetFrameworkMinimum), and writes the correct dependencies toproject.assets.json. -
AddDependencyOnlyTFMsToPackFrameworks(AfterTargets="_WalkEachTargetPerFramework"): adds dep-only TFMs to_TargetFrameworksafter_WalkEachTargetPerFrameworkhas already invoked all inner builds for the real TFMs. ThePackTasksees them in_TargetFrameworksand generates nuspec dependency groups by readingproject.assets.json— no inner build is triggered.
This eliminates IncludeBuildOutput, _NormalizedDependencyOnlyTargetFrameworks, NU5128 suppression scope reduction, and the two P2P filtering targets.
There was a problem hiding this comment.
@copilot This won't work for RestoreUseStaticGraphEvaluation. I think we might be able to use the previous technique with ExcludeDependencyOnlyTFMsFromP2PQuery, but hook at BeforeTargets="GetTargetFrameworksWithPlatformFromInnerBuilds;DispatchToInnerBuilds" to remove from both build and p2p and don't bother adding them back. We'll need to bring back IncludeBuildOutput setting
There was a problem hiding this comment.
Done in da13fde. Reverted to the TargetFrameworks-based approach to work correctly with RestoreUseStaticGraphEvaluation. The two previous targets are replaced with a single ExcludeDependencyOnlyTFMsFromBuildAndP2P target:
<Target Name="ExcludeDependencyOnlyTFMsFromBuildAndP2P"
Condition="'$(DependencyOnlyTargetFrameworks)' != '' and '$(IsCrossTargetingBuild)' == 'true'"
BeforeTargets="GetTargetFrameworksWithPlatformFromInnerBuilds;DispatchToInnerBuilds">This removes dep-only TFMs from _InnerBuildProjects before both the P2P query and the build dispatch, and does not restore them afterward. Pack's _WalkEachTargetPerFramework still invokes them directly via MSBuild task (not through _InnerBuildProjects), so nuspec dep groups are still generated. IncludeBuildOutput=false is restored to prevent lib/ output for those TFMs during pack.
…ad of builds Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/eb6f7e0b-1d8b-47a5-9041-14462b515497 Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
… target Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/eb6f7e0b-1d8b-47a5-9041-14462b515497 Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
The docs have been updated in the latest commits to reflect the simplified restore/pack-hook approach — no mention of builds or |
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/58641b9e-4679-48a0-8d13-0bc7069613f9 Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
…jects matching Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/58641b9e-4679-48a0-8d13-0bc7069613f9 Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
net471 projects referencing these packages get
System.ValueTuplepulled into the NuGet graph via thenet462dependency group, even though net471 hasSystem.ValueTupleinbox. This adds anet471dependency group (with noSystem.ValueTuple) to the 13 packages that directly reference it, without shipping any additional assemblies and without any compilation overhead during regular builds.Description
Approach
Three options were evaluated. Size measurements are based on the 10.0.0 packages from the NuGet global cache.
Option 1 — Add
net471to all ~43 packages (transitive + direct) ❌ DiscardedEach of the 43 packages that transitively depends on
System.ValueTuplewould get alib/net471assembly added. The newnet471assemblies would be source identical to the existingnet462assemblies — pure overhead.lib/net462sizeTotal additional assembly size: ~4.1 MB (3,835 KB measured + ~300 KB estimated for System.Linq.AsyncEnumerable). 43 packages grow.
Option 2 — Add
net471only to the 13 packages with directSystem.ValueTuplereferences ❌ DiscardedNuGet resolves each package's dependency group independently, so fixing only the 13 packages that directly reference
System.ValueTupleis sufficient for net471 consumers. However, each of the 13 packages would still need alib/net471assembly — again source identical to thelib/net462assembly.lib/net462size (= addedlib/net471size)Total additional assembly size: ~2.4 MB (2,102 KB measured + ~300 KB estimated for System.Linq.AsyncEnumerable). 13 packages grow.
Option 3 (chosen) —
DependencyOnlyTargetFrameworks✅Same NuGet graph fix as Option 2 (a
net471dependency group is emitted in the nuspec for each of the 13 packages), but nolib/net471assembly is added. Consumers fall back tolib/net462for the assembly. Total additional assembly size: 0 bytes. No packages grow. Dep-only TFMs are not compiled during regular builds.Changes
eng/packaging.targets— IntroducesDependencyOnlyTargetFrameworks: a general-purpose semicolon-separated property for TFMs that should emit a NuGet dependency group but ship nolib/assembly. When set:TargetFrameworksso NuGet restore (includingRestoreUseStaticGraphEvaluation) and pack both see them._NormalizedDependencyOnlyTargetFrameworks+IncludeBuildOutput=false(delimiter-based, case-insensitive) suppresseslib/output for dep-only TFM inner builds invoked during pack.NU5128suppressed (intentional dep-group/lib mismatch — consumers fall back to the nearest compatiblelib/entry).ExcludeDependencyOnlyTFMsFromBuildAndP2P(BeforeTargets="GetTargetFrameworksWithPlatformFromInnerBuilds;DispatchToInnerBuilds"): removes dep-only TFMs from_InnerBuildProjectsbefore both the P2P TFM query (so consuming projects don't accidentally resolve a dep-only TFM) and the outer build dispatch (so dep-only TFMs are not compiled during regulardotnet build). The removed TFMs are not restored — pack's_WalkEachTargetPerFrameworkstill invokes their inner builds directly via MSBuild task (not through_InnerBuildProjects), generating the nuspec dependency groups fromproject.assets.json.Directory.Build.props— IntroducesNetFrameworkWithValueTuple=net471as a repo-wide property.13 src
.csprojfiles — Add aDependencyOnlyTargetFrameworksproperty set to$(NetFrameworkWithValueTuple)and restrictSystem.ValueTuplePackageReferenceto$(NetFrameworkMinimum)only. Affected packages:Microsoft.Bcl.Memory,Microsoft.Bcl.TimeProvider,Microsoft.Extensions.Caching.Memory,Microsoft.Extensions.Configuration.Abstractions,Microsoft.Extensions.Logging,Microsoft.Extensions.Options,System.Formats.Asn1,System.Formats.Cbor,System.Formats.Nrbf,System.Linq.AsyncEnumerable,System.Security.Cryptography.Cose,System.Text.Encoding.CodePages,System.Text.Jsondocs/coding-guidelines/libraries-packaging.md— Added aDependencyOnlyTargetFrameworkssection documenting the feature: what it is, when to use it, a code example using$(NetFrameworkWithValueTuple), and how theeng/packaging.targetsinfrastructure implements it.Result
For each affected package, the nuspec gains a
.NETFramework4.7.1dependency group withoutSystem.ValueTuple. net471 consumers resolve noSystem.ValueTupledependency and fall back tolib/net462for the assembly. No new assemblies ship; package sizes are unchanged. Dep-only TFMs are excluded from_InnerBuildProjectsso they are never compiled during regular builds and never selected by P2P resolution. The approach works correctly with both standard and static-graph (RestoreUseStaticGraphEvaluation) restore.DependencyOnlyTargetFrameworksto 13 affected .csproj filesNetFrameworkWithValueTupleinDirectory.Build.propsDependencyOnlyTargetFrameworksineng/packaging.targetsviaTargetFrameworksaugmentation +ExcludeDependencyOnlyTFMsFromBuildAndP2Ptarget +IncludeBuildOutput=false(compatible withRestoreUseStaticGraphEvaluation)DependencyOnlyTargetFrameworksTFMs from P2P TFM resolution and regular build dispatchDependencyOnlyTargetFrameworksdocumentation todocs/coding-guidelines/libraries-packaging.md