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.
…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 |
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.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 and no compilation happens for dep-only TFMs at all. Consumers fall back tolib/net462for the assembly. Total additional assembly size: 0 bytes. No packages grow. No build time added for dep-only TFMs 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 and require no compilation. Dep-only TFMs are never added toTargetFrameworks. Instead, two targeted hooks are used:AddDependencyOnlyTFMsToRestoreGraph(AfterTargets="_GetRestoreTargetFrameworksAsItems"): adds dep-only TFMs to_RestoreTargetFrameworkItemsso NuGet evaluates the project for each dep-only TFM and writes per-TFM dependency info toproject.assets.json. Fornet471, this means noSystem.ValueTupleentry (thePackageReferenceis conditioned onNetFrameworkMinimum).AddDependencyOnlyTFMsToPackFrameworks(AfterTargets="_WalkEachTargetPerFramework"): adds dep-only TFMs to_TargetFrameworksafter all inner builds have already completed, so thePackTaskgenerates nuspec dependency groups fromproject.assets.jsonwithout triggering any inner builds for dep-only TFMs.NU5128suppressed (intentional dep-group/lib mismatch — consumers fall back to the nearest compatiblelib/entry).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 via the restore and pack hooks.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. No compilation happens for dep-only TFMs, so there is no build time overhead and no P2P resolution concern.DependencyOnlyTargetFrameworksto 13 affected .csproj filesNetFrameworkWithValueTupleinDirectory.Build.propsDependencyOnlyTargetFrameworksineng/packaging.targetsvia restore and pack hooks (noTargetFrameworksmodification, no builds, noIncludeBuildOutput, no P2P filtering needed)DependencyOnlyTargetFrameworksdocumentation todocs/coding-guidelines/libraries-packaging.md