Skip to content

Fix modifiedApps incremental builds for projects with external appFolders#2194

Open
Groenbech96 wants to merge 1 commit intomicrosoft:mainfrom
Groenbech96:fix/incremental-builds-path-mismatch
Open

Fix modifiedApps incremental builds for projects with external appFolders#2194
Groenbech96 wants to merge 1 commit intomicrosoft:mainfrom
Groenbech96:fix/incremental-builds-path-mismatch

Conversation

@Groenbech96
Copy link
Copy Markdown

@Groenbech96 Groenbech96 commented Apr 1, 2026

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:

# 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:

# 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

  • New test fails on old code, passes on new code
  • All 28 existing tests continue to pass
  • Validate on a real BCApps PR build

…l appFolders

When a project is nested (e.g. build/projects/MyProject) and its appFolders
reference sources outside the project via ../ paths, the modifiedApps
incremental build mode failed to match any unmodified apps for baseline
download. This caused all apps to be recompiled on every PR even when only
one file changed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Groenbech96 Groenbech96 requested a review from a team as a code owner April 1, 2026 11:03
Copilot AI review requested due to automatic review settings April 1, 2026 11:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes incremental builds in modifiedApps mode for projects whose appFolders resolve to paths outside the project directory (via ../), ensuring unmodified apps can be downloaded from the baseline workflow run instead of always being rebuilt.

Changes:

  • Replace brittle SubString(2)-based folder matching with a new helper that resolves project-relative folders and converts them to repo-relative paths for comparison.
  • Add Pester coverage for both nested-project (../ paths) and standard (.\ paths) layouts to prevent regressions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
Actions/DetermineProjectsToBuild/DetermineProjectsToBuild.psm1 Fixes skip/download folder matching by resolving to absolute paths and converting to repo-relative paths before comparing with $skipFolders.
Tests/DetermineProjectsToBuild.Test.ps1 Adds two tests validating correct unmodified-app detection for external appFolders and standard in-project folders.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants