Fix incremental build problems (#1488)

It can be hard to discover the reason why a full build was triggered
deep inside the log.
This PR adds a notice to the build when a full build is triggered, with
the reason behind it.

Fixes #1487

Additionally, a Pull Request, which should trigger a full build (due to
files changed) wouldn't trigger the change due to a parameter error.

Thirdly - in some cases, we would attempt to do a git diff with an empty
sha.

Lastly - skip the build cleanup step if no build step was ever run
(saves ~10 seconds)

---------

Co-authored-by: freddydk <[email protected]>

Author: freddydk

Actions/DetermineProjectsToBuild/DetermineProjectsToBuild.Action.ps1

@@ -21,37 +21,47 @@ if ($ENV:GITHUB_EVENT_NAME -eq 'pull_request') {
     $targetBranch = $env:GITHUB_BASE_REF
 }
 
+#region Action: Determine projects to build
 Write-Host "$($ENV:GITHUB_EVENT_NAME) on $targetBranch"
 $buildAllProjects, $publishSkippedProjects = Get-BuildAllProjectsBasedOnEventAndSettings -ghEventName $ENV:GITHUB_EVENT_NAME -settings $settings
 
+$modifiedFiles = @()
 $baselineWorkflowRunId = 0 #default to 0, which means no baseline workflow run ID is set
 $baselineWorkflowSHA = ''
 if(-not $buildAllProjects) {
     Write-Host "::group::Determine Baseline Workflow ID"
     $baselineWorkflowRunId,$baselineWorkflowSHA = FindLatestSuccessfulCICDRun -repository $env:GITHUB_REPOSITORY -branch $targetBranch -token $token -retention $settings.incrementalBuilds.retentionDays
     Write-Host "::endgroup::"
-}
 
-#region Action: Determine projects to build
-Write-Host "::group::Get Modified Files"
-try {
-    $modifiedFiles = @(Get-ModifiedFiles -baselineSHA $baselineWorkflowSHA)
-    OutputMessageAndArray -message "Modified files" -arrayOfStrings $modifiedFiles
-}
-catch {
-    Write-Host "Failed to calculate modified files, building all projects"
-    $buildAllProjects = $true
-    $modifiedFiles = @()
+    if (-not ($baselineWorkflowSHA -and $baselineWorkflowRunId -ne '0')) {
+        OutputNotice -message "Unable to locate a successful CI/CD run for branch $targetBranch, newer than $($settings.incrementalBuilds.retentionDays) days, building all projects"
+        $buildAllProjects = $true
+    }
+    else {
+        Write-Host "::group::Get Modified Files"
+        try {
+            $modifiedFiles = Get-ModifiedFiles -baselineSHA $baselineWorkflowSHA
+            OutputMessageAndArray -message "Modified files" -arrayOfStrings $modifiedFiles
+        }
+        catch {
+            OutputWarning -message "Failed to calculate modified files since $baselineWorkflowSHA, the Error was $($_.Exception.Message). Building all projects"
+            $buildAllProjects = $true
+        }
+        Write-Host "::endgroup::"
+    }
 }
-Write-Host "::endgroup::"
 
-Write-Host "::group::Determine Incremental Build"
-$buildAllProjects = $buildAllProjects -or (Get-BuildAllProjects -modifiedFiles $modifiedFiles -baseFolder $baseFolder)
-Write-Host "::endgroup::"
+if (-not $buildAllProjects) {
+    Write-Host "::group::Determine Incremental Build"
+    $buildAllProjects = Get-BuildAllProjects -modifiedFiles $modifiedFiles -baseFolder $baseFolder
+    Write-Host "::endgroup::"
+}
 
 # If we are to publish artifacts for skipped projects later, we include the full project list and in the build step, just avoid building the skipped projects
+# buildAllProjects is set to true if we are to build all projects
+# publishSkippedProjects is set to true if we are to publish artifacts for skipped projects (meaning we are still going through the build process for all projects, just not building)
 Write-Host "::group::Get Projects To Build"
-$allProjects, $modifiedProjects, $projectsToBuild, $projectDependencies, $buildOrder = Get-ProjectsToBuild -baseFolder $baseFolder -buildAllProjects $publishSkippedProjects -modifiedFiles $modifiedFiles -maxBuildDepth $maxBuildDepth
+$allProjects, $modifiedProjects, $projectsToBuild, $projectDependencies, $buildOrder = Get-ProjectsToBuild -baseFolder $baseFolder -buildAllProjects ($buildAllProjects -or $publishSkippedProjects) -modifiedFiles $modifiedFiles -maxBuildDepth $maxBuildDepth
 if ($buildAllProjects) {
     $skippedProjects = @()
 }

Actions/DetermineProjectsToBuild/DetermineProjectsToBuild.psm1

@@ -282,24 +282,24 @@ function Get-BuildAllProjectsBasedOnEventAndSettings {
 .Description
     Determines whether a full build is required.
     A full build is required if:
-    - No files were modified
     - The modified files contain a file that matches one of the fullBuildPatterns
 #>
-function Get-BuildAllProjects {
+function IsFullBuildRequired {
     param(
         [Parameter(HelpMessage = "The base folder", Mandatory = $true)]
         [string] $baseFolder,
         [Parameter(HelpMessage = "The modified files", Mandatory = $false)]
         [string[]] $modifiedFiles = @(),
         [Parameter(HelpMessage = "Full build patterns", Mandatory = $false)]
-        [string[]] $fullBuildPatterns = @()
+        [string[]] $fullBuildPatterns = @(),
+        [string] $noticeMessage = ''
     )
 
     $settings = $env:Settings | ConvertFrom-Json
 
     if (!$modifiedFiles) {
-        Write-Host "No files modified, building everything"
-        return $true
+        Write-Host "No modified files"
+        return $false
     }
 
     $fullBuildPatterns += @(Join-Path '.github' '*.json')
@@ -315,25 +315,49 @@ function Get-BuildAllProjects {
         $fullBuildFolder = Join-Path $baseFolder $fullBuildFolder
 
         if ($modifiedFiles -like $fullBuildFolder) {
-            Write-Host "Changes to $fullBuildFolder, building everything"
+            if ($noticeMessage) {
+                Write-Host "::notice::Changes to $fullBuildFolder detected, $noticeMessage"
+            }
+            else {
+                Write-Host "Changes to $fullBuildFolder detected"
+            }
             return $true
         }
     }
-
-    Write-Host "No changes to fullBuildPatterns, not building everything"
-
+    Write-Host "No changes to full build patterns detected"
     return $false
 }
 
 <#
 .Synopsis
-    Determines whether all apps in a project should be built
+    Determines whether all projects in a repository should be built
 .Outputs
     A boolean indicating whether a full build is required.
 .Description
     Determines whether a full build is required.
     A full build is required if:
-    - Get-BuildAllProjects returns true
+    - The modified files contain a file that matches one of the fullBuildPatterns
+#>
+function Get-BuildAllProjects {
+    param(
+        [Parameter(HelpMessage = "The base folder", Mandatory = $true)]
+        [string] $baseFolder,
+        [Parameter(HelpMessage = "The modified files", Mandatory = $false)]
+        [string[]] $modifiedFiles = @()
+    )
+
+    return (IsFullBuildRequired -baseFolder $baseFolder -modifiedFiles $modifiedFiles -noticeMessage "building everything")
+}
+
+<#
+.Synopsis
+    Determines whether all apps in a project should be built
+.Outputs
+    A boolean indicating whether all apps in a project should be built.
+.Description
+    Determines whether all apps in a project should be built.
+    All apps should be built if:
+    - The modified files contain a file that matches one of the fullBuildPatterns
     - The .AL-Go/settings.json file has been modified
 #>
 function Get-BuildAllApps {
@@ -346,13 +370,17 @@ function Get-BuildAllApps {
         [string[]] $modifiedFiles = @()
     )
 
+    if (IsFullBuildRequired -baseFolder $baseFolder -modifiedFiles $modifiedFiles) {
+        # Notice already given in Initialize
+        return $true
+    }
     if ($project) {
         $ALGoSettingsFile = @(Join-Path $project '.AL-Go/settings.json')
     }
     else {
         $ALGoSettingsFile = @('.AL-Go/settings.json')
     }
-    return (Get-BuildAllProjects -baseFolder $baseFolder -modifiedFiles $modifiedFiles -fullBuildPatterns @($ALGoSettingsFile))
+    return (IsFullBuildRequired -baseFolder $baseFolder -modifiedFiles $modifiedFiles -fullBuildPatterns @($ALGoSettingsFile) -noticeMessage "building all apps")
 }
 
 <#

Actions/RunPipeline/RunPipeline.ps1

@@ -136,7 +136,7 @@ try {
             $buildAll = Get-BuildAllApps -baseFolder $baseFolder -project $project -modifiedFiles $modifiedFiles
         }
         catch {
-            OutputWarning -message "Failed to calculate modified files since $baselineWorkflowSHA, building all apps"
+            OutputNotice -message "Failed to calculate modified files since $baselineWorkflowSHA, building all apps"
             $buildAll = $true
         }
         if (!$buildAll) {

Templates/AppSource App/.github/workflows/_BuildALGoProject.yaml

@@ -280,7 +280,7 @@ jobs:
           project: ${{ inputs.project }}
 
       - name: Cleanup
-        if: always()
+        if: always() && steps.DetermineBuildProject.outputs.BuildIt == 'True'
         uses: microsoft/AL-Go-Actions/PipelineCleanup@main
         with:
           shell: ${{ inputs.shell }}

Templates/Per Tenant Extension/.github/workflows/_BuildALGoProject.yaml

@@ -280,7 +280,7 @@ jobs:
           project: ${{ inputs.project }}
 
       - name: Cleanup
-        if: always()
+        if: always() && steps.DetermineBuildProject.outputs.BuildIt == 'True'
         uses: microsoft/AL-Go-Actions/PipelineCleanup@main
         with:
           shell: ${{ inputs.shell }}

Tests/DetermineProjectsToBuild.Test.ps1

@@ -762,13 +762,13 @@ Describe "Get-BuildAllProjects" {
         $baseFolder = (New-Item -ItemType Directory -Path (Join-Path $([System.IO.Path]::GetTempPath()) $([System.IO.Path]::GetRandomFileName()))).FullName
     }
 
-    It ('returns true if there are no modified filed') {
+    It ('returns false if there are no modified files') {
         # Add AL-Go settings
         $alGoSettings = @{ fullBuildPatterns = @() }
         $env:Settings = ConvertTo-Json $alGoSettings -Depth 99 -Compress
 
         $buildAllProjects = Get-BuildAllProjects -baseFolder $baseFolder
-        $buildAllProjects | Should -Be $true
+        $buildAllProjects | Should -Be $false
     }
 
     It ('returns true if any of the modified files matches any of the patterns in fullBuildPatterns setting') {