Skip to content

Add regression test for exclude filter on non-normalized TFMs - #1131

Merged
mtsfoni merged 3 commits into
CycloneDX:masterfrom
mgnslndh:fix/issue-1126
Oct 11, 2026
Merged

mtsfoni merged 3 commits into
CycloneDX:masterfrom
mgnslndh:fix/issue-1126

Conversation

@mgnslndh

Copy link
Copy Markdown
Contributor

This PR adds a regession test for the issue described in #1126. A fix is strictly not necessary since a recent change from NuGet.ProjectModel 7.0.1 to 7.6.0 solved the issue.

With NuGet.ProjectModel 7.0.1, LockFileTarget.Name was normalized (e.g. "net48" -> ".NETFramework,Version=v4.8") and no longer matched the raw projectFileDependencyGroups keys. No package was marked as a direct reference, so --exclude-filter removed every package as an orphan.

NuGet.ProjectModel 7.6.0 keeps the raw target name, which resolves the issue. Add a functional test using a real net48 project.assets.json to guard against a regression.

The test fails against 7.0.1 with 0 of 10 components remaining.

…ycloneDX#1126)

With NuGet.ProjectModel 7.0.1, LockFileTarget.Name was normalized (e.g.
"net48" -> ".NETFramework,Version=v4.8") and no longer matched the raw
projectFileDependencyGroups keys. No package was marked as a direct
reference, so --exclude-filter removed every package as an orphan.

NuGet.ProjectModel 7.6.0 keeps the raw target name, which resolves the
issue. Add a functional test using a real net48 project.assets.json to
guard against a regression. The test fails against 7.0.1 with 0 of 10
components remaining.

Signed-off-by: Magnus Lindhe <magnus@lindhe.org>
@mgnslndh
mgnslndh requested a review from a team as a code owner September 25, 2026 18:36
@mgnslndh

Copy link
Copy Markdown
Contributor Author

Question: should direct-reference matching stop depending on NuGet's target name format?

This PR only adds a regression test. The bug itself was fixed by the NuGet.ProjectModel 7.0.1 → 7.6.0 update in 52ea46d. I'd like your advice on whether we should also change the matching logic.

Background. ProjectAssetsFileService.GetDotnetDependencys finds a target's direct references by comparing strings:

.Where(f => f.FrameworkName == targetRuntime.Name)

ProjectFileDependencyGroup.FrameworkName is the raw key from project.assets.json, which is the project's target alias (net48, Net8.0, …). Whether LockFileTarget.Name matches it depends on NuGet's reader:

TFM 7.0.1 LockFileTarget.Name 7.6.0 LockFileTarget.Name
net48 .NETFramework,Version=v4.8 net48
netstandard2.0 .NETStandard,Version=v2.0 netstandard2.0
Net8.0 net8.0 Net8.0
net8.0-windows10.0.19041.0 net8.0-windows10.0.19041 net8.0-windows10.0.19041.0
net8.0 + RID net8.0/win-x64 net8.0/win-x64

So the current code works because of how 7.6.0 happens to read names. RID targets never match under either version. That has no visible effect today, because the plain target's copy of each package is the one kept.

Proposed change:

.Where(f => string.Equals(f.FrameworkName, targetFramework.TargetAlias, StringComparison.OrdinalIgnoreCase)
            || NuGetFramework.Parse(f.FrameworkName).Equals(targetRuntime.TargetFramework))
  • It matches on the target alias from PackageSpec, which is what the group keys are. This also covers RID targets and custom aliases.
  • It falls back to comparing parsed frameworks, for older assets files whose keys look like .NETStandard,Version=v2.1. The existing tests cover that format.

I have this change ready with a unit test that builds LockFile objects with names that differ from the group keys (6 cases). On current master all 6 fail; with the change the full suite passes on net8.0, net9.0 and net10.0. I also applied the change on top of NuGet 7.0.1: every case that dropped to 0 components kept its full list.

Questions:

  1. Is it worth making the matching robust like this, or is relying on the 7.6.0 behavior acceptable?
  2. If yes, should it go into this PR or a separate one?

This does not fix #1130, where RID-only packages are removed by --exclude-filter. That's a separate problem in how packages from different targets are merged.

Signed-off-by: Magnus Lindhe <magnus@lindhe.org>
@uo-uhbc

uo-uhbc commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Q1: yes, make the matching robust — and I can confirm the premise empirically. I ran your regression test against the same net48 assets file with NuGet.ProjectModel pinned both ways: 10 components on 7.6.0, 0 on 7.0.1 (Assert.Equal() Failure: Expected 10, Actual 0, both tests failing). So the correctness of the whole component list currently rests on how a transitive dependency happens to format a string. #1122 is open to take it to 7.9.0 and touches only Directory.Packages.props and packages.lock.json — nothing in it checks the format.

I also applied your proposed matching and tested it: at 7.0.1, where master gives 0 of 10, both tests pass and the full suite is 220 passed / 0 failed / 2 skipped; at 7.6.0 it is 220 / 0 / 2 on net8.0, net9.0 and net10.0. It removes the dependency on NuGet's name format and regresses nothing, so I would say it is worth doing.

Q2: separate PR. This one is mergeable and valuable on its own.

I have corrected my comment on #1126 and the related passage in #1133 — I had generalised from a net10.0 repro, which matches under both NuGet versions and so can never reach the path you describe. Your analysis holds; mine never tested a TFM where it could fail.

Your RID observation also reproduces for me: adding <RuntimeIdentifier>win-x64</RuntimeIdentifier> gives "targets": ["net10.0", "net10.0/win-x64"] against "projectFileDependencyGroups": ["net10.0"] with no change in output, as you said.

Last thing: ExcludeFilter_RemovesOnlyExcludedPackageAndItsOrphans pins something worth pinning — the cascade is the sweep's intent, not a bug. That is the right frame for #1130 and #1133 too: the problem there isn't that the sweep cascades, it is that it walks a graph the tool already knows is incomplete. Agreed this PR doesn't touch #1130 — I measured a fix for that one on #1129 and commented there.

Signed-off-by: Michael Tsfoni <80639729+mtsfoni@users.noreply.github.com>
@mtsfoni
mtsfoni merged commit 2e90504 into CycloneDX:master Oct 11, 2026
14 checks passed
This was referenced Oct 12, 2026
Ank-KhoaHo pushed a commit to Ank-KhoaHo/DocToolkit that referenced this pull request Oct 16, 2026
Updated [cyclonedx](https://github.com/CycloneDX/cyclonedx-dotnet) from
6.2.0 to 6.3.0.

<details>
<summary>Release notes</summary>

_Sourced from [cyclonedx's
releases](https://github.com/CycloneDX/cyclonedx-dotnet/releases)._

## 6.3.0

## Added

- **NuGet purls for project references** (#​1137, fixes #​1136):
combining `--set-nuget-purl` with `--include-project-references`
generates missing project purls. Existing purls and custom BOM
references are preserved, and dependency edges use the final references.

## Changed

- **Development dependency scopes** (#​1138): development dependencies
and their development-only transitive dependencies are included by
default with `scope="excluded"`. Packages shared with production runtime
use remain `required`.
- **`--exclude-dev` is deprecated but continues to filter**: it omits
development-only and test-project-only packages while retaining packages
shared with production runtime use.

## Fixed

- **Assembly-name resolution** (#​1128, fixes #​917): resolve
`$(RootNamespace)` in assembly names, including nested
`$(MSBuildProjectName)` tokens. Missing, empty, or circular
`$(AssemblyName)`-referencing namespaces fall back to the project
filename. This is best-effort substitution, not full MSBuild evaluation.
- **Exclusion filtering** (#​1135, fixes #​1133): `--exclude-filter`
removes only packages it newly orphans; pre-existing orphans are
retained.
- **Target-framework regression coverage** (#​1131, fixes #​1126):
verifies exclusion filtering for non-normalized target frameworks,
including `net48`, `netstandard2.0`, `Net8.0`, and Windows-qualified
TFMs. The underlying NuGet.ProjectModel 7.6.0 fix was already included
in 6.2.1.

## Contributors

Thanks to everyone who contributed code, tests, and issue reports to
this release:

- @​MichaelKopfMkf — NuGet purls for project references (#​1137,
#​1136).
- @​mtsfoni — development dependency scopes and compatible exclusion
behavior (#​1138).
- @​uo-uhbc — exclusion-filter orphan handling fix and reproduction
(#​1135, #​1133).
- @​mgnslndh — regression coverage for exclusion filtering on
non-normalized target frameworks (#​1131).
- @​Eljees — root-namespace assembly-name resolution fix (#​1128).
- @​TobiDimmel — reported the assembly-name property resolution issue
(#​917).
- @​jasonlhills — reported the exclusion-filter failure (#​1126).

## 🎉 New Contributors

A warm welcome and a big thank you to our first-time contributors:

- @​MichaelKopfMkf made their first contribution in
[#​1137](CycloneDX/cyclonedx-dotnet#1137).
- @​mgnslndh made their first contribution in
[#​1131](CycloneDX/cyclonedx-dotnet#1131).
- @​Eljees made their first contribution in
[#​1128](CycloneDX/cyclonedx-dotnet#1128).

**Full changelog:**
CycloneDX/cyclonedx-dotnet@v6.2.1...v6.3.0

## 6.2.1

## CycloneDX for .NET 6.2.1

This patch release fixes dependency resolution in multi-project
solutions and hash computation for uncached NuGet packages.

### Fixed

- **Resolve dependency ranges with multiple matching package versions**
(#​1113, fixes #​1110) — solution scans no longer fail with `Sequence
contains more than one matching element` when multiple merged package
versions satisfy an unresolved NuGet dependency range. The BOM now
retains a dependency edge to every matching component.
- **Fix hash computation for uncached NuGet packages** (#​1099) — reset
the package stream before computing hashes when falling back to
downloading an uncached `.nupkg`.

### Changed

- Update .NET dependencies and CI tooling.
- Upgrade Testcontainers to 4.16.0 to resolve SSH.NET security
advisories in the E2E test infrastructure. This is a test-infrastructure
update, not a runtime dependency change to the CLI.

### Documentation

- Add a [best-practices
guide](https://github.com/CycloneDX/cyclonedx-dotnet/blob/v6.2.1/docs/best-practices.md)
covering root project targeting, restore-first workflows, and BOM
metadata setup.

### Contributors

Thanks to everyone who contributed to this release:

- @​LukasChristel — fix hash computation for uncached NuGet packages
(#​1099).
- @​michael-eberhard-skidata — report and reproduction for the
multi-version dependency range failure (#​1110).
- @​mtsfoni — dependency range resolution fix and regression coverage
(#​1113).

**Full changelog:**
CycloneDX/cyclonedx-dotnet@v6.2.0...v6.2.1

Commits viewable in [compare
view](CycloneDX/cyclonedx-dotnet@v6.2.0...v6.3.0).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=cyclonedx&package-manager=nuget&previous-version=6.2.0&new-version=6.3.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This was referenced Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants