diff --git a/CHANGELOG.md b/CHANGELOG.md index 0baa132..4317b47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,30 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +- [**#185**](https://github.com/psake/PowerShellBuild/issues/185) + Consumers on a non-English Windows PowerShell 5.1 host get the module's + real messages. `PowerShellBuild.psm1` carried a hand-maintained second copy + of every string, bound whenever `Import-LocalizedData` resolved nothing, and + it had drifted sixteen strings behind `en-US/Messages.psd1` — every + certificate and signing message was absent, and the Pester floor was still + reported as `5.0.0`. That lookup resolves by UI culture and the module ships + `en-US` only: PowerShell 7 falls back to `en-US` and never saw the drift, + but Windows PowerShell 5.1 does not fall back at all, so a French or + Japanese install bound the stale copy. A missing string does not throw, so + the symptom was a blank `WARNING:` line, or a bare `ScriptHalted` where the + signing tasks meant to say no certificate was found. The copy is gone, + `en-US` is now requested by name when the culture lookup misses, and an + import that cannot find the strings at all fails outright rather than + blanking every message. + + The same sweep found one string the module read and never shipped: + `Publish-PSBuildModule` validated `-Path` against `PathDoesNotExist`, which + `en-US/Messages.psd1` did not define, so passing a path that does not exist + failed with a blank message on **every** host and culture while the very + next check in the same validation block reported itself properly. The string + is now defined, and a test asserts that every string the module reads is one + it ships. + - [**#124**](https://github.com/psake/PowerShellBuild/issues/124) The docs tree can hold documentation that is not generated help. A `README.md` at its root, a `CONTRIBUTING.md` beside the generated markdown, an `images/` or diff --git a/PowerShellBuild/PowerShellBuild.psm1 b/PowerShellBuild/PowerShellBuild.psm1 index d6ebe19..faa2b88 100644 --- a/PowerShellBuild/PowerShellBuild.psm1 +++ b/PowerShellBuild/PowerShellBuild.psm1 @@ -9,42 +9,25 @@ foreach ($import in $public + $private) { } } -data LocalizedData { - # Load here in case Import-LocalizedData is not available - ConvertFrom-StringData @' -NoCommandsExported=No commands have been exported. Skipping markdown generation. -FailedToGenerateMarkdownHelp=Failed to generate markdown help. : {0} -AddingFileToPsm1=Adding [{0}] to PSM1 -MakeCabNotAvailable=MakeCab.exe is not available. Cannot create help cab. -DirectoryAlreadyExists=Directory already exists [{0}]. -PathLongerThan3Chars=Path [{0}] must be longer than 3 characters. -BuildSystemDetails=Build System Details: -BuildModule=Build Module: {0}:{1} -PowerShellVersion=PowerShell Version: {0} -EnvironmentVariables={0}Environment variables: -PublishingVersionToRepository=Publishing version [{0}] to repository [{1}]... -FolderDoesNotExist=Folder does not exist: {0} -PathArgumentMustBeAFolder=The Path argument must be a folder. File paths are not allowed. -UnableToFindModuleManifest=Unable to find module manifest [{0}]. Can't import module -PesterTestsFailed=One or more Pester tests failed -PesterVersionNotSupported=Pester version [{0}] is loaded, but Test-PSBuildPester requires Pester 5.0.0 or newer. -CodeCoverage=Code Coverage -Type=Type -CodeCoverageLessThanThreshold=Code coverage: [{0}] is [{1:p}], which is less than the threshold of [{2:p}] -CodeCoverageCodeCoverageFileNotFound=Code coverage file [{0}] not found. -SeverityThresholdSetTo=SeverityThreshold set to: {0} -PSScriptAnalyzerResults=PSScriptAnalyzer results: -ScriptAnalyzerErrors=One or more ScriptAnalyzer errors were found! -ScriptAnalyzerWarnings=One or more ScriptAnalyzer warnings were found! -ScriptAnalyzerIssues=One or more ScriptAnalyzer issues were found! -'@ -} -$importLocalizedDataSplat = @{ +# Every user-facing string lives in en-US/Messages.psd1 and nowhere else. Import-LocalizedData +# resolves that file through the current UI culture and the culture's parent chain, so the lookup +# misses whenever the machine's UI culture has no directory of its own -- and this module ships +# en-US only. PowerShell 7 has a final en-US fallback that hides the miss; Windows PowerShell 5.1 +# has none and binds nothing at all, which is why a French or Japanese Windows install got no +# strings. Ask for en-US by name when the culture lookup comes up empty, and fail the import if +# even that is not there: silent, missing strings turn every message into an empty warning or a +# bare ScriptHalted (psake/PowerShellBuild#185). +$importLocalizedDataParameters = @{ BindingVariable = 'LocalizedData' FileName = 'Messages.psd1' ErrorAction = 'SilentlyContinue' } -Import-LocalizedData @importLocalizedDataSplat +Import-LocalizedData @importLocalizedDataParameters +if (-not $LocalizedData) { + $importLocalizedDataParameters['UICulture'] = 'en-US' + $importLocalizedDataParameters['ErrorAction'] = 'Stop' + Import-LocalizedData @importLocalizedDataParameters +} Export-ModuleMember -Function $public.Basename diff --git a/PowerShellBuild/en-US/Messages.psd1 b/PowerShellBuild/en-US/Messages.psd1 index 0feb84b..b2ef48e 100644 --- a/PowerShellBuild/en-US/Messages.psd1 +++ b/PowerShellBuild/en-US/Messages.psd1 @@ -1,4 +1,4 @@ -ConvertFrom-StringData @' +ConvertFrom-StringData @' NoCommandsExported=No commands have been exported. Skipping markdown generation. FailedToGenerateMarkdownHelp=Failed to generate markdown help. : {0} AddingFileToPsm1=Adding [{0}] to PSM1 @@ -13,6 +13,7 @@ PowerShellVersion=PowerShell Version: {0} EnvironmentVariables={0}Environment variables: PublishingVersionToRepository=Publishing version [{0}] to repository [{1}]... FolderDoesNotExist=Folder does not exist: {0} +PathDoesNotExist=Path does not exist: {0} PathArgumentMustBeAFolder=The Path argument must be a folder. File paths are not allowed. UnableToFindModuleManifest=Unable to find module manifest [{0}]. Can't import module PesterTestsFailed=One or more Pester tests failed diff --git a/tests/LocalizedData.tests.ps1 b/tests/LocalizedData.tests.ps1 new file mode 100644 index 0000000..d0d084c --- /dev/null +++ b/tests/LocalizedData.tests.ps1 @@ -0,0 +1,195 @@ +# Coverage for how the module resolves its user-facing strings at import +# (psake/PowerShellBuild#185). +# +# PowerShellBuild.psm1 used to carry a second, hand-written copy of every string in an inline +# `data LocalizedData { ConvertFrom-StringData ... }` block, kept for the case where +# Import-LocalizedData resolved nothing. The import runs with -ErrorAction SilentlyContinue, so a +# lookup that misses binds that copy without saying anything -- and the copy went sixteen strings +# stale, still naming the pre-#182 Pester floor of 5.0.0. +# +# The lookup misses more often than it looks. Import-LocalizedData resolves Messages.psd1 through +# the current UI culture and that culture's parent chain, and the module ships en-US only. +# PowerShell 7 has a final en-US fallback, so it always lands on the shipped file; Windows +# PowerShell 5.1 has no such fallback, so on a machine whose display language is French or +# Japanese the lookup binds nothing at all and the stale copy is what consumers got. 5.1 is a +# supported host (`PowerShellVersion = '5.1'`), which is what made the drift reachable rather +# than theoretical. +# +# Two things are pinned here. The first It is the drift guard: one definition of the strings, so +# there is no second copy left to fall out of step. The Context below is the behavioral +# guarantee, and it runs in a child process per supported host -- a UI culture has to be in place +# before the module is imported, and setting it in-process would leak into the rest of the suite. + +BeforeDiscovery { + # Probe the host running the suite, plus Windows PowerShell when it is installed, because the + # two do not agree about the en-US fallback and only the 5.1 leg catches the regression. The + # current process is used for the first leg rather than a PATH lookup so this always produces + # at least one leg, whichever host the suite was started with; identical paths collapse. + $currentHostPath = [System.Diagnostics.Process]::GetCurrentProcess().Path + $windowsPowerShellPath = ( + Get-Command -Name 'powershell.exe' -CommandType 'Application' -ErrorAction SilentlyContinue | + Select-Object -First 1 + ).Source + + # Sort-Object -Unique rather than Select-Object -Unique: when the suite is running under + # Windows PowerShell the two lookups return the same executable spelled differently + # (powershell.EXE and powershell.exe), and only Sort-Object collapses that by default. + $script:localizationHost = @( + $currentHostPath, $windowsPowerShellPath | + Where-Object { $_ } | + Sort-Object -Unique | + ForEach-Object { @{ HostName = [IO.Path]::GetFileName($_); HostPath = $_ } } + ) + if ($script:localizationHost.Count -eq 0) { + # An empty -ForEach collection throws during discovery, which reads as a file that + # silently disappeared. Say what actually went wrong instead. + throw 'Could not resolve the path of the host running this suite; no host can be probed.' + } +} + +Describe 'Localized string resolution' { + + BeforeAll { + $script:repositoryRoot = Split-Path -Path $PSScriptRoot -Parent + $script:sourceModulePath = [IO.Path]::Combine( + $script:repositoryRoot, 'PowerShellBuild', 'PowerShellBuild.psm1' + ) + + # Probe the built module rather than the source tree: it is what a consumer installs, and + # the versioned directory is the -BaseDirectory Import-LocalizedData needs. + $sourceManifestPath = [IO.Path]::Combine( + $script:repositoryRoot, 'PowerShellBuild', 'PowerShellBuild.psd1' + ) + $moduleVersion = (Import-PowerShellDataFile -Path $sourceManifestPath).ModuleVersion + $script:builtModulePath = [IO.Path]::Combine( + $script:repositoryRoot, 'Output', 'PowerShellBuild', $moduleVersion + ) + # The probe imports the .psm1 directly, not the manifest. The manifest's RequiredModules + # are resolved against the child process's inherited PSModulePath, which on Windows + # PowerShell means the PowerShell 7 module directories it was spawned from -- a failure + # that has nothing to do with which strings got bound. The .psm1 is the file under test. + $script:builtModuleFilePath = Join-Path -Path $script:builtModulePath -ChildPath 'PowerShellBuild.psm1' + + # The strings as shipped. Read through Import-LocalizedData because Messages.psd1 is a + # ConvertFrom-StringData document rather than a hashtable literal, so + # Import-PowerShellDataFile cannot read it. + $importShippedStringParameters = @{ + BindingVariable = 'shippedString' + BaseDirectory = $script:builtModulePath + FileName = 'Messages.psd1' + UICulture = 'en-US' + ErrorAction = 'Stop' + } + Import-LocalizedData @importShippedStringParameters + $script:shippedString = $shippedString + + # The probe reports what the module bound at import, from inside the module's own scope. + # Written to $TestDrive rather than checked in, following the convention in + # Test-PSBuildPester.tests.ps1 for generated fixtures. + $script:probeScriptPath = Join-Path -Path $TestDrive -ChildPath 'Get-ResolvedString.ps1' + Set-Content -Path $script:probeScriptPath -Value @' +param( + [Parameter(Mandatory)] + [string] + $ModuleFilePath, + + [Parameter(Mandatory)] + [string] + $UICulture +) + +# Set before the import, because Import-LocalizedData reads the UI culture as the module loads. +# This is the state a machine whose Windows display language is not English starts up in. +[System.Threading.Thread]::CurrentThread.CurrentUICulture = + [System.Globalization.CultureInfo]::new($UICulture) + +Import-Module -Name $ModuleFilePath -Force -WarningAction SilentlyContinue -ErrorAction Stop +$resolvedString = & (Get-Module -Name 'PowerShellBuild') { $LocalizedData } +$resolvedString | ConvertTo-Json -Depth 2 -Compress +'@ + } + + It 'defines its user-facing strings in exactly one place' { + # en-US/Messages.psd1 is the only place a string may be written. A second copy inside the + # module file cannot be kept in step by hand, which is the whole of #185. + $moduleSource = Get-Content -Path $script:sourceModulePath -Raw + $moduleSource | Should -Not -Match 'ConvertFrom-StringData' + } + + It 'ships every string the module asks for' { + # The guard above keeps a second copy from existing. This one keeps the single copy + # complete, which is the other half of the same promise and fails differently: a key + # the module reads but Messages.psd1 never defines resolves to $null, and $null -f + # $argument is an empty string rather than an error. The message simply comes out + # blank -- on every host and every culture, not just the ones the Context below + # covers. + # + # That is not hypothetical. Publish-PSBuildModule read PathDoesNotExist from a + # ValidateScript on a mandatory parameter while Messages.psd1 never defined it, so + # passing a path that did not exist failed validation with no text at all, while the + # very next check in the same script block reported itself properly. + $sourceFile = Get-ChildItem -Path ( + [IO.Path]::Combine($script:repositoryRoot, 'PowerShellBuild') + ) -Include '*.ps1', '*.psm1' -File -Recurse + + $referencedKey = @( + $sourceFile | + Select-String -Pattern '\$LocalizedData\.(\w+)' -AllMatches | + ForEach-Object { $_.Matches } | + ForEach-Object { $_.Groups[1].Value } | + Sort-Object -Unique + ) + $referencedKey | Should -Not -BeNullOrEmpty -Because 'the pattern must still match something' + + $undefined = @($referencedKey.Where({ $_ -notin $script:shippedString.Keys })) + $undefined -join ', ' | Should -BeNullOrEmpty + } + + Context 'Imported by <_.HostName> under a non-English UI culture' -ForEach $script:localizationHost { + + BeforeAll { + # fr-FR is chosen because the module ships no fr-FR directory and none of its parents + # is en-US, so nothing but a deliberate fallback can reach the shipped strings. + $probeArgument = @( + '-NoProfile' + '-NonInteractive' + '-File', $script:probeScriptPath + '-ModuleFilePath', $script:builtModuleFilePath + '-UICulture', 'fr-FR' + ) + $probeOutput = & $_.HostPath @probeArgument 2>&1 + + # The probe writes one JSON document and nothing else. Selecting the line rather than + # taking all output keeps a stray host warning from breaking the parse. + $probeJson = $probeOutput | + Where-Object { $_ -is [string] -and $_.TrimStart().StartsWith('{') } | + Select-Object -Last 1 + + $script:probeOutput = $probeOutput + $script:resolvedString = if ($probeJson) { $probeJson | ConvertFrom-Json } + } + + It 'reports a string table from the probe' { + # Guards the two assertions below: a probe that produced nothing would otherwise + # satisfy a foreach over an empty set and prove nothing. + $script:resolvedString | + Should -Not -BeNullOrEmpty -Because "the probe wrote: $($script:probeOutput -join '; ')" + } + + It 'binds every string the module ships' { + $resolvedKey = $script:resolvedString.PSObject.Properties.Name | Sort-Object + $shippedKey = $script:shippedString.Keys | Sort-Object + $resolvedKey | Should -Be $shippedKey + } + + It 'binds each string to its shipped text' { + # Key-for-key rather than a count, because the failure this pins was a copy that had + # the right shape and the wrong words -- it named a Pester floor of 5.0.0 long after + # #182 raised the real one to 6.0.0. + foreach ($key in $script:shippedString.Keys) { + $script:resolvedString.$key | + Should -Be $script:shippedString[$key] -Because "[$key] must match en-US/Messages.psd1" + } + } + } +}