fix(pipeline): add arm64 job and update GitHub service connection name - #50
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the Azure DevOps OneBranch signing pipeline to match a known-good DeepPrompt pipeline by adding Windows ARM64 signing/packaging and updating the GitHub service connection used for publishing release assets.
Changes:
- Add a new Windows ARM64 job (
build_sign_windows_arm64) that cross-compiles, signs, and packagesaarch64-pc-windows-msvc. - Update the GitHub Release publish task to use the
github.com_msftsiwei_microsoftssoservice connection. - Align GitHubRelease task behavior (
addChangeLog: false) with the referenced working pipeline.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/sign-and-release.yml | Adds a Windows ARM64 build/sign job and updates GitHub service connection/publish settings. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 3
| # - crates.io redirect to ADO DeepPrompt Cargo feed | ||
| # - BinSkim-required rustflags (OneBranch BA2007) |
There was a problem hiding this comment.
The Step 5 comment says .cargo/config.toml provides a "crates.io redirect to ADO DeepPrompt Cargo feed", but .cargo/config.toml explicitly documents that source replacement is intentionally NOT done and crates.io remains the default. Please update this comment to avoid misleading future pipeline maintainers.
| # - crates.io redirect to ADO DeepPrompt Cargo feed | |
| # - BinSkim-required rustflags (OneBranch BA2007) | |
| # - the ADO registry definition used by CargoAuthenticate@0 | |
| # - BinSkim-required rustflags (OneBranch BA2007) | |
| # - crates.io remains the default registry; no source replacement is configured |
There was a problem hiding this comment.
Fixed: updated the Step 5 comment to clarify that ado-cargo-config.toml (not the repo-root .cargo/config.toml) is what gets copied into place, and that crates.io remains the default registry in the repo-root config.
| - job: build_sign_windows_arm64 | ||
| displayName: 'Build tgrep Windows arm64 + Sign' | ||
| timeoutInMinutes: 120 | ||
| cancelTimeoutInMinutes: 1 | ||
| pool: | ||
| type: windows | ||
| variables: | ||
| ob_outputDirectory: '$(Build.ArtifactStagingDirectory)/ONEBRANCH_ARTIFACT' | ||
| ob_sdl_binskim_break: true # https://aka.ms/obpipelines/sdl | ||
|
|
||
| steps: | ||
| - checkout: self | ||
| fetchDepth: 1 | ||
| fetchTags: false | ||
| displayName: 'Checkout microsoft/tgrep' | ||
|
|
||
| - task: RustInstaller@1 | ||
| displayName: 'Install Rust toolchain (with arm64 target)' | ||
| inputs: | ||
| rustVersion: ms-stable | ||
| additionalTargets: aarch64-pc-windows-msvc | ||
| toolchainFeed: https://devdiv.pkgs.visualstudio.com/_packaging/Rust/nuget/v3/index.json |
There was a problem hiding this comment.
The Windows x64 and arm64 jobs duplicate the same sequence of steps (checkout/install/auth/build/sign/package/publish). Consider using a job template or a matrix strategy keyed by target to reduce duplication and ensure future changes (e.g., signing settings) stay in sync across architectures.
There was a problem hiding this comment.
Acknowledged. Refactoring to a matrix strategy is a good long-term improvement but is out of scope for this fix PR. Will track as a follow-up.
|
|
||
| - script: | | ||
| cargo build --release --locked -p tgrep-cli --bin tgrep --target aarch64-pc-windows-msvc | ||
| displayName: 'Build tgrep Windows arm64 (cross-compile)' | ||
| workingDirectory: $(Build.SourcesDirectory) |
There was a problem hiding this comment.
The ARM64 build likely inherits /CETCOMPAT from .cargo/config.toml (see rustflags), but the GitHub Actions release workflow explicitly disables CET compatibility for aarch64-pc-windows-msvc. Without overriding RUSTFLAGS here, the ARM64 job may fail to link or produce an invalid binary. Add a step in this job to set RUSTFLAGS (or equivalent) to disable /CETCOMPAT for the ARM64 target before cargo build.
| - script: | | |
| cargo build --release --locked -p tgrep-cli --bin tgrep --target aarch64-pc-windows-msvc | |
| displayName: 'Build tgrep Windows arm64 (cross-compile)' | |
| workingDirectory: $(Build.SourcesDirectory) | |
| - powershell: | | |
| Write-Host "##vso[task.setvariable variable=RUSTFLAGS]-C link-arg=/CETCOMPAT:NO" | |
| displayName: 'Set ARM64 Rust flags to disable CET compatibility' | |
| - script: | | |
| cargo build --release --locked -p tgrep-cli --bin tgrep --target aarch64-pc-windows-msvc | |
| displayName: 'Build tgrep Windows arm64 (cross-compile)' | |
| workingDirectory: $(Build.SourcesDirectory) | |
| env: | |
| RUSTFLAGS: $(RUSTFLAGS) |
There was a problem hiding this comment.
Already handled: ado-cargo-config.toml defines separate [target.aarch64-pc-windows-msvc] rustflags without /CETCOMPAT (only /DYNAMICBASE), while [target.x86_64-pc-windows-msvc] includes /CETCOMPAT. No additional RUSTFLAGS override is needed.
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
.github/workflows/sign-and-release.yml:143
- The PR description says the repo-root
.cargo/config.tomlis used as-is, but this job overwrites it in the checkout by copyingado-cargo-config.tomlinto.cargo/config.toml. Consider either updating the PR description / comments here to explicitly call out that the pipeline replaces the checked-in config for the ADO build, or avoid overwriting by using a separate CARGO_HOME/config location if the tasks support it.
# Step 4: Copy ADO-specific .cargo/config.toml into place.
# .github/workflows/ado-cargo-config.toml redirects crates.io to the
# ADO DeepPrompt feed and includes BinSkim rustflags. It is separate
# from the repo-root .cargo/config.toml (which intentionally does NOT
# redirect crates.io, to avoid breaking external contributor builds).
# ---------------------------------------------------------------
- powershell: |
$src = "$(Build.SourcesDirectory)\.github\workflows\ado-cargo-config.toml"
$dstDir = "$(Build.SourcesDirectory)\.cargo"
New-Item -ItemType Directory -Force -Path $dstDir | Out-Null
Copy-Item -Path $src -Destination "$dstDir\config.toml" -Force
Write-Host "Copied: $src -> $dstDir\config.toml"
- Files reviewed: 2/2 changed files
- Comments generated: 3
| # - .cargo/config.toml (checked in at repo root) provides: | ||
| # * [registries.devdiv-deepprompt]: required by CargoAuthenticate@0 to inject credentials | ||
| # * [target.*] rustflags: BinSkim-required flags (OneBranch BA2007) | ||
| # * NOTE: crates.io is NOT redirected here to avoid breaking external contributor builds |
There was a problem hiding this comment.
The header comment says crates.io is not redirected, but the pipeline now copies .github/workflows/ado-cargo-config.toml into .cargo/config.toml, and that file does redirect [source.crates-io]. Please update the header docs to reflect that crates.io redirection happens in this ADO pipeline via the copied config (and that the repo-root config remains non-redirecting).
This issue also appears on line 132 of the same file.
| # - .cargo/config.toml (checked in at repo root) provides: | |
| # * [registries.devdiv-deepprompt]: required by CargoAuthenticate@0 to inject credentials | |
| # * [target.*] rustflags: BinSkim-required flags (OneBranch BA2007) | |
| # * NOTE: crates.io is NOT redirected here to avoid breaking external contributor builds | |
| # - .cargo/config.toml checked in at the repo root provides: | |
| # * [registries.devdiv-deepprompt]: required by CargoAuthenticate@0 to inject credentials | |
| # * [target.*] rustflags: BinSkim-required flags (OneBranch BA2007) | |
| # * It intentionally does NOT redirect crates.io, to avoid breaking external contributor builds | |
| # - This ADO pipeline later copies .github/workflows/ado-cargo-config.toml to .cargo/config.toml; | |
| # that ADO-specific config DOES redirect [source.crates-io] for pipeline builds |
There was a problem hiding this comment.
Fixed: updated the header comment to accurately describe that ado-cargo-config.toml (copied at build time) redirects crates.io to the ADO feed, while the repo-root .cargo/config.toml intentionally does not.
| New-Item -ItemType Directory -Force -Path $dstDir | Out-Null | ||
| Copy-Item -Path $src -Destination "$dstDir\config.toml" -Force | ||
| Write-Host "Copied: $src -> $dstDir\config.toml" | ||
| Get-Content "$dstDir\config.toml" |
There was a problem hiding this comment.
Get-Content "$dstDir\config.toml" will dump the entire Cargo config to logs on every run, which adds noise and can make troubleshooting harder. Since this file is already in the repo, consider removing the full print (or replacing with a brief Write-Host confirming the copy) to keep pipeline logs smaller.
| Get-Content "$dstDir\config.toml" |
There was a problem hiding this comment.
Fixed: removed the Get-Content line to avoid dumping the config to logs on every run.
| # Step 6: Build Windows x64 release binary | ||
| # --------------------------------------------------------------- | ||
| - script: | | ||
| cargo build --release --locked -p tgrep-cli --bin tgrep --target x86_64-pc-windows-msvc | ||
| displayName: 'Build tgrep Windows x64' | ||
| workingDirectory: $(Build.SourcesDirectory) | ||
|
|
||
| # --------------------------------------------------------------- | ||
| # Step 6: Verify binary exists | ||
| # --------------------------------------------------------------- | ||
| - script: | |
There was a problem hiding this comment.
Step numbering in comments is duplicated: the binary verification section is labeled "Step 6" even though the build is already Step 6. Renumbering the comment headers will make the pipeline easier to follow when debugging.
There was a problem hiding this comment.
Fixed: renumbered the steps sequentially (Steps 7–11) to eliminate the duplicate Step 6.
There was a problem hiding this comment.
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
.github/workflows/sign-and-release.yml:155
- The Step 5 comment says “crates.io remains the default registry; no source replacement is configured”, but this job has just copied
ado-cargo-config.tomlinto.cargo/config.toml, and that file includes[source.crates-io] replace-with = "devdiv-deepprompt". Please reword this comment to clearly distinguish the repo-root.cargo/config.toml(no crates.io replacement) from the ADO-copied config (crates.io replacement enabled), to avoid misleading future maintainers.
# Step 5: Authenticate with ADO DeepPrompt Cargo feed.
# CargoAuthenticate@0 reads .cargo/config.toml and injects credentials
# for all ADO registries listed there (ado-cargo-config.toml defines
# - the ADO registry definition used by CargoAuthenticate@0
# - BinSkim-required rustflags (OneBranch BA2007)
# - crates.io remains the default registry; no source replacement is configured
# in the repo-root .cargo/config.toml). This is the official Microsoft
# way to authenticate cargo to ADO Artifact feeds.
.github/workflows/sign-and-release.yml:276
- This
Get-Contentprints the entire copied Cargo config into logs. Since the file is already in-repo and this job already logs a successful copy, consider removing the full dump (or gating it behind thedebugparameter) to keep logs smaller and avoid accidental info disclosure if the config content changes in the future.
- powershell: |
$src = "$(Build.SourcesDirectory)\.github\workflows\ado-cargo-config.toml"
$dstDir = "$(Build.SourcesDirectory)\.cargo"
New-Item -ItemType Directory -Force -Path $dstDir | Out-Null
Copy-Item -Path $src -Destination "$dstDir\config.toml" -Force
Write-Host "Copied: $src -> $dstDir\config.toml"
Get-Content "$dstDir\config.toml"
displayName: 'Copy ADO cargo config (crates.io redirect + BinSkim flags)'
- Files reviewed: 2/2 changed files
- Comments generated: 2
| - ${{ if eq(parameters.PublishToGitHub, true) }}: | ||
| - task: GitHubRelease@1 | ||
| displayName: 'Publish signed binary to GitHub Release' | ||
| inputs: | ||
| gitHubConnection: 'github.com_msftsiwei_microsoftsso' | ||
| repositoryName: 'microsoft/tgrep' | ||
| action: 'edit' | ||
| target: '$(Build.SourceVersion)' | ||
| tagSource: 'userSpecifiedTag' | ||
| tag: '$(TGREP_TAG)' | ||
| assets: '$(Build.ArtifactStagingDirectory)/*.zip' | ||
| assetUploadMode: 'replace' | ||
| addChangeLog: false |
There was a problem hiding this comment.
When PublishToGitHub is true, both the x64 job and the arm64 job will run GitHubRelease@1 against the same tag/release in parallel. This can lead to flaky release updates and/or one job overwriting the other’s assets depending on how assetUploadMode: replace is applied. Consider publishing from a single job that depends on both builds (e.g., download both zips into one directory and upload them together).
| - ${{ if eq(parameters.PublishToGitHub, true) }}: | ||
| - task: GitHubRelease@1 | ||
| displayName: 'Publish arm64 signed binary to GitHub Release' | ||
| inputs: | ||
| gitHubConnection: 'github.com_msftsiwei_microsoftsso' | ||
| repositoryName: 'microsoft/tgrep' | ||
| action: 'edit' | ||
| target: '$(Build.SourceVersion)' | ||
| tagSource: 'userSpecifiedTag' | ||
| tag: '$(TGREP_TAG)' | ||
| assets: '$(Build.ArtifactStagingDirectory)/*.zip' | ||
| assetUploadMode: 'replace' | ||
| addChangeLog: false |
There was a problem hiding this comment.
Same as the x64 job: this job also edits/uploads to the same GitHub release. To reduce risk of concurrent edits and asset overwrites, prefer a single publish job after both build/sign jobs complete, uploading both architecture zips together.
Summary
Align
.github/workflows/sign-and-release.ymlwith the workingdp-ado-sign-tgrep.ymlpipeline in DeepPromptClientSdk.Changes
build_sign_windows_arm64): cross-compileaarch64-pc-windows-msvctarget, sign, package, and optionally publish to GitHub Releasetgrep-github→github.com_msftsiwei_microsoftsso(matches the existing working service connection in DeepPrompt ADO project)Notes
.cargo/config.tomlis unchanged — it is already checked in at repo root and used viacheckout: selfcheckout: selfpattern (no manual clone needed)addChangeLog: falseto match dp-ado-sign-tgrep.yml behavior