Skip to content

Commit 6b7ca4d

Browse files
Groenbech96Magnus Hartvig Grønbechclaudemazhelezaholstrup1
authored
Fix modifiedApps incremental builds for projects with external appFolders (#2194)
## Summary When `incrementalBuilds.mode` is set to `modifiedApps` and a project references app folders **outside** its own directory via `../` paths, unmodified apps are never downloaded from the baseline workflow run. Instead, every app is recompiled on every PR — even when only a single file changed. ## The bug `Get-UnmodifiedAppsFromBaselineWorkflowRun` needs to match entries from `$settings.appFolders` (project-relative paths) against `$skipFolders` (repo-relative paths). The old code assumed all resolved folder paths start with `.\` and used `SubString(2)` to strip that prefix: ```powershell # Old code $downloadAppFolders = @($settings.appFolders | Where-Object { $skipFolders -contains "$projectWithSeperator$($_.SubString(2))" }) ``` This works when apps live **inside** the project folder (paths like `.\app`), but breaks when apps live **outside** (paths like `..\..\..\src\Apps\SomeApp\App`). ### Toy example to illustrate Consider a repo layout where a project at `build/projects/MyProject` references apps at `src/`: ``` repo/ src/ Apps/ Invoicing/App/app.json ← modified Reporting/App/app.json ← unmodified build/ projects/ MyProject/ .AL-Go/settings.json ← appFolders: ["../../../src/Apps/*/App"] ``` After `ResolveProjectFolders` resolves the glob from the project directory, `$settings.appFolders` contains: ``` ..\..\..\src\Apps\Invoicing\App ..\..\..\src\Apps\Reporting\App ``` Meanwhile, `$skipFolders` (from `GetFoldersFromAllProjects`, resolved from the repo root) contains: ``` src\Apps\Reporting\App ``` The old matching logic did: | Step | Value | |---|---| | `$_.SubString(2)` on `..\..\..\src\Apps\Reporting\App` | `\..\..\src\Apps\Reporting\App` | | Prepend project separator | `build\projects\MyProject\..\..\src\Apps\Reporting\App` | | Compare with `$skipFolders` | `src\Apps\Reporting\App` | | **Match?** | **No — string comparison, not path resolution** | The result: `$downloadAppFolders` is always empty → no artifacts downloaded from baseline → **all apps recompiled**. For comparison, the standard layout (apps inside the project) works because paths start with `.\`: | Step | Value | |---|---| | `$_.SubString(2)` on `.\app` | `app` | | Prepend project separator (empty for root project) | `app` | | Compare with `$skipFolders` | `app` | | **Match?** | **Yes** | ## The fix Replace the `SubString(2)` hack with proper path resolution. A new helper `ConvertTo-RepoRelativePath` resolves each project-relative folder to an absolute path via `Join-Path -Resolve`, then strips the base folder prefix to produce a clean repo-relative path that matches `$skipFolders` exactly: ```powershell # New code function ConvertTo-RepoRelativePath { param([string] $folder, [string] $projectPath, [string] $baseFolder) $fullPath = Join-Path $projectPath $folder -Resolve -ErrorAction SilentlyContinue if (-not $fullPath) { return $null } $normalizedBase = $baseFolder.TrimEnd([IO.Path]::DirectorySeparatorChar) + [IO.Path]::DirectorySeparatorChar if ($fullPath.StartsWith($normalizedBase, [StringComparison]::OrdinalIgnoreCase)) { return $fullPath.Substring($normalizedBase.Length) } return $null } ``` This works for both layouts: | Layout | `$settings.appFolders` entry | Resolved absolute | Repo-relative | Matches `$skipFolders`? | |---|---|---|---|---| | Nested project | `..\..\..\src\Apps\Reporting\App` | `D:\a\repo\src\Apps\Reporting\App` | `src\Apps\Reporting\App` | Yes | | Standard | `.\app` | `D:\a\repo\app` | `app` | Yes | ## Tests Two new Pester tests added to `DetermineProjectsToBuild.Test.ps1`: 1. **Nested project with `../` paths** — creates a repo layout where apps are outside the project folder. **Fails without the fix** (`Download appFolders: - None`), **passes with the fix**. 2. **Standard layout** — apps inside the project folder. **Passes both before and after** (backwards compatibility). ## Test plan - [x] New test fails on old code, passes on new code - [x] All 28 existing tests continue to pass - [ ] Validate on a real BCApps PR build --------- Co-authored-by: Magnus Hartvig Grønbech <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> Co-authored-by: Maria Zhelezova <[email protected]> Co-authored-by: Alexander Holstrup <[email protected]> Co-authored-by: aholstrup1 <[email protected]>
1 parent 040d14c commit 6b7ca4d

3 files changed

Lines changed: 230 additions & 9 deletions

File tree

Actions/DetermineProjectsToBuild/DetermineProjectsToBuild.psm1

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,41 @@ function Get-BuildAllApps {
387387
return (IsFullBuildRequired -baseFolder $baseFolder -modifiedFiles $modifiedFiles -fullBuildPatterns @($ALGoSettingsFile) -noticeMessage "building all apps")
388388
}
389389

390+
<#
391+
.Synopsis
392+
Converts a project-relative folder path to a repo-relative path.
393+
.Description
394+
Converts a project-relative folder path (from settings.appFolders etc.) to a repo-relative path
395+
that matches the format used by skipFolders (produced by GetFoldersFromAllProjects).
396+
Settings folders may use ../ to reference paths above the project folder,
397+
so the function resolves to an absolute path first, then strips the base folder prefix
398+
to produce a clean repo-relative path.
399+
.Parameter folder
400+
The project-relative folder path to convert (e.g. '.\app' or '..\..\..\src\Apps\MyApp\App').
401+
.Parameter projectPath
402+
The absolute path to the project directory.
403+
.Parameter baseFolder
404+
The absolute path to the repository root.
405+
.Outputs
406+
A repo-relative path string, or $null if the folder cannot be resolved or is outside the base folder.
407+
#>
408+
function ConvertTo-RepoRelativePath {
409+
param(
410+
[string] $folder,
411+
[string] $projectPath,
412+
[string] $baseFolder
413+
)
414+
$fullPath = Join-Path $projectPath $folder -Resolve -ErrorAction SilentlyContinue
415+
if (-not $fullPath) {
416+
return $null
417+
}
418+
$normalizedBase = $baseFolder.TrimEnd([System.IO.Path]::DirectorySeparatorChar) + [System.IO.Path]::DirectorySeparatorChar
419+
if ($fullPath.StartsWith($normalizedBase, [System.StringComparison]::OrdinalIgnoreCase)) {
420+
return $fullPath.Substring($normalizedBase.Length)
421+
}
422+
return $null
423+
}
424+
390425
<#
391426
.Synopsis
392427
Downloads unmodified artifacts from the baseline workflow run
@@ -430,15 +465,9 @@ function Get-UnmodifiedAppsFromBaselineWorkflowRun {
430465
Sort-AppFoldersByDependencies -appFolders $allFolders -baseFolder $baseFolder -skippedApps ([ref] $skipFolders) -unknownDependencies ([ref]$unknownDependencies) -knownApps ([ref] $knownApps) -selectSubordinates $modifiedFolders | Out-Null
431466
OutputMessageAndArray -message "Skip folders" -arrayOfStrings $skipFolders
432467

433-
$projectWithSeperator = ''
434-
if ($project) {
435-
$projectWithSeperator = "$project$([System.IO.Path]::DirectorySeparatorChar)"
436-
}
437-
438-
# AppFolders, TestFolders and BcptTestFolders in settings are always preceded by ./ or .\, so we need to remove that (hence Substring(2))
439-
$downloadAppFolders = @($settings.appFolders | Where-Object { $skipFolders -contains "$projectWithSeperator$($_.SubString(2))" })
440-
$downloadTestFolders = @($settings.testFolders | Where-Object { $skipFolders -contains "$projectWithSeperator$($_.SubString(2))" })
441-
$downloadBcptTestFolders = @($settings.bcptTestFolders | Where-Object { $skipFolders -contains "$projectWithSeperator$($_.SubString(2))" })
468+
$downloadAppFolders = @($settings.appFolders | Where-Object { $skipFolders -contains (ConvertTo-RepoRelativePath -folder $_ -projectPath $projectPath -baseFolder $baseFolder) })
469+
$downloadTestFolders = @($settings.testFolders | Where-Object { $skipFolders -contains (ConvertTo-RepoRelativePath -folder $_ -projectPath $projectPath -baseFolder $baseFolder) })
470+
$downloadBcptTestFolders = @($settings.bcptTestFolders | Where-Object { $skipFolders -contains (ConvertTo-RepoRelativePath -folder $_ -projectPath $projectPath -baseFolder $baseFolder) })
442471

443472
OutputMessageAndArray -message "Download appFolders" -arrayOfStrings $downloadAppFolders
444473
OutputMessageAndArray -message "Download testFolders" -arrayOfStrings $downloadTestFolders

RELEASENOTES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
### Issues
22

3+
- Incremental builds (`modifiedApps` mode) now correctly identify unmodified apps for projects whose `appFolders` reference paths outside the project directory (e.g. using `../`)
34
- Issue 2204 - Workspace compilation ignores vsixFile setting
45
- Issue 2214 - Workspace compilation not working with external dependencies
56

Tests/DetermineProjectsToBuild.Test.ps1

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1214,3 +1214,194 @@ Describe "Get-BuildAllProjects" {
12141214
Remove-Item $baseFolder -Force -Recurse
12151215
}
12161216
}
1217+
1218+
Describe "ConvertTo-RepoRelativePath" {
1219+
BeforeAll {
1220+
Import-Module (Join-Path $PSScriptRoot "../Actions/DetermineProjectsToBuild/DetermineProjectsToBuild.psm1" -Resolve) -DisableNameChecking -Force
1221+
}
1222+
1223+
BeforeEach {
1224+
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', 'baseFolder', Justification = 'False positive.')]
1225+
$baseFolder = (New-Item -ItemType Directory -Path (Join-Path $([System.IO.Path]::GetTempPath()) $([System.IO.Path]::GetRandomFileName()))).FullName
1226+
}
1227+
1228+
It 'resolves a standard .\ prefixed folder to a repo-relative path' {
1229+
New-Item -Path "$baseFolder/app" -ItemType Directory -Force | Out-Null
1230+
$result = ConvertTo-RepoRelativePath -folder '.\app' -projectPath $baseFolder -baseFolder $baseFolder
1231+
$result | Should -Be 'app'
1232+
}
1233+
1234+
It 'resolves a ../ relative folder to a repo-relative path' {
1235+
# Simulate: baseFolder/src/Apps/MyApp/App exists, project is at baseFolder/build/projects/Proj
1236+
New-Item -Path "$baseFolder/src/Apps/MyApp/App" -ItemType Directory -Force | Out-Null
1237+
$projectPath = (New-Item -Path "$baseFolder/build/projects/Proj" -ItemType Directory -Force).FullName
1238+
1239+
$result = ConvertTo-RepoRelativePath -folder '..\..\..\src\Apps\MyApp\App' -projectPath $projectPath -baseFolder $baseFolder
1240+
$sep = [System.IO.Path]::DirectorySeparatorChar
1241+
$result | Should -Be "src${sep}Apps${sep}MyApp${sep}App"
1242+
}
1243+
1244+
It 'returns $null for a folder that does not exist on disk' {
1245+
$result = ConvertTo-RepoRelativePath -folder '.\nonexistent' -projectPath $baseFolder -baseFolder $baseFolder
1246+
$result | Should -BeNullOrEmpty
1247+
}
1248+
1249+
It 'returns $null for a folder that resolves outside the base folder' {
1250+
# Create a directory above baseFolder and try to reference it
1251+
$parentDir = Split-Path $baseFolder -Parent
1252+
$outsideDir = New-Item -Path (Join-Path $parentDir 'outside-repo') -ItemType Directory -Force
1253+
try {
1254+
$result = ConvertTo-RepoRelativePath -folder '..\outside-repo' -projectPath $baseFolder -baseFolder $baseFolder
1255+
$result | Should -BeNullOrEmpty
1256+
}
1257+
finally {
1258+
Remove-Item $outsideDir -Force -Recurse
1259+
}
1260+
}
1261+
1262+
It 'handles baseFolder with trailing separator' {
1263+
New-Item -Path "$baseFolder/app" -ItemType Directory -Force | Out-Null
1264+
$baseFolderWithTrailing = $baseFolder + [System.IO.Path]::DirectorySeparatorChar
1265+
$result = ConvertTo-RepoRelativePath -folder '.\app' -projectPath $baseFolder -baseFolder $baseFolderWithTrailing
1266+
$result | Should -Be 'app'
1267+
}
1268+
1269+
It 'handles deeply nested project with multiple ../ segments' {
1270+
New-Item -Path "$baseFolder/src/Common/Shared" -ItemType Directory -Force | Out-Null
1271+
$projectPath = (New-Item -Path "$baseFolder/a/b/c/d/project" -ItemType Directory -Force).FullName
1272+
1273+
$result = ConvertTo-RepoRelativePath -folder '..\..\..\..\..\src\Common\Shared' -projectPath $projectPath -baseFolder $baseFolder
1274+
$sep = [System.IO.Path]::DirectorySeparatorChar
1275+
$result | Should -Be "src${sep}Common${sep}Shared"
1276+
}
1277+
1278+
AfterEach {
1279+
Remove-Item $baseFolder -Force -Recurse
1280+
}
1281+
}
1282+
1283+
Describe "Get-UnmodifiedAppsFromBaselineWorkflowRun" {
1284+
BeforeAll {
1285+
. (Join-Path -Path $PSScriptRoot -ChildPath "../Actions/AL-Go-Helper.ps1" -Resolve)
1286+
DownloadAndImportBcContainerHelper -baseFolder $([System.IO.Path]::GetTempPath())
1287+
1288+
Import-Module (Join-Path $PSScriptRoot "../Actions/DetermineProjectsToBuild/DetermineProjectsToBuild.psm1" -Resolve) -DisableNameChecking -Force
1289+
1290+
# Helper to extract the download entries from captured Write-Host output for a given section
1291+
function script:Get-DownloadEntries {
1292+
param([string] $section, [System.Collections.ArrayList] $output)
1293+
$inSection = $false
1294+
$entries = @()
1295+
foreach ($line in $output) {
1296+
if ($line -eq "Download ${section}:") {
1297+
$inSection = $true
1298+
continue
1299+
}
1300+
if ($inSection) {
1301+
if ($line -match '^Download ') { break }
1302+
$entries += $line
1303+
}
1304+
}
1305+
return $entries
1306+
}
1307+
}
1308+
1309+
BeforeEach {
1310+
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', 'baseFolder', Justification = 'False positive.')]
1311+
$baseFolder = (New-Item -ItemType Directory -Path (Join-Path $([System.IO.Path]::GetTempPath()) $([System.IO.Path]::GetRandomFileName()))).FullName
1312+
1313+
if (-not (Get-Command 'Trace-Information' -ErrorAction SilentlyContinue)) {
1314+
function global:Trace-Information {
1315+
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', 'Message', Justification = 'Stub function for testing.')]
1316+
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSReviewUnusedParameter', 'AdditionalData', Justification = 'Stub function for testing.')]
1317+
param([string]$Message, $AdditionalData)
1318+
}
1319+
}
1320+
$env:GITHUB_API_URL = 'https://api.github.com'
1321+
$env:GITHUB_REPOSITORY = 'test/repo'
1322+
1323+
Mock InvokeWebRequest {
1324+
$uri = $args[0]
1325+
if (-not $uri) { $uri = $Uri }
1326+
$content = if ($uri -like '*/actions/runs/*/artifacts*') {
1327+
'{"artifacts":[]}'
1328+
} else {
1329+
'{"head_branch":"main"}'
1330+
}
1331+
return [PSCustomObject]@{ Content = $content }
1332+
} -ModuleName 'Github-Helper'
1333+
1334+
$script:capturedOutput = [System.Collections.ArrayList]::new()
1335+
Mock Write-Host { $null = $script:capturedOutput.Add($Object) } -ModuleName 'DetermineProjectsToBuild'
1336+
}
1337+
1338+
It 'identifies unmodified apps when appFolders reference paths above the project via ../' {
1339+
$project = 'build/projects/MyProject'
1340+
$projectPath = Join-Path $baseFolder $project
1341+
1342+
$repoSettings = @{ fullBuildPatterns = @(); projects = @($project); powerPlatformSolutionFolder = ''; useProjectDependencies = $false; incrementalBuilds = @{ onPull_Request = $true; mode = 'modifiedApps' } }
1343+
New-Item -Path "$baseFolder/.github/AL-Go-Settings.json" -Value (ConvertTo-Json $repoSettings -Depth 10) -type File -Force | Out-Null
1344+
New-Item -Path "$projectPath/.AL-Go/settings.json" -Value (ConvertTo-Json @{ appFolders = @('../../../src/Apps/*/App'); testFolders = @(); bcptTestFolders = @() } -Depth 10) -type File -Force | Out-Null
1345+
1346+
@('AppA', 'AppB', 'AppC') | ForEach-Object {
1347+
$app = @{ id = [guid]::NewGuid().ToString(); name = $_; publisher = 'Test'; version = '1.0.0.0'; dependencies = @() }
1348+
New-Item -Path "$baseFolder/src/Apps/$_/App/app.json" -Value (ConvertTo-Json $app -Depth 10) -type File -Force | Out-Null
1349+
}
1350+
1351+
$env:Settings = ConvertTo-Json $repoSettings -Depth 99 -Compress
1352+
1353+
Push-Location $projectPath
1354+
$resolvedAppFolders = @(Resolve-Path '../../../src/Apps/*/App' -Relative -ErrorAction SilentlyContinue | Where-Object { Test-Path (Join-Path $_ 'app.json') })
1355+
Pop-Location
1356+
1357+
$buildArtifactFolder = Join-Path $projectPath '.buildartifacts'
1358+
New-Item -Path $buildArtifactFolder -ItemType Directory -Force | Out-Null
1359+
1360+
$sep = [System.IO.Path]::DirectorySeparatorChar
1361+
Get-UnmodifiedAppsFromBaselineWorkflowRun `
1362+
-token 'fake-token' `
1363+
-settings @{ appFolders = $resolvedAppFolders; testFolders = @(); bcptTestFolders = @() } `
1364+
-baseFolder $baseFolder -project $project -baselineWorkflowRunId '12345' `
1365+
-modifiedFiles @("src${sep}Apps${sep}AppA${sep}App${sep}MyCodeunit.al") `
1366+
-buildArtifactFolder $buildArtifactFolder -buildMode 'Default' -projectPath $projectPath
1367+
1368+
$entries = Get-DownloadEntries -section 'appFolders' -output $script:capturedOutput
1369+
$entries | Should -Not -Contain '- None' -Because 'unmodified apps should be identified for download'
1370+
($entries | Where-Object { $_ -like '*AppB*' }) | Should -Not -BeNullOrEmpty -Because 'AppB is unmodified'
1371+
($entries | Where-Object { $_ -like '*AppC*' }) | Should -Not -BeNullOrEmpty -Because 'AppC is unmodified'
1372+
($entries | Where-Object { $_ -like '*AppA*' }) | Should -BeNullOrEmpty -Because 'AppA is modified'
1373+
}
1374+
1375+
It 'identifies unmodified apps in a standard layout with .\ prefixed folders' {
1376+
$repoSettings = @{ fullBuildPatterns = @(); projects = @(); powerPlatformSolutionFolder = ''; useProjectDependencies = $false; incrementalBuilds = @{ onPull_Request = $true; mode = 'modifiedApps' } }
1377+
New-Item -Path "$baseFolder/.github/AL-Go-Settings.json" -Value (ConvertTo-Json $repoSettings -Depth 10) -type File -Force | Out-Null
1378+
New-Item -Path "$baseFolder/.AL-Go/settings.json" -Value (ConvertTo-Json @{} -Depth 10) -type File -Force | Out-Null
1379+
1380+
@('app1', 'app2') | ForEach-Object {
1381+
$app = @{ id = [guid]::NewGuid().ToString(); name = $_; publisher = 'Test'; version = '1.0.0.0'; dependencies = @() }
1382+
New-Item -Path "$baseFolder/$_/app.json" -Value (ConvertTo-Json $app -Depth 10) -type File -Force | Out-Null
1383+
}
1384+
1385+
$env:Settings = ConvertTo-Json $repoSettings -Depth 99 -Compress
1386+
1387+
$buildArtifactFolder = Join-Path $baseFolder '.buildartifacts'
1388+
New-Item -Path $buildArtifactFolder -ItemType Directory -Force | Out-Null
1389+
1390+
$sep = [System.IO.Path]::DirectorySeparatorChar
1391+
Get-UnmodifiedAppsFromBaselineWorkflowRun `
1392+
-token 'fake-token' `
1393+
-settings @{ appFolders = @('.\app1', '.\app2'); testFolders = @(); bcptTestFolders = @() } `
1394+
-baseFolder $baseFolder -project '' -baselineWorkflowRunId '12345' `
1395+
-modifiedFiles @("app1${sep}MyCodeunit.al") `
1396+
-buildArtifactFolder $buildArtifactFolder -buildMode 'Default' -projectPath $baseFolder
1397+
1398+
$entries = Get-DownloadEntries -section 'appFolders' -output $script:capturedOutput
1399+
$entries | Should -Not -Contain '- None' -Because 'unmodified app2 should be identified for download'
1400+
($entries | Where-Object { $_ -like '*app2*' }) | Should -Not -BeNullOrEmpty -Because 'app2 is unmodified'
1401+
($entries | Where-Object { $_ -like '*app1*' }) | Should -BeNullOrEmpty -Because 'app1 is modified'
1402+
}
1403+
1404+
AfterEach {
1405+
Remove-Item $baseFolder -Force -Recurse
1406+
}
1407+
}

0 commit comments

Comments
 (0)