Incremental Builds #1304
Incremental Builds #1304
Conversation
You are right but controlling which modes makes sense for AL-Go is (I think) better than allowing people to potentially setup modes which doesn't really work and we have to support.
It has always been the case that major.minor of the github artifacts are defined by the repoversion and build.revision is defined by the build - and - the major.minor of the apps are defined by app.json and build.revision is defined by the build. So the only thing you could be certain about was that build.revision was the same - now they can also differ - I do not see a problem in that.
If incremental builds is enabled - PR builds will follow the mode specified. |
| @@ -302,6 +303,38 @@ function GetWorkflowContentWithChangesFromSettings { | |||
| $yamlOn = $yaml.Get('on:/') | |||
| $yaml.Replace('on:/', $yamlOn.content+@('schedule:', " - cron: '$($repoSettings."$workflowScheduleKey")'")) | |||
| } | |||
| $concurrency = 'allowed' | |||
| if ($repoSettings.Keys -contains $workflowConcurrencyKey) { | |||
There was a problem hiding this comment.
I don't recall quite well if there was an agreement about these settings that are dynamically determined.
But my opinion is that we should try to avoid them, as we cannot include them in a JSON schema, for example.
Why not have a workflowConcurrency setting that could be set either on workflow-level or maybe as a conditional setting?
There was a problem hiding this comment.
I will consider how to change this with the other workflow base-name settings in mind
| if ($ghEvent.PSObject.Properties.name -eq 'pull_request') { | ||
| $headSHA = $ghEvent.pull_request.head.sha | ||
| Write-Host "Using head SHA $headSHA from pull request" | ||
| git fetch origin $headSHA | Out-Host |
There was a problem hiding this comment.
Consider adding RunAndCheck function similar to https://github.com/microsoft/BCApps/blob/9810da9083d3989f77291ce86b74560ae7a324ff/build/scripts/EnlistmentHelperFunctions.psm1#L442
| $ghEvent = Get-Content $env:GITHUB_EVENT_PATH -Encoding UTF8 | ConvertFrom-Json | ||
| if ($ghEvent.PSObject.Properties.name -eq 'pull_request') { | ||
| # Pull request | ||
| $buildAllProjects = $settings.alwaysBuildAllProjects |
There was a problem hiding this comment.
The old alwaysBuildAllProjects and the new IncrementalBuilds settings seem incompatible.
The doc for alwaysBuildAllProjects says:
This setting only makes sense if the repository is setup for multiple projects.
Standard behavior of the CI/CD workflow is to only build the projects, in which files have changes when running the workflow due to a push or a pull request
In that sense if alwaysBuildAllProjects is set to true, that disables incremental builds, even if IncrementalBuilds.enable is true.
There was a problem hiding this comment.
Correct.
alwaysBuildAllProjects was default false and only had meaning in pull requests - meaning that incremental builds have been in effect in pull requests and not in CI/CD.
I will consider how to merge these and deprecate alwaysBuildAllProjects
| @@ -95,6 +98,7 @@ jobs: | |||
| with: | |||
| shell: powershell | |||
| maxBuildDepth: ${{ env.workflowDepth }} | |||
| publishSkippedProjectArtifacts: 'true' | |||
There was a problem hiding this comment.
Why is this set on the workflow when is dynamically determined, based on the GitHub event name?
Also, why not always publish the skipped project artifacts? That way, it will be way easier to "publish a PR to an environment", for instance.
| @@ -0,0 +1,51 @@ | |||
| Param( | |||
There was a problem hiding this comment.
Is there a way to add tests for the new action?
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Support incremental builds
TODO
Use git diff instead of API
In v6.1 and earlier versions, we use the GitHub API to determine last known good build and determine modified files in the Pull Request. It is however possible to have done modifications to the main branch after the last known good build - the changes will not be included in the PR and this the PR builds will only include the modified files in the PR together with the last known good build - potentially missing files.
This version locates the SHA, which was used to build the last known good build and uses git diff to determine all changes from that build including the changes in the PR to build what's needed.
git diff also supports many more files than the API, is faster and it doesn't count as API calls against the GitHub limit.
Add concurrency protection to workflows
Add new setting called
<workflow>Concurrency, which controls concurrency of the workflow. It is recommended to setcicdConcurrencyto CancelRef when using incremental builds.Allowed values are:
Allowed for allowing concurrency
Wait for waiting for any instances of the workflow currently running
WaitRef for waiting for instances of the workflow currently running on the same GitHub ref
Cancel for cancelling any currently running instances
CancelRef for cancelling any currently running instances on the same GitHub ref.
Default is Allowed (except for the CreateRelease workflow, which defaults to Wait).
Remove warnings that tests doesn't exist if doNotBuildTests is true
No reason to warn about missing test apps if you don't want to build them anyway
Remove warnings and error when locating previous builds
Getting rid of warnings like this:
due to DownloadProjectDependencies downloads artifacts double
and errors like:
if there wasn't a successful build in the repo.
Check whether a package has been delivered to GitHubPackages or NuGet before delivering
If the package already exists in the current version it has already been delivered (might be from an earlier build)