From 947b17eec120768c231844719bd4e5218536b289 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Wed, 7 Oct 2026 10:07:34 -0700 Subject: [PATCH 1/2] Read a Non-Version winget Token as an Unread Installed Version install-tools.ps1 carried the first token of winget's Version column forward as the installed version, so an entry recording Unknown, or the bare < or > of winget's range forms, reached the report and the status comparison as if it were one. Get-VersionKey read it as 0, which reported such a tool as outdated, and the JSON report's installed field carried the token where its help promises null. Test-WingetVersion now decides whether a token is a version. A row carrying one that is not leaves the installed version unread, the tool reports unknown rather than outdated or multiple, and the report prints the raw row. -Install leaves an installed tool whose version does not compare alone, as it already did for an outdated one, rather than installing over it. Part of #2258. Co-Authored-By: Claude Opus 5.5 --- host-setup/windows/install-tools.ps1 | 19 +++++- tests/test_install_tools.py | 94 ++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 3 deletions(-) diff --git a/host-setup/windows/install-tools.ps1 b/host-setup/windows/install-tools.ps1 index a8d34cf09..a43d230b7 100644 --- a/host-setup/windows/install-tools.ps1 +++ b/host-setup/windows/install-tools.ps1 @@ -248,12 +248,19 @@ function Get-WingetInstalled { return , (Read-WingetTable -Text $text -Id $Id) } -# One version for a package winget lists more than once, or nothing where the rows do not describe one product. +# Whether a Version column token is a version, rather than the Unknown winget prints for an entry recording none or the bare < or > its range forms leave. +function Test-WingetVersion { + param([string]$Token) + return $Token -match '^\d' +} + +# One version for a package winget lists more than once, or nothing where the rows do not describe one product or one of them carries no version. # Rows sharing a major version are side by side builds of one product and the newest is the answer, which is what a dotnet SDK line looks like. # Rows whose majors differ are two products sharing an id, which is what the legacy WSL installer looks like beside WSL itself, and there no single version compares. function Resolve-InstalledVersion { param([string[]]$Version) if (-not $Version -or $Version.Count -eq 0) { return $null } + if (@($Version | Where-Object { -not (Test-WingetVersion $_) }).Count -gt 0) { return $null } if ($Version.Count -eq 1) { return $Version[0] } $majors = @($Version | ForEach-Object { ($_ -split '\.')[0] } | Sort-Object -Unique) if ($majors.Count -ne 1) { return $null } @@ -514,7 +521,10 @@ function Get-ToolStatus { if (-not $State.Available) { return 'unavailable' } return 'missing' } - if (-not $State.Installed) { return 'multiple' } + if (-not $State.Installed) { + if (@($State.Rows | Where-Object { -not (Test-WingetVersion $_) }).Count -gt 0) { return 'unknown' } + return 'multiple' + } if (-not $State.Available) { return 'unknown' } if ((Get-ExplicitUpgrade) -contains $State.Package) { return 'self-updating' } if ((Compare-HostVersion $State.Installed $State.Available) -ge 0) { return 'current' } @@ -1036,7 +1046,7 @@ function Show-Report { $state = Get-ToolState -Tool $record # Every row is printed only where they did not resolve to one version, since a dotnet line carrying three side by side builds resolves cleanly and listing all three would overflow the column for nothing. if (-not $script:JSON_OUTPUT) { - $installed = if ($state.Status -eq 'multiple') { $state.Rows -join ',' } elseif ($state.Installed) { $state.Installed } else { '-' } + $installed = if (-not $state.Installed -and $state.Rows.Count -gt 0) { $state.Rows -join ',' } elseif ($state.Installed) { $state.Installed } else { '-' } $available = if ($state.Available) { $state.Available } else { '-' } $scope = if ($state.Scope.Count -gt 0) { $state.Scope -join '+' } else { '-' } log ($format -f $record.Name, $installed, $available, $state.Package, $scope, $state.Status) @@ -1140,6 +1150,9 @@ function Invoke-ToolApply { } elseif ($script:MODE -eq 'install' -and $state.Status -eq 'outdated') { log "${ToolName}: at $($state.Installed), the source carries $($state.Available), -Upgrade moves it" return + } elseif ($script:MODE -eq 'install' -and $state.Status -eq 'unknown' -and $state.Rows.Count -gt 0) { + log "${ToolName}: winget lists $($state.Rows -join ', ') and no version comparison is possible, -Upgrade or -Reinstall acts on it" + return } # Reached only once real work is about to happen: every "leave it alone" status above already returned. diff --git a/tests/test_install_tools.py b/tests/test_install_tools.py index ec4f61314..712c4ef23 100755 --- a/tests/test_install_tools.py +++ b/tests/test_install_tools.py @@ -332,5 +332,99 @@ def test_json_is_refused_beside_another_action(self) -> None: self.assertIn("-Json", result.stderr) +WINGET_HARNESS = r""" +param([string]$Installer, [string]$Cases) +$ErrorActionPreference = 'Stop' +Set-StrictMode -Version Latest +$tokens = $null +$errors = $null +$ast = [System.Management.Automation.Language.Parser]::ParseFile($Installer, [ref]$tokens, [ref]$errors) +$wanted = @('Read-WingetTable', 'Test-WingetVersion', 'Resolve-InstalledVersion', 'Get-VersionKey', 'Compare-HostVersion', 'Get-ToolStatus') +foreach ($definition in $ast.FindAll({ param($node) $node -is [System.Management.Automation.Language.FunctionDefinitionAst] -and $wanted -contains $node.Name }, $true)) { + . ([scriptblock]::Create($definition.Extent.Text)) +} +function Get-ExplicitUpgrade { return @() } +$results = [ordered]@{} +foreach ($case in (Get-Content -Raw -LiteralPath $Cases | ConvertFrom-Json).PSObject.Properties) { + $rows = Read-WingetTable -Text $case.Value -Id 'jqlang.jq' + $installed = Resolve-InstalledVersion -Version $rows + $state = @{ Readable = $true; Rows = $rows; Installed = $installed; Available = '1.8.2'; Package = 'jqlang.jq' } + $status = Get-ToolStatus -Tool @{ Name = 'jq'; Probe = 'jq' } -State $state + $results[$case.Name] = [ordered]@{ rows = @($rows); installed = $installed; status = $status } +} +$results | ConvertTo-Json -Depth 4 +""" + + +def winget_list(*versions: str) -> str: + """A `winget list` table carrying one row per version for the id the harness asks about.""" + lines = [ + "Name Id Version Available Source", + "-------------------------------------------", + *(f"jq jqlang.jq {version:<8} 1.8.2 winget" for version in versions), + ] + return "\n".join(lines) + + +@unittest.skipUnless( + shutil.which("pwsh"), "needs pwsh to drive the Windows installer's own functions" +) +class TestWindowsInstalledVersion(unittest.TestCase): + """A Version column token that is not a version leaves the installed version unread.""" + + def test_a_non_version_token_is_unread_rather_than_installed(self) -> None: + cases = { + "unknown": winget_list("Unknown"), + "below": winget_list("<"), + "above": winget_list(">"), + "unknown beside a version": winget_list("Unknown", "1.8.1"), + "one version": winget_list("1.8.1"), + "majors differ": winget_list("1.8.1", "2.0.0"), + } + with tempfile.TemporaryDirectory() as directory: + harness = Path(directory, "harness.ps1") + harness.write_text(WINGET_HARNESS, encoding="utf-8") + data = Path(directory, "cases.json") + data.write_text(json.dumps(cases), encoding="utf-8") + result = subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(harness), + "-Installer", + str(WINDOWS_INSTALLER), + "-Cases", + str(data), + ], + capture_output=True, + text=True, + encoding="utf-8", + check=False, + timeout=120, + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual( + json.loads(result.stdout), + { + "unknown": {"rows": ["Unknown"], "installed": None, "status": "unknown"}, + "below": {"rows": ["<"], "installed": None, "status": "unknown"}, + "above": {"rows": [">"], "installed": None, "status": "unknown"}, + "unknown beside a version": { + "rows": ["Unknown", "1.8.1"], + "installed": None, + "status": "unknown", + }, + "one version": {"rows": ["1.8.1"], "installed": "1.8.1", "status": "outdated"}, + "majors differ": { + "rows": ["1.8.1", "2.0.0"], + "installed": None, + "status": "multiple", + }, + }, + ) + + if __name__ == "__main__": unittest.main() From 72134dd379bd4f72691a87d5b96b134f2caba79d Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Wed, 7 Oct 2026 10:12:45 -0700 Subject: [PATCH 2/2] Keep self-updating Ahead of an Unread Version, and Cover the Apply Path Get-ToolStatus returned unknown for an unread row before it checked the explicit-upgrade list, so a self-updating package whose winget row reads Unknown lost that status and -Upgrade would act on it. The unread check now sits after that list, as the base order already had it. The -Scope refusal printed an empty version for a tool with no single installed version, so it now names the rows winget listed. A second harness drives Show-Report and an -Install apply over an unread row with winget's writes stubbed, checking the INSTALLED column carries the row and that nothing is installed. Each of the three changes was reverted in turn and the tests failed for each. Part of #2258. Co-Authored-By: Claude Opus 5.5 --- host-setup/windows/install-tools.ps1 | 9 +- tests/test_install_tools.py | 125 ++++++++++++++++++++------- 2 files changed, 98 insertions(+), 36 deletions(-) diff --git a/host-setup/windows/install-tools.ps1 b/host-setup/windows/install-tools.ps1 index a43d230b7..24f5eac37 100644 --- a/host-setup/windows/install-tools.ps1 +++ b/host-setup/windows/install-tools.ps1 @@ -521,12 +521,11 @@ function Get-ToolStatus { if (-not $State.Available) { return 'unavailable' } return 'missing' } - if (-not $State.Installed) { - if (@($State.Rows | Where-Object { -not (Test-WingetVersion $_) }).Count -gt 0) { return 'unknown' } - return 'multiple' - } + $unread = @($State.Rows | Where-Object { -not (Test-WingetVersion $_) }).Count -gt 0 + if (-not $State.Installed -and -not $unread) { return 'multiple' } if (-not $State.Available) { return 'unknown' } if ((Get-ExplicitUpgrade) -contains $State.Package) { return 'self-updating' } + if ($unread) { return 'unknown' } if ((Compare-HostVersion $State.Installed $State.Available) -ge 0) { return 'current' } return 'outdated' } @@ -1111,7 +1110,7 @@ function Invoke-ToolApply { # Naming a scope the installed copy does not sit in would add a second copy beside it, so the removal is asked for rather than done on the way past. if ($script:WANT_SCOPE -and $state.Rows.Count -gt 0 -and $state.Scope.Count -gt 0 -and $state.Scope -notcontains $script:WANT_SCOPE -and $script:MODE -ne 'reinstall') { - die "${ToolName}: installed $($state.Scope -join ' and ') wide at $($state.Installed), and -Scope $($script:WANT_SCOPE) was given. Installing would add a second copy beside it. Remove the existing copy first with: install-tools.ps1 -Reinstall $ToolName -Scope $($script:WANT_SCOPE)" + die "${ToolName}: installed $($state.Scope -join ' and ') wide at $(if ($state.Installed) { $state.Installed } else { $state.Rows -join ', ' }), and -Scope $($script:WANT_SCOPE) was given. Installing would add a second copy beside it. Remove the existing copy first with: install-tools.ps1 -Reinstall $ToolName -Scope $($script:WANT_SCOPE)" } if ($script:MODE -eq 'reinstall') { diff --git a/tests/test_install_tools.py b/tests/test_install_tools.py index 712c4ef23..2d944b2b2 100755 --- a/tests/test_install_tools.py +++ b/tests/test_install_tools.py @@ -343,10 +343,12 @@ def test_json_is_refused_beside_another_action(self) -> None: foreach ($definition in $ast.FindAll({ param($node) $node -is [System.Management.Automation.Language.FunctionDefinitionAst] -and $wanted -contains $node.Name }, $true)) { . ([scriptblock]::Create($definition.Extent.Text)) } -function Get-ExplicitUpgrade { return @() } +$script:EXPLICIT = @() +function Get-ExplicitUpgrade { return $script:EXPLICIT } $results = [ordered]@{} foreach ($case in (Get-Content -Raw -LiteralPath $Cases | ConvertFrom-Json).PSObject.Properties) { - $rows = Read-WingetTable -Text $case.Value -Id 'jqlang.jq' + $script:EXPLICIT = @(if ($case.Value.PSObject.Properties['explicit']) { $case.Value.explicit }) + $rows = Read-WingetTable -Text $case.Value.table -Id 'jqlang.jq' $installed = Resolve-InstalledVersion -Version $rows $state = @{ Readable = $true; Rows = $rows; Installed = $installed; Available = '1.8.2'; Package = 'jqlang.jq' } $status = Get-ToolStatus -Tool @{ Name = 'jq'; Probe = 'jq' } -State $state @@ -355,6 +357,42 @@ def test_json_is_refused_beside_another_action(self) -> None: $results | ConvertTo-Json -Depth 4 """ +UNREAD_APPLY_HARNESS = r""" +param([string]$Installer, [string]$Cases) +$ErrorActionPreference = 'Stop' +Set-StrictMode -Version Latest +$tokens = $null +$errors = $null +$ast = [System.Management.Automation.Language.Parser]::ParseFile($Installer, [ref]$tokens, [ref]$errors) +$wanted = @('Show-Report', 'Invoke-ToolApply', 'Get-NoteText', 'note') +foreach ($definition in $ast.FindAll({ param($node) $node -is [System.Management.Automation.Language.FunctionDefinitionAst] -and $wanted -contains $node.Name }, $true)) { + . ([scriptblock]::Create($definition.Extent.Text)) +} +function log { param([string]$Message = '') Write-Host "LOG $Message" } +function info { param([string]$Message) Write-Host "INFO $Message" } +function warn { param([string]$Message) Write-Host "WARN $Message" } +function die { param([string]$Message) throw $Message } +function Get-Tool { param([string]$Name) @{ Name = $Name; Package = 'jqlang.jq'; Probe = 'jq'; Optional = @() } } +function Get-ToolState { + param([hashtable]$Tool) + return @{ Package = 'jqlang.jq'; Installed = $null; Rows = @('Unknown'); Readable = $true; Available = '1.8.2'; Scope = @('user'); Status = 'unknown' } +} +function Add-ToolNote { param([hashtable]$Tool, [hashtable]$State) } +function Invoke-WingetInstall { param([string]$Id) Write-Host "INSTALL $Id"; return 0 } +function Invoke-WingetUpgrade { param([string]$Id) Write-Host "UPGRADE $Id"; return 0 } +$NOTES = @() +$NOTE_TEXTS = @() +$JSON_OUTPUT = $false +$ELEVATED = $true +$SELECTED = @('jq') +$FAILED = @() +$WANT_SCOPE = $null +$WITH_OPTIONAL = $false +$MODE = 'install' +Show-Report +Invoke-ToolApply -ToolName 'jq' +""" + def winget_list(*versions: str) -> str: """A `winget list` table carrying one row per version for the id the harness asks about.""" @@ -366,6 +404,33 @@ def winget_list(*versions: str) -> str: return "\n".join(lines) +def run_pwsh_harness(harness_text: str, cases: object) -> subprocess.CompletedProcess[str]: + """Run a harness against the Windows installer, handing it `cases` as a JSON file.""" + with tempfile.TemporaryDirectory() as directory: + harness = Path(directory, "harness.ps1") + harness.write_text(harness_text, encoding="utf-8") + data = Path(directory, "cases.json") + data.write_text(json.dumps(cases), encoding="utf-8") + return subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(harness), + "-Installer", + str(WINDOWS_INSTALLER), + "-Cases", + str(data), + ], + capture_output=True, + text=True, + encoding="utf-8", + check=False, + timeout=120, + ) + + @unittest.skipUnless( shutil.which("pwsh"), "needs pwsh to drive the Windows installer's own functions" ) @@ -374,36 +439,18 @@ class TestWindowsInstalledVersion(unittest.TestCase): def test_a_non_version_token_is_unread_rather_than_installed(self) -> None: cases = { - "unknown": winget_list("Unknown"), - "below": winget_list("<"), - "above": winget_list(">"), - "unknown beside a version": winget_list("Unknown", "1.8.1"), - "one version": winget_list("1.8.1"), - "majors differ": winget_list("1.8.1", "2.0.0"), + "unknown": {"table": winget_list("Unknown")}, + "below": {"table": winget_list("<")}, + "above": {"table": winget_list(">")}, + "unknown beside a version": {"table": winget_list("Unknown", "1.8.1")}, + "unknown and self-updating": { + "table": winget_list("Unknown"), + "explicit": ["jqlang.jq"], + }, + "one version": {"table": winget_list("1.8.1")}, + "majors differ": {"table": winget_list("1.8.1", "2.0.0")}, } - with tempfile.TemporaryDirectory() as directory: - harness = Path(directory, "harness.ps1") - harness.write_text(WINGET_HARNESS, encoding="utf-8") - data = Path(directory, "cases.json") - data.write_text(json.dumps(cases), encoding="utf-8") - result = subprocess.run( - [ - "pwsh", - "-NoProfile", - "-NonInteractive", - "-File", - str(harness), - "-Installer", - str(WINDOWS_INSTALLER), - "-Cases", - str(data), - ], - capture_output=True, - text=True, - encoding="utf-8", - check=False, - timeout=120, - ) + result = run_pwsh_harness(WINGET_HARNESS, cases) self.assertEqual(result.returncode, 0, result.stderr) self.assertEqual( json.loads(result.stdout), @@ -416,6 +463,11 @@ def test_a_non_version_token_is_unread_rather_than_installed(self) -> None: "installed": None, "status": "unknown", }, + "unknown and self-updating": { + "rows": ["Unknown"], + "installed": None, + "status": "self-updating", + }, "one version": {"rows": ["1.8.1"], "installed": "1.8.1", "status": "outdated"}, "majors differ": { "rows": ["1.8.1", "2.0.0"], @@ -425,6 +477,17 @@ def test_a_non_version_token_is_unread_rather_than_installed(self) -> None: }, ) + def test_the_report_prints_the_row_and_install_leaves_the_tool_alone(self) -> None: + result = run_pwsh_harness(UNREAD_APPLY_HARNESS, {}) + self.assertEqual(result.returncode, 0, result.stderr) + reported = [ + line.split() for line in result.stdout.splitlines() if line.startswith("LOG jq ") + ] + self.assertEqual([row[:3] for row in reported], [["LOG", "jq", "Unknown"]], result.stdout) + self.assertIn("no version comparison is possible", result.stdout) + self.assertNotIn("INSTALL ", result.stdout) + self.assertNotIn("UPGRADE ", result.stdout) + if __name__ == "__main__": unittest.main()