From ee449a9978fc6f821d501e0555398861cc4540ce Mon Sep 17 00:00:00 2001 From: Trent Blackburn Date: Fri, 28 Aug 2026 16:17:49 -0400 Subject: [PATCH] fix: Stop compile mode sweeping the working directory and skipping about help Two defects in Build-PSBuildModule's staging, both silent, both producing a plausible artifact from a build that reported success. Researched against consumer usage and peer tools before choosing between the options each issue listed. #206 -- CompileDirectories defaulted to @(), and PowerShell treats an empty -Path as "not supplied" and falls back to the current location. So -Compile without the parameter concatenated every .ps1 beneath the working directory into the root module. Found when a test compiled this repository's own test files into a fixture module. The fix is a default plus a guard, because they close different halves. The default -- @('Enum', 'Classes', 'Private', 'Public') -- covers an omitted argument and matches what the tasks already pass, what the README has always documented, and what ModuleBuilder declares in the same position. The guard covers an explicit empty array, which both task files forward unguarded when a consumer sets $PSBPreference.Build.CompileDirectories = @(); that reached the same sweep through supported configuration, so the default alone would not have closed it. ValidateNotNullOrEmpty was considered and rejected on evidence: validation attributes are not applied to default values, so it leaves the omitted case open while breaking the explicit one. Throwing was rejected because compiling only to wrap an existing .psm1 in a header and footer is a coherent request. Upstream declined to change Get-ChildItem's null handling (PowerShell/PowerShell#17793, Won't Fix) with the working group advising callers to validate, so the guard belongs here. Compiling to an empty set now warns. That is the #201 precedent -- the failure mode is a module with no functions and a green build, and a warning makes it visible without forbidding the legitimate case. #207 -- the Copy-Item writing about_.help.txt sat inside the Test-Path branch that creates the culture directory, so an existing directory meant no about help file at all. CopyDirectories runs earlier, and in compile mode naming the culture directory there is the only way to ship a locale directory, so any compiled module with localized data suppressed its own about help. The Force on that copy was already present and unreachable: the author intended an unconditional overwrite and a misplaced brace prevented it. This restores that. The shape now matches Build-PSBuildMAMLHelp and ModuleBuilder's CopyReadMe, from which this code appears to descend -- the port collapsed two guards into one. The test pinning the broken behaviour is inverted deliberately, not discovered as a failure. Its comment now records what was fixed. Closes #206 Closes #207 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U1Jhu7fgTRJq7LK5MuKteE --- CHANGELOG.md | 22 +++++ .../Public/Build-PSBuildModule.ps1 | 63 +++++++++--- PowerShellBuild/en-US/Messages.psd1 | 1 + docs/migration-v0.8-to-v1.0.md | 31 ++++++ tests/Build-PSBuildModule.tests.ps1 | 97 +++++++++++++++++-- 5 files changed, 190 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1069497..f0ceed4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -108,6 +108,28 @@ Everything below is the detail, one entry per issue. ### Fixed +- [**#206**](https://github.com/psake/PowerShellBuild/issues/206) + `Build-PSBuildModule -Compile` no longer compiles your working directory when no + compile directories are given. `CompileDirectories` defaulted to `@()`, and + `Get-ChildItem -Path @()` treats an empty path as "not supplied" and falls back to the + current location — so every `.ps1` beneath wherever the build ran was concatenated into + the root module, which then failed to import while the build reported success. The + parameter now defaults to `@('Enum', 'Classes', 'Private', 'Public')`, matching what the + tasks already pass and what the README has always documented, and an explicitly empty + list is guarded rather than expanded — that path was reachable through supported + configuration by setting `$PSBPreference.Build.CompileDirectories = @()`. Compiling to + an empty set now warns instead of silently producing a module with no functions. + +- [**#207**](https://github.com/psake/PowerShellBuild/issues/207) + `$PSBPreference.Help.ConvertReadMeToAboutHelp` works when the output already has a + culture directory. The `Copy-Item` that writes `about_.help.txt` sat inside the + `Test-Path` branch that creates that directory, so an existing one meant no about help + file was written at all — silently. It was reachable in compile mode whenever + `$PSBPreference.Build.CopyDirectories` named the culture directory, which in compile + mode is the only way to ship a locale directory at all. The `-Force` on that copy was + already there and unreachable; the guard now covers only the directory creation, as it + does elsewhere in the module. + - [**#203**](https://github.com/psake/PowerShellBuild/issues/203) A publish that fails now fails the build. `Publish-Module` reports a failed publish as a non-terminating error — an unregistered repository, a rejected diff --git a/PowerShellBuild/Public/Build-PSBuildModule.ps1 b/PowerShellBuild/Public/Build-PSBuildModule.ps1 index ae41fe9..249d367 100644 --- a/PowerShellBuild/Public/Build-PSBuildModule.ps1 +++ b/PowerShellBuild/Public/Build-PSBuildModule.ps1 @@ -75,7 +75,12 @@ function Build-PSBuildModule { [string]$ReadMePath, - [string[]]$CompileDirectories = @(), + # Defaulted rather than left empty. Get-ChildItem -Path @() binds nothing and + # falls back to the current location, so -Compile with no directories recursed + # the caller's working directory into the built module and still reported + # success. This is the value build.properties.ps1 supplies, so a direct call now + # behaves like the task path. See psake/PowerShellBuild#206. + [string[]]$CompileDirectories = @('Enum', 'Classes', 'Private', 'Public'), [string[]]$CopyDirectories = @(), @@ -113,15 +118,21 @@ function Build-PSBuildModule { $culturePath, "about_$($ModuleName).help.txt" ) - if (-not (Test-Path $culturePath -PathType Container)) { - New-Item $culturePath -Type Directory -Force > $null - $copyItemSplat = @{ - LiteralPath = $ReadMePath - Destination = $aboutModulePath - Force = $true - } - Copy-Item @copyItemSplat + # The guard belongs to New-Item alone. With the copy inside it, an existing + # culture directory meant no about help file was written at all -- and + # CopyDirectories runs above, so naming the culture directory there was enough + # to suppress it silently. That is psake/PowerShellBuild#207. The Force below was + # already here and unreachable; this restores the overwrite it was written for. + if (-not (Test-Path -LiteralPath $culturePath -PathType Container)) { + New-Item -Path $culturePath -ItemType Directory -Force > $null } + + $copyItemSplat = @{ + LiteralPath = $ReadMePath + Destination = $aboutModulePath + Force = $true + } + Copy-Item @copyItemSplat } # Copy source files to destination and optionally combine *.ps1 files @@ -174,14 +185,34 @@ function Build-PSBuildModule { $resolvedCompileDirectories = $CompileDirectories | ForEach-Object { [IO.Path]::Combine($Path, $_) } - $getChildItemSplat = @{ - Path = $resolvedCompileDirectories - Filter = '*.ps1' - File = $true - Recurse = $true - ErrorAction = 'SilentlyContinue' + # An empty compile list leaves -Path null, and Get-ChildItem treats null or empty as + # "not supplied" and falls back to the current location -- so this would recurse the + # working directory and concatenate every .ps1 under it into the root module, while + # the build reported success. PowerShell/PowerShell#17793 is Won't Fix, with the + # working group's advice being to validate in the caller, so the guard belongs here. + # + # The parameter default covers an omitted argument. This covers an explicit empty + # array, which is what both task files forward when a consumer sets + # $PSBPreference.Build.CompileDirectories = @(). See psake/PowerShellBuild#206. + $allScripts = @() + if ($resolvedCompileDirectories) { + $getChildItemSplat = @{ + Path = $resolvedCompileDirectories + Filter = '*.ps1' + File = $true + Recurse = $true + ErrorAction = 'SilentlyContinue' + } + $allScripts = Get-ChildItem @getChildItemSplat + } + + # Compiling to an empty set produces a root module holding only its header, the + # appended source .psm1, and its footer -- a plausible artifact with every function + # missing. Warned about rather than treated as an error: wrapping an already-complete + # .psm1 in a header and footer is a coherent thing to ask for. + if (-not $allScripts) { + Write-Warning ($LocalizedData.NoScriptsToCompile -f ($CompileDirectories -join ', ')) } - $allScripts = Get-ChildItem @getChildItemSplat $allScripts = $allScripts | Remove-ExcludedItem -Exclude $Exclude diff --git a/PowerShellBuild/en-US/Messages.psd1 b/PowerShellBuild/en-US/Messages.psd1 index 03847aa..6b5c93b 100644 --- a/PowerShellBuild/en-US/Messages.psd1 +++ b/PowerShellBuild/en-US/Messages.psd1 @@ -2,6 +2,7 @@ NoCommandsExported=No commands have been exported. Skipping markdown generation. FailedToGenerateMarkdownHelp=Failed to generate markdown help. : {0} AddingFileToPsm1=Adding [{0}] to PSM1 +NoScriptsToCompile=No .ps1 files were found to compile. The compiled module will contain no functions. Searched: [{0}]. MakeCabNotAvailable=MakeCab.exe is not available. Cannot create help cab. HelpInfoUriRequired=Updatable help was skipped for [{0}]. The module manifest does not declare a HelpInfoUri, which is where Update-Help looks for the help content, so a cabinet built without one cannot be used. ModuleLandingPageNotFound=Updatable help was skipped for locale [{1}]. The module landing page [{0}] does not exist. It is generated by the GenerateMarkdown task; regenerate the documentation and try again. diff --git a/docs/migration-v0.8-to-v1.0.md b/docs/migration-v0.8-to-v1.0.md index 635bf07..af19ce3 100644 --- a/docs/migration-v0.8-to-v1.0.md +++ b/docs/migration-v0.8-to-v1.0.md @@ -58,6 +58,8 @@ One line per break; follow the link for details and migration steps. — psake users must upgrade to 5.0.4+; Invoke-Build users are unaffected. - [Pester 5.x is no longer supported; the floor is now 6.0.0](#pester-5x-is-no-longer-supported-the-floor-is-now-600) — Pester 6 keeps the `Should -Be` syntax, so most suites need no changes. +- [`Build-PSBuildModule -CompileDirectories` has a real default](#build-psbuildmodule--compiledirectories-has-a-real-default) + — only affects direct callers of the function; the tasks always passed it. - [`$PSBPreference.Sign.SkipCertificateValidation` now has an effect](#psbpreferencesignskipcertificatevalidation-now-has-an-effect) — the escape hatch did nothing on 0.8.x; a build that failed on an expired certificate may now succeed by signing with it. @@ -946,6 +948,35 @@ with a confusing `CommandNotFoundException`. And the exact pin in Decision and evidence in [#172](https://github.com/psake/PowerShellBuild/issues/172). +### `Build-PSBuildModule -CompileDirectories` has a real default + +**Only affects code that calls `Build-PSBuildModule` directly.** Consumers +going through the psake or Invoke-Build tasks are unaffected, because both +pass the setting explicitly. + +The parameter used to default to `@()`. That was never usable: PowerShell +treats an empty `-Path` as *not supplied* and falls back to the current +location, so `-Compile` without `-CompileDirectories` concatenated every +`.ps1` beneath the working directory into the built module — and reported +success. The default is now the same value the tasks pass and the README +has always documented: + + @('Enum', 'Classes', 'Private', 'Public') + +**No action is required if your sources live in those directories**, which +is the layout the setting has documented since 0.5.0. + +**If they do not**, and you called the function without the parameter while +standing in your module's source root, the old fallback happened to sweep +your sources up anyway. That stops. Name your directories explicitly: + + Build-PSBuildModule -Path ./src -Compile -CompileDirectories @('functions') + +A build that compiles nothing now warns rather than producing a module with +no functions, so the change announces itself rather than being discovered in +a published package. + +Tracked in [#206](https://github.com/psake/PowerShellBuild/issues/206). ### `$PSBPreference.Sign.SkipCertificateValidation` now has an effect **Only affects builds with `$PSBPreference.Sign.Enabled = $true`.** diff --git a/tests/Build-PSBuildModule.tests.ps1 b/tests/Build-PSBuildModule.tests.ps1 index 3ed1150..69d0b3c 100644 --- a/tests/Build-PSBuildModule.tests.ps1 +++ b/tests/Build-PSBuildModule.tests.ps1 @@ -621,6 +621,78 @@ foreach ($sourceDirectoryName in @('Public', 'Private')) { } } + Context 'Compiling without naming the compile directories' { + + # -CompileDirectories used to default to @(), and Get-ChildItem -Path @() binds + # nothing and falls back to the current location -- so a direct call that omitted + # the parameter recursed the caller's working directory into the built module and + # still reported success. The tests here caught it by building a module that + # contained this repository's own test files. See psake/PowerShellBuild#206. + # + # The assertion that matters is the negative one: the built module must contain + # the fixture's functions and nothing from wherever the test happened to run. + BeforeAll { + $script:scenario = New-PSBuildModuleScenario -Path $TestDrive -Name 'default-compile-directories' + $buildParameter = @{ + Path = $script:scenario.SourcePath + DestinationPath = $script:scenario.DestinationPath + ModuleName = $script:scenario.ModuleName + Compile = $true + } + Build-PSBuildModule @buildParameter + $script:rootModuleContent = Get-Content -LiteralPath $script:scenario.RootModulePath -Raw + } + + It 'Compiles the source module functions' { + $script:rootModuleContent | Should -Match 'function Get-Widget' + $script:rootModuleContent | Should -Match 'function Set-Widget' + } + + It 'Compiles nothing from the current working directory' { + # Pester files are the tell: they exist under the working directory and never + # under the fixture, so their presence means the glob escaped the source tree. + $script:rootModuleContent | Should -Not -Match 'Describe\s+' + $script:rootModuleContent | Should -Not -Match 'BeforeAll\s*\{' + } + + It 'Builds a module that exports its public functions' { + $exportedFunctionName = Get-BuiltModuleExportedFunctionName -ManifestPath $script:scenario.ManifestPath + + $exportedFunctionName | Should -Be @('Get-Widget', 'Set-Widget') + } + } + + Context 'Compiling with an explicitly empty compile directory list' { + + # The parameter default covers an omitted argument, but both task files forward + # $PSBPreference.Build.CompileDirectories unguarded, so a consumer who sets it to + # @() binds an explicit empty array and the default never applies. That path + # reached the same working-directory sweep through supported configuration. + BeforeAll { + $script:scenario = New-PSBuildModuleScenario -Path $TestDrive -Name 'empty-compile-directories' + $buildParameter = @{ + Path = $script:scenario.SourcePath + DestinationPath = $script:scenario.DestinationPath + ModuleName = $script:scenario.ModuleName + Compile = $true + CompileDirectories = @() + } + $script:buildWarning = @() + Build-PSBuildModule @buildParameter -WarningVariable 'buildWarning' -WarningAction 'SilentlyContinue' + $script:buildWarning = @($buildWarning) + $script:rootModuleContent = Get-Content -LiteralPath $script:scenario.RootModulePath -Raw + } + + It 'Compiles nothing from the current working directory' { + $script:rootModuleContent | Should -Not -Match 'Describe\s+' + $script:rootModuleContent | Should -Not -Match 'BeforeAll\s*\{' + } + + It 'Warns that the compiled module will contain no functions' { + $script:buildWarning -join ' ' | Should -Match 'no functions' + } + } + Context 'Converting the readme into about help' { BeforeAll { @@ -680,12 +752,12 @@ foreach ($sourceDirectoryName in @('Public', 'Private')) { Context 'Converting the readme when the culture directory already exists' { - # Pins current behavior, which looks wrong: the Copy-Item that writes the about help file - # sits inside the `if (-not (Test-Path $culturePath))` branch that creates the culture - # directory, so a build whose output already has that directory silently writes no about - # help file at all. It is reachable in compile mode whenever CopyDirectories names the - # culture directory. Left as-is here because it is a behavior change outside the scope of - # psake/PowerShellBuild#98 and #201; reported separately. + # An existing culture directory used to mean no about help file was written at all: + # the Copy-Item sat inside the branch that creates the directory, so the guard that + # was meant to protect New-Item suppressed the copy too. Reachable in compile mode + # whenever CopyDirectories names the culture directory -- which, in compile mode, is + # the only way to ship a locale directory at all. Fixed in + # psake/PowerShellBuild#207; this context asserted Should -Not -Exist beforehand. BeforeAll { $script:scenario = New-PSBuildModuleScenario -Path $TestDrive -Name 'about-help-existing' $readMePath = Join-Path -Path $script:scenario.SourcePath -ChildPath 'README.md' @@ -705,10 +777,19 @@ foreach ($sourceDirectoryName in @('Public', 'Private')) { Build-PSBuildModule @buildParameter } - It 'Writes no about help file' { + It 'Writes the about help file anyway' { [IO.Path]::Combine( $script:scenario.DestinationPath, 'en-US', 'about_PSBuildTestFixture.help.txt' - ) | Should -Not -Exist + ) | Should -Exist + } + + It 'Writes the readme content into it' { + $aboutHelpPath = [IO.Path]::Combine( + $script:scenario.DestinationPath, 'en-US', 'about_PSBuildTestFixture.help.txt' + ) + + Get-Content -LiteralPath $aboutHelpPath -Raw | + Should -Match 'PSBuildTestFixture readme content' } }