Condition Artifact Upload to running lavapipe - #8724
Conversation
There was a problem hiding this comment.
Pull request overview
Conditions DXC artifact uploads on whether lavapipe offload tests will run, reducing artifact storage usage.
Changes:
- Moves the offload gate before the build stage.
- Passes the gate result to the Windows artifact publishing step.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
azure-pipelines.yml:131
- Azure Pipelines condition expressions do not resolve a bare
uploadArtifactidentifier, so this condition fails to parse/evaluate instead of gating publication. Reference the mapped job variable through thevariablescontext, as the surrounding conditions do.
condition: and(succeeded(), ne(variables['artifactName'], ''), eq(variables['uploadArtifact'], 'true'))
dc79142 to
81455ac
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (1)
azure-pipelines.yml:115
- A transient GitHub API error or unauthenticated rate limit makes
Invoke-RestMethodthrow, so this auxiliary label check now fails theVS2022_Releasebuild after its tests have passed. Catch the request failure and retain the existing false default for PRs; this skips artifact/offload work without turning core CI red.
$labels = Invoke-RestMethod "https://api.github.com/repos/$(Build.Repository.Name)/issues/$(System.PullRequest.PullRequestNumber)/labels"
|
|
||
| stages: | ||
| - stage: Build | ||
| condition: succeededOrFailed() |
There was a problem hiding this comment.
I don't think we need a condition on the first stage, do we?
| condition: and(succeeded(), eq(variables['decide.runOffloadTests'], 'true')) | ||
| inputs: | ||
| targetPath: '$(Build.ArtifactStagingDirectory)\dxc' | ||
| artifact: '$(artifactName)' |
There was a problem hiding this comment.
Does artifactName get set any more? It'd be good to see a run that runs the tests.
There was a problem hiding this comment.
Yes, but now it is only used to name the artifact being produced: https://github.com/microsoft/DirectXShaderCompiler/pull/8724/changes#diff-7915b9b726a397ae7ba6af7b9703633d21c031ebf21682f3ee7e6a4ec52837a5L41.
Currently we upload a new DXC artifact every time we run a test; this leads us to consume a lot more artifact storage than intended. This patch changes that so we only upload artifacts when lavapipe needs to run.