Skip to content

Refactor targeting pack properties for unknown frameworks - #17627

Merged
ViktorHofer merged 5 commits into
mainfrom
ViktorHofer-patch-2
Sep 29, 2026
Merged

ViktorHofer merged 5 commits into
mainfrom
ViktorHofer-patch-2

Conversation

@ViktorHofer

Copy link
Copy Markdown
Member

Extracted from #17621

To double check:

Copilot AI lite review requested due to automatic review settings September 28, 2026 14:32

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 critical fallback condition can affect unrelated frameworks and alter package dependency graphs.

Review effort: Lite
Findings: 1 High severity

Open (1)
What changed in this PR

Refactors targeting-pack handling to support frameworks unknown to the installed SDK.

Changes:

  • Uses the bundled framework as the targeting-pack baseline.
  • Adds compatibility and prune-data fallback logic for newer frameworks.
  • Updates framework-version handling.
File Summary
src/​Microsoft.DotNet.Arcade.Sdk/​tools/​TargetingPacks.BeforeCommonTargets.targets Refactors targeting-pack compatibility and fallback behavior; the fallback must be scoped to the current target framework.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI review requested due to automatic review settings September 28, 2026 14:36

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

Two unresolved moderate findings affect multi-targeting behavior and framework-version compatibility.

Review effort: Lite
Findings: 1 High severity

Open (1)

Added logic to handle targeting pack requirements for unknown target frameworks.
Copilot AI review requested due to automatic review settings September 28, 2026 15:41

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

Close the unclosed comment before the property declarations.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (1)

Copilot AI review requested due to automatic review settings September 28, 2026 15:44

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

Platform-qualified current TFMs are not covered by the current equality check, potentially causing restore or build failures.

Review effort: Lite
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Support platform-qualified current TFMs in targeting pack bootstrap

src/​Microsoft.DotNet.Arcade.Sdk/​tools/​TargetingPacks.BeforeCommonTargets.targets:24

This exact equality excludes platform-qualified current TFMs such as net12.0-windows. Their inner builds leave _NeedsTargetingPackCurrentForUnknownTargetFramework unset, so the bootstrap SDK does not receive the synthesized current pack entries, maximum-version override, or pruning fallback and can still fail to restore/build; match the SDK's same-TFM-or-StartsWith(current + "-") condition instead.

Removed comments related to enabling known pack items for unknown target frameworks and updated conditions for targeting packs.
Copilot AI review requested due to automatic review settings September 28, 2026 16:00

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

Restrict the fallback condition to the current targeting-pack framework build.

Review effort: Lite
Findings: None

@wtgodbe wtgodbe left a comment

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.

LGTM! We'd have to consume this in a dependency update to unblock #17621, right?

@ViktorHofer
ViktorHofer merged commit 70e9b99 into main Sep 29, 2026
13 checks passed
@ViktorHofer
ViktorHofer deleted the ViktorHofer-patch-2 branch September 29, 2026 07:37
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 12.0-preview1 milestone Sep 29, 2026
wtgodbe pushed a commit to dotnet/dotnet that referenced this pull request Sep 29, 2026
Include the shared targeting-pack baseline, SDK maximum-version adjustment, and pruning fallback needed by downstream net12.0 consumers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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