Skip to content

Externalize findMissingNotices target framework selection with ordered Windows fallback#27269

Merged
TravisEz13 merged 4 commits intomasterfrom
copilot/update-findmissingnotices-config
Apr 15, 2026
Merged

Externalize findMissingNotices target framework selection with ordered Windows fallback#27269
TravisEz13 merged 4 commits intomasterfrom
copilot/update-findmissingnotices-config

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 15, 2026

This updates tools/findMissingNotices.ps1 to remove hard-coded TFMs and drive target selection from repo-tracked JSON config. Windows target resolution now uses full configured Windows TFMs in priority order, with explicit fallback to base target when available and no implicit Win7 target construction.

  • Config-driven target frameworks

    • Added tools/findMissingNotices.targets.json with:
      • dotnetTargetName (base/non-Windows target)
      • windowsTargetNames (ordered full Windows TFM candidates; optional/empty allowed)
  • Script-scope config loading and validation

    • Added early config load in tools/findMissingNotices.ps1.
    • Fails fast with clear errors when config is missing, malformed, wrong shape, or contains invalid entries.
  • Windows target selection logic in Get-CGRegistrations

    • Removed hard-coded $dotnetTargetName / $dotnetTargetNameWin7.
    • For Windows runtimes:
      1. Enumerate available targets from project.assets.json via New-PADrive.
      2. Select first matching entry from windowsTargetNames (in order).
      3. Fallback to dotnetTargetName if present.
      4. Throw actionable error if none match; include available targets in verbose output.
    • No longer derives/assumes legacy Win7 target names.
  • Behavior preserved for non-Windows and modules

    • Non-Windows runtimes continue to use base target from config.
    • modules runtime behavior remains unchanged (base target + linux-x64 runtime).
# Windows selection behavior (simplified)
$availableTargets = Get-ChildItem "${folder}:/targets" | Select-Object -ExpandProperty Name
$selectedTargetName = $script:windowsTargetNames | Where-Object { $_ -in $availableTargets } | Select-Object -First 1
if (-not $selectedTargetName -and $baseTargetName -in $availableTargets) { $selectedTargetName = $baseTargetName }
if (-not $selectedTargetName) { throw "Unable to find a target..." }
Original prompt

Create a pull request in PowerShell/PowerShell (base branch: master) to update tools/findMissingNotices.ps1 so that:

  1. Target frameworks are no longer hard-coded in the script (currently at https://github.com/PowerShell/PowerShell/blob/master/tools/findMissingNotices.ps1#L198).
  2. Target framework settings are moved into a new JSON config file committed to the repo (suggested path: tools/findMissingNotices.targets.json).
  3. The Windows target should be specified as full Windows TFMs in JSON (not derived from the base TFM), because the remainder of the name can vary by project.
  4. The legacy Win7 target should not be assumed or constructed; the script should treat Windows target selection as optional and attempt candidates in order, with fallback to the base (non-windows) target when appropriate.

Implementation guidance:

  • Add script-scope config loading near the top of tools/findMissingNotices.ps1 (after imports is fine). Fail with a clear error if the JSON config file is missing or invalid.
  • In Get-CGRegistrations, remove hard-coded $dotnetTargetName / $dotnetTargetNameWin7.
  • For non-Windows runtimes, continue using the base target from config.
  • For Windows runtimes, after restoring and mounting project.assets.json via New-PADrive, list available targets under "${folder}:/targets" and select the first matching TFM from windowsTargetNames (array) in the JSON file. If none match, fall back to the base target if present. If still none, throw with a helpful message and include available targets in verbose output.
  • Ensure current behavior remains for 'modules' runtime.

Deliverables:

  • New file: tools/findMissingNotices.targets.json with keys:
    • dotnetTargetName (string)
    • windowsTargetNames (array of strings; may be empty)
      Provide a sensible default based on existing hard-coded values (but do not include Win7-specific tfm like *-windows8.0 unless it truly exists).
  • Update tools/findMissingNotices.ps1 accordingly.

Please include any necessary documentation/comments in the script/config so future maintainers understand ordering/fallback behavior.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Suggest improvements to this code.

Assistant: ### 1) Smells / antipatterns in the highlighted area

The snippet is just:

$dotnetTargetName = 'net11.0'

…but in context it points to a few concrete issues:

  • Magic string / hard-coded framework moniker (TFM): net11.0 is embedded directly in the function. When the repo updates TFMs (e.g., net9.0 → net10.0), this becomes a recurring maintenance hotspot and easy to miss.
  • Duplication / inconsistency risk: You also have $dotnetTargetNameWin7 = 'net11.0-windows8.0'. These are coupled (same base TFM), but are specified separately—easy for them to drift.
  • Suspicious/likely incorrect TFM: .NET 11 is not a currently released TFM. PowerShell typically targets a released .NET (recently net8.0 / net9.0). If this is a real repo line, it’s likely a typo or a temporary experiment that will break restores in most environments.
  • Configuration buried inside function logic: Get-CGRegistrations is doing restore/asset parsing and defining what TFMs to use. That makes it harder to test and harder to reuse.

2) Concrete improvements (with working code)

Option A: Derive the TFM from the project being restored (best: avoids hard-coding)

Since you Push-Location into the project folder, you can read the project file and pull TargetFramework / TargetFrameworks.

function Get-ProjectTFM {
    param(
        [Parameter(Mandatory)]
        [string] $ProjectFile
    )

    [xml]$xml = Get-Content -Raw $ProjectFile
    $ns = New-Object System.Xml.XmlNamespaceManager($xml.NameTable)
    $ns.AddNamespace('msb', 'http://schemas.microsoft.com/developer/msbuild/2003')

    # handle SDK-style projects without XML namespace too
    $tfm  = $xml.Project.PropertyGroup.TargetFramework  | Select-Object -First 1
    $tfms = $xml.Project.PropertyGroup.TargetFrameworks | Select-Object -First 1

    if ($tfm)  { return $tfm.'#text' }
    if ($tfms) { return ($tfms.'#text' -split ';' | Select-Object -First 1) }

    throw "Could not determine TargetFramework(s) from $ProjectFile"
}

Then in Get-CGRegistrations, replace the hard-coded assignment with something like:

$projectFile = if ($folder -eq $windowsProjectName) {
    Join-Path $PSScriptRoot "..\src\$folder\powershell-win-core.csproj"
} else {
    Join-Path $PSScriptRoot "..\src\$folder\powershell-unix.csproj"
}

$dotnetTargetName = Get-ProjectTFM -ProjectFile $projectFile
$dotnetTargetNameWin7 = "$dotnetTargetName-windows8.0"

Benefits:

  • No repo-wide sweep needed when TFMs change.
  • Prevents “net11.0”/typos from silently living in tooling.

Option B: Make TFMs parameters (good for CI and local overrides)

If you want the script to be explicit and override-able:

para...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines:
5 pipeline(s) require an authorized user to comment /azp run to run.

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines:
5 pipeline(s) require an authorized user to comment /azp run to run.

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines:
5 pipeline(s) require an authorized user to comment /azp run to run.

Copilot AI changed the title [WIP] Update findMissingNotices.ps1 to use JSON config for target frameworks Externalize findMissingNotices target framework selection with ordered Windows fallback Apr 15, 2026
Copilot AI requested a review from TravisEz13 April 15, 2026 17:04
Comment thread tools/findMissingNotices.targets.json Outdated
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines:
5 pipeline(s) require an authorized user to comment /azp run to run.

@TravisEz13 TravisEz13 marked this pull request as ready for review April 15, 2026 21:16
@TravisEz13 TravisEz13 requested a review from jshigetomi as a code owner April 15, 2026 21:16
Copilot AI review requested due to automatic review settings April 15, 2026 21:16
@TravisEz13 TravisEz13 requested a review from a team as a code owner April 15, 2026 21:16
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines:
5 pipeline(s) require an authorized user to comment /azp run to run.

@TravisEz13 TravisEz13 added the CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log label Apr 15, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR moves findMissingNotices.ps1 target framework selection into a repo-tracked JSON config and updates Windows target resolution to pick the first configured Windows TFM that exists in project.assets.json, with fallback to the base target.

Changes:

  • Added tools/findMissingNotices.targets.json to define dotnetTargetName and ordered windowsTargetNames.
  • Added script-scope config loading + validation in tools/findMissingNotices.ps1.
  • Reworked Windows target selection in Get-CGRegistrations to choose a configured TFM based on available assets targets (with base-target fallback).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

File Description
tools/findMissingNotices.targets.json Introduces config-driven base/Windows TFM selection settings.
tools/findMissingNotices.ps1 Loads/validates the config and updates Windows/non-Windows target selection logic accordingly.

Comment thread tools/findMissingNotices.targets.json
Comment thread tools/findMissingNotices.targets.json
Comment thread tools/findMissingNotices.ps1
Comment thread tools/findMissingNotices.ps1
Comment thread tools/findMissingNotices.ps1
Comment thread tools/findMissingNotices.ps1
Comment thread tools/findMissingNotices.ps1
Comment thread tools/findMissingNotices.ps1
@TravisEz13 TravisEz13 merged commit 30d14ea into master Apr 15, 2026
40 of 41 checks passed
@TravisEz13 TravisEz13 deleted the copilot/update-findmissingnotices-config branch April 15, 2026 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backport-7.4.x-Consider Backport-7.5.x-Consider Backport-7.6.x-Consider CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants