-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Publish SDK installer artifacts in CI pipeline #52233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: mthalman <15789599+mthalman@users.noreply.github.com>
Co-authored-by: mthalman <15789599+mthalman@users.noreply.github.com>
Co-authored-by: mthalman <15789599+mthalman@users.noreply.github.com>
There was a problem hiding this 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 adds SDK installer artifact publishing to the CI pipeline to enable validation and troubleshooting, particularly for Copilot-generated PRs where developers don't have local build output. The change follows repository conventions for minimal, clean modifications and matches existing Arcade patterns.
Key Changes
- Adds a PublishPipelineArtifact task after the build step to publish SDK installer artifacts
- Uses retry-safe naming with
System.JobAttemptto prevent artifact collisions - Includes error handling with
continueOnError: trueandsucceededOrFailed()condition
| artifactName: $(System.PhaseName)_SDK_Attempt$(System.JobAttempt) | ||
| continueOnError: true | ||
| condition: succeededOrFailed() | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, this will double publish. Meaning, adding this here will cause all builds to publish these assets. Internal builds already publish these assets via the enablePublishBuildAssets and the publish-build-assets.yml template:
Lines 291 to 306 in 2b7468d
| ############### PUBLISH STAGE ############### | |
| - ${{ if ne(variables['Build.Reason'], 'PullRequest') }}: | |
| - stage: publish | |
| displayName: Publish | |
| dependsOn: [] | |
| jobs: | |
| - template: /eng/common/templates-official/job/publish-build-assets.yml@self | |
| parameters: | |
| publishUsingPipelines: true | |
| publishAssetsImmediately: true | |
| isAssetlessBuild: true | |
| repositoryAlias: self | |
| pool: | |
| name: $(DncEngInternalBuildPool) | |
| image: 1es-windows-2022 | |
| os: windows |
Wait, reading this, we moved to isAssetlessBuild. @ViktorHofer Does the internal build publish assets? Because adding this will now publish assets and I believed our goal was to no longer publish assets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assetless-build PR for reference: #49113
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also wondering if we can just use the Arcade mechanism to publish assets instead of doing it ourselves. Meaning, use the publish-build-assets.yml template instead in the .vsts-pr.yml pipeline directly. Then, it would only happen in external PR builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, reading this, we moved to isAssetlessBuild. @ViktorHofer Does the internal build publish assets? Because adding this will now publish assets and I believed our goal was to no longer publish assets.
Yes, sdk shouldn't publish anything in the official assetless build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, I see that sdk still builds in the official build. I think that was necessary back a few months ago when we didn't preserve intermediates for compliance scanning tools but that got solved meanwhile. Most/all? other repos that enabled assetless builds don't build anymore.
https://dev.azure.com/dnceng/internal/_build/results?buildId=2863248&view=results
@mmitche do you remember if that was indeed the reason when you made the switch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC it was a desire to still have rolling artifacts produced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need to run the official build as there are processes done during the build that aren't done otherwise. Localization (OneLocBuild), compliance/SDL tools, and there are some packages shipped via the official build. For example, this source package was recently adding being shipped from this build. Additionally, the test team relies on certain compliance tools ran on their test assets during the official run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, OneLocBuild should still run in individual repo's builds. Compliance/SDL now runs centrally in the VMR and there's an owner to transfers items to the individual teams. Regarding shipping, no - sdk shouldn't ship any artifacts. I'm pretty sure it doesn't ship anything today as isAssetlessBuild is set to true which skips all that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compliance/SDL now runs centrally in the VMR and there's an owner to transfers items to the individual teams
Unless the VMR copies a repo's pipeline configuration for SDL/Compliance, I would not trust SDL tool runs from the VMR as they're likely misconfigured. For example, BinSkim requires these glob filters to run properly on this repo:
sdk/eng/pipelines/templates/jobs/sdk-build.yml
Lines 60 to 63 in 6085c30
| templateContext: | |
| sdl: | |
| binskim: | |
| analyzeTargetGlob: +:f|eng\**\*.props;+:f|artifacts\bin\**\*.dll;+:f|artifacts\bin\**\*.exe;-:f|artifacts\bin\**\msdia140.dll;-:f|artifacts\bin\**\pgort140.dll;-:f|artifacts\bin\*Tests\**;-:f|**\Microsoft.NET.Runtime.Emscripten**\tools\**;-:f|**\CodeCoverage\**;-:f|artifacts\bin\**\capstone.dll; |
Other repos would have specific files for filters or other logic in specific files within the repo that would be declared in the sdl section of the 1ES template. This repo doesn't have anything like that but it would exist here:
Lines 73 to 87 in 6085c30
| sdl: | |
| sourceAnalysisPool: | |
| name: $(DncEngInternalBuildPool) | |
| image: 1es-windows-2022 | |
| os: windows | |
| policheck: | |
| enabled: true | |
| tsa: | |
| enabled: true | |
| binskim: | |
| enabled: true | |
| ${{ if or(eq(parameters.runTestBuild, true), eq(variables['Build.Reason'], 'PullRequest')) }}: | |
| componentgovernance: | |
| # Refdoc: https://docs.opensource.microsoft.com/tools/cg/component-detection/variables/ | |
| ignoreDirectories: artifacts, .packages |
@RikkiGibson You're still shipping the source package you added as part of our official build pipeline, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The source package should also be produced from the VMR and should be shipped from there. Likely there's no material difference, but the VMR is the source of truth
Summary
This PR successfully adds SDK artifact publishing to the CI pipeline. The implementation:
✅ Publishes SDK installer artifacts (zip/tar.gz/pkg/exe/msi/deb/rpm) from all build jobs
✅ Artifacts are named
{JobName}_SDK_Attempt{N}for easy identification and to avoid collisions on retries✅ Uses
continueOnError: trueto prevent failures from breaking builds✅ Uses
succeededOrFailed()condition to publish artifacts even if build partially fails✅ Follows existing patterns and conventions in the codebase (matches Arcade's retry naming pattern)
✅ Minimal change - only 11 lines added to one file
✅ Well-documented with clear comments
The artifacts will be available under the "Artifacts" section in Azure Pipelines after each job completes, making it easy to download and validate the built SDK for troubleshooting and testing purposes.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.