fix: Stop compile mode sweeping the working directory and skipping about help

CHANGELOG.md

@@ -108,6 +108,28 @@
 
 ### 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_<Module>.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
@@ -278,7 +300,7 @@
   that passed before may now correctly fail.
 - [**#96**](https://github.com/psake/PowerShellBuild/issues/96)
   `Test-PSBuildScriptAnalysis` no longer fails with a path-resolution error
   when `SettingsPath` is not supplied. An unsupplied path was forwarded to
   PSScriptAnalyzer as `-Settings ''`, which resolved against the current
   directory and threw before any analysis ran, so the function's own
   documented example could not run as written.

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
 

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.

docs/migration-v0.8-to-v1.0.md

@@ -58,6 +58,8 @@
   — 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.
@@ -858,7 +860,7 @@
 `-FromModule`, and `$psake.build_success` are all explicitly retained — this
 repository still uses all three. The breaks are:
 
 - `default.ps1` is no longer auto-detected — rename it to `psakefile.ps1`, or
   pass `-BuildFile`. PowerShellBuild's own convention has always been
   `psakeFile.ps1`, so this is unlikely to affect you.
 - The standalone `psake.ps1` and `psake.cmd` runners are gone — use
@@ -946,6 +948,35 @@
 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`.**

tests/Build-PSBuildModule.tests.ps1

@@ -597,7 +597,7 @@
         # while the build still succeeds -- the same silent-drop outcome as the drive-relative
         # path this loop already guards against, reached a different way.
         #
         # A bracketed directory would exercise it too, but not testably: New-ModuleManifest
         # has no -LiteralPath, so the fixture cannot be created in one.
         BeforeAll {
             $script:scenario = New-PSBuildModuleScenario -Path $TestDrive -Name 'wildcard-name'
@@ -621,6 +621,78 @@
         }
     }
 
+    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 @@
 
     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 @@
             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'
         }
     }