Skip whole CI legs for documentation/.github-only PRs#14384
Skip whole CI legs for documentation/.github-only PRs#14384jankratochvilcz wants to merge 6 commits into
Conversation
Gating the heavy build/test steps with a per-step condition still allocated an agent for every leg -- a docs-only PR would sit waiting for machine allocation across all legs before skipping the build. This introduces a single lightweight `EvaluateDocumentationOnlyChange` job that computes `onlyDocChanged` once and exposes it as an output variable; every heavy job in .vsts-dotnet-ci.yml and azure-pipelines/quarantine.yml now `dependsOn` it and carries a job-level `condition`, so for a documentation/.github-only PR the entire leg is skipped and no agent is ever requested. The source-build leg is gated via `jobProperties`. Quarantine legs keep running on non-PR (rolling/batched and scheduled) builds so the quarantined-test signal on main is unaffected. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95de2e0b-cc2f-47fc-a95a-3d66f8cd1d1a
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
This PR optimizes MSBuild’s Azure Pipelines CI for documentation- and .github-only pull requests by moving the “docs-only change” detection to a single lightweight gating job and using job-level conditions to skip allocating agents for heavy CI legs.
Changes:
- Add an
EvaluateDocumentationOnlyChangejob that runs the existingcheck-documentation-only-change.ymltemplate once and exposesonlyDocChangedas a job output. - Gate all heavy jobs in
.vsts-dotnet-ci.ymlon the output variable so docs-only PRs skip entire jobs (no agent allocation). - Apply the same gating pattern to the quarantined-test pipeline (
azure-pipelines/quarantine.yml) while still always running non-PR executions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
azure-pipelines/quarantine.yml |
Adds a lightweight evaluation job and gates quarantine jobs so docs-only PRs skip agent allocation while non-PR runs still execute. |
.vsts-dotnet-ci.yml |
Introduces a shared docs-only evaluation job and applies job-level gating across all heavy CI legs, including the templated source-build leg via jobProperties. |
GitHub Actions
Inactive
GitHub Actions
Inactive
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
Overall: Good approach to skip CI for doc-only PRs at the job level. Three issues found:
High: Incorrect diff detection (check-documentation-only-change.yml)
git diff HEAD HEAD~1 only checks the last commit, not the full PR diff. Multi-commit PRs can be misclassified. Use the PR target branch diff instead.
Medium: Fail-closed gate (.vsts-dotnet-ci.yml)
If the gate job fails, all downstream jobs silently skip (output variable unset ≠ '0'). This is a fail-closed design that can mask CI failures. Consider fail-open conditions.
Low: Quarantine pool inconsistency (quarantine.yml)
Missing internal project pool configuration.
Verified OK
source-build.ymltemplate does supportjobPropertieson the platform object ✓- Quarantine conditions correctly allow non-PR runs via
or(ne(variables['Build.Reason'], 'PullRequest'), ...)✓ CodeCoveragejob'sand(succeeded(), ...)correctly blocks if gate fails ✓
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
awmgmcpg
To allow these domains, add them to the
network.allowedlist in your workflow frontmatter:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
Generated by Expert Code Review (on open) for #14384 · 58.8 AIC · ⌖ 7.66 AIC · ⊞ 4.8K
GitHub Actions
Abandoned
Address review feedback: if the EvaluateDocumentationOnlyChange gate job fails
(shallow clone, git/agent issue), its output variable is unset, which under the
previous conditions caused every downstream job to skip -- a transient gate
failure would report the whole PR CI green (fail-closed, masking real breakage).
Every gated job condition now runs the leg when the gate did not succeed
(`not(succeeded('EvaluateDocumentationOnlyChange'))`), so a gate failure runs the
full matrix instead of silently skipping it. Legs are skipped only when the gate
succeeded AND classified the change as documentation/.github-only.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 95de2e0b-cc2f-47fc-a95a-3d66f8cd1d1a
Address review feedback on diff detection. For AzDO PR validation builds the checkout is the merge ref refs/pull/<N>/merge, whose first parent (HEAD~1) is the base-branch tip, so `git diff HEAD HEAD~1` already yields the full cumulative PR diff regardless of commit count. Documented that reasoning inline. Additionally wrap detection in error handling that defaults onlyDocChanged=0 (run full CI) on any git failure or empty diff. A detection problem can now never skip CI for a code change -- it can only ever over-run, which is the safe direction -- complementing the job-level fail-open conditions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95de2e0b-cc2f-47fc-a95a-3d66f8cd1d1a
|
Addressed the three review points in 71c919f + a929820:
For AzDO PR validation builds the checkout is the GitHub-generated merge ref On top of that, I added detection-level fail-open: detection now defaults to
Done — every gated job condition now runs the leg when the gate did not succeed (
Intentional — |
In the past, scheduling pressure has meant that this strategy slowed down builds that need to run full CI dramatically:
We're actually seeing this already, right now: the check job has been waiting 13 minutes and is still 203 in the queue, and all the other jobs have not even been considered. So I expect this PR check to spend at least 25 minutes queuing. |
|
I agree this is not great @rainersigwald, trying to think other ways we could drop the entire jobs.... |
The fail-open gate condition was copied verbatim onto ~10 jobs across both
pipelines. Azure Pipelines cannot reuse a whole condition via $(var) or
${{ variables.x }} (those insert a string that isn't evaluated), but a
compile-time parameter reference IS substituted as expression text before
evaluation.
Author the expression once as a `runUnlessDocumentationOnly` pipeline parameter
in each file and reference it via ${{ parameters.runUnlessDocumentationOnly }}
on every gated job (CodeCoverage keeps its and(succeeded(), ...) wrapper). No
behavioral change -- the compiled conditions are identical to before.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 95de2e0b-cc2f-47fc-a95a-3d66f8cd1d1a
The EvaluateDocumentationOnlyChange gate is always-on (it must report the required msbuild-pr check and it needs the runtime changed-file list), so it cannot be removed without a server-side branch-policy path filter. Running it on the self-hosted NetCore-Public pool, however, added it to the very queue it exists to relieve -- every PR paid a self-hosted allocation for the gate, and code PRs serialized behind it. Move the gate to the Microsoft-hosted pool (vmImage: ubuntu-latest), which is free and uncongested for this public project. Net effect: a docs-only PR now allocates zero self-hosted agents (all heavy legs skipped, gate on hosted), and a code PR waits only for a short hosted job before the heavy legs start. Documented the zero-job branch-policy path-filter alternative inline for maintainers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95de2e0b-cc2f-47fc-a95a-3d66f8cd1d1a
Running the gate on the Microsoft-hosted pool for public PR builds is a security concern (untrusted fork PR code on hosted agents). Restore the gate job in both pipelines to the self-hosted NetCore-Public pool, matching every other leg. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95de2e0b-cc2f-47fc-a95a-3d66f8cd1d1a
Problem
The previous docs-only optimization (#14371) gated the heavy steps of each CI leg with
condition: eq(variables.onlyDocChanged, 0). That still allocated an agent for every leg — a docs-only PR would sit waiting for machine allocation across all legs (Windows Full/Core, Linux, macOS, MT, Full Release, Code Coverage, Source-Build) before skipping the actual build. As seen on the mock PR, a leg can get stuck waiting on the pool even though it does nothing.Fix
Gate at the job level so a docs-only PR never requests an agent for the heavy legs:
EvaluateDocumentationOnlyChangejob that runs the existing detection template once and exposesonlyDocChangedas an output variable..vsts-dotnet-ci.ymlnowdependsOnit and carries a job-levelcondition: eq(dependencies.EvaluateDocumentationOnlyChange.outputs['CheckDocumentationOnlyChange.onlyDocChanged'], '0'). Skipped jobs allocate no machine.jobProperties(no changes to Arcade-managedeng/common).azure-pipelines/quarantine.ymlgets the same treatment, but keeps the existing semantics: non-PR runs (rolling/batched and the daily schedule) always run, so the quarantined-test signal onmainis unaffected — only docs-only PR runs are skipped.Net effect for a documentation/
.github-only PR: only the ~seconds-long detection job runs; all build/test legs show as skipped without ever waiting on a pool. Real code PRs are unaffected (this PR itself touches pipeline YAML, so it runs full CI).The per-job detection step is intentionally retained inside each heavy job so the existing per-step conditions and the
true-onlyDocChangedscript argument keep working unchanged when a leg does run.