Add SemVer checks #3375
Add SemVer checks #3375
Conversation
danieljurek
force-pushed
the
djurek/semver-checks
branch
2 times, most recently
from
c190716 to
726de70
Compare
danieljurek
force-pushed
the
djurek/semver-checks
branch
from
726de70 to
f84e0e2
Compare
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
There was a problem hiding this comment.
Pull request overview
This PR adds semantic versioning (SemVer) compatibility checks to the Azure SDK for Rust build pipeline using cargo-semver-checks. The changes ensure that package version updates comply with SemVer rules before releases.
Key changes:
- Introduces a new PowerShell script to check SemVer compatibility of packages
- Extracts common cargo metadata functions to a shared module for reuse
- Adds cgmanifest.json to track the cargo-semver-checks tool version used in builds
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/scripts/shared/Cargo.ps1 | New shared module containing reusable cargo metadata and package querying functions |
| eng/scripts/Test-Semver.ps1 | New script that installs cargo-semver-checks and validates package changes against SemVer rules |
| eng/scripts/Pack-Crates.ps1 | Refactored to use shared cargo functions from Cargo.ps1, eliminating duplication |
| eng/pipelines/templates/jobs/pack.yml | Adds SemVer check tasks to both auto and manual service directory build scenarios |
| eng/cgmanifest.json | New manifest tracking cargo-semver-checks version 0.45.0 as a development dependency |
| $versionParams += '--version' | ||
| $cgManfiest = Get-Content ([System.IO.Path]::Combine($PSScriptRoot, '..', 'cgmanifest.json')) ` | ||
| | ConvertFrom-Json | ||
| $versionParams += $cgManfiest. |
There was a problem hiding this comment.
Typo: "CgManfiest" should be "CgManifest" (missing 'i').
| filePath: $(Build.SourcesDirectory)/eng/scripts/Test-Semver.ps1 | ||
| arguments: -PackageNames $(PackageNames) | ||
|
|
||
|
|
There was a problem hiding this comment.
Extra blank line that should be removed for consistency with the rest of the file formatting.
| function Get-OutputPackageNames($workspacePackages) { | ||
| $packablePackages = $workspacePackages | Where-Object -Property publish -NE -Value @() | ||
| $packablePackageNames = $packablePackages.name | ||
|
|
||
| $names = @() | ||
| switch ($PsCmdlet.ParameterSetName) { | ||
| 'Named' { | ||
| $names = $PackageNames | ||
| } | ||
|
|
||
| 'PackageInfo' { | ||
| $names = Get-PackageNamesFromPackageInfo $PackageInfoDirectory | ||
| } | ||
|
|
||
| default { | ||
| return $packablePackageNames | ||
| } | ||
| } | ||
|
|
||
| foreach ($name in $names) { | ||
| if (-not $packablePackageNames.Contains($name)) { | ||
| Write-Error "Package '$name' is not in the workspace or does not publish" | ||
| exit 1 | ||
| } | ||
| } | ||
|
|
||
| return $names | ||
| } | ||
|
|
There was a problem hiding this comment.
The Get-OutputPackageNames function is duplicated between this file and Pack-Crates.ps1 (lines 59-86). This function should be extracted to the shared Cargo.ps1 module to avoid code duplication and maintain consistency.
| function Get-OutputPackageNames($workspacePackages) { | |
| $packablePackages = $workspacePackages | Where-Object -Property publish -NE -Value @() | |
| $packablePackageNames = $packablePackages.name | |
| $names = @() | |
| switch ($PsCmdlet.ParameterSetName) { | |
| 'Named' { | |
| $names = $PackageNames | |
| } | |
| 'PackageInfo' { | |
| $names = Get-PackageNamesFromPackageInfo $PackageInfoDirectory | |
| } | |
| default { | |
| return $packablePackageNames | |
| } | |
| } | |
| foreach ($name in $names) { | |
| if (-not $packablePackageNames.Contains($name)) { | |
| Write-Error "Package '$name' is not in the workspace or does not publish" | |
| exit 1 | |
| } | |
| } | |
| return $names | |
| } |
| [string[]]$PackageNames, | ||
| [Parameter(ParameterSetName = 'PackageInfo')] | ||
| [string]$PackageInfoDirectory, | ||
| [switch]$IgnoreCgManfiestVersion |
There was a problem hiding this comment.
Typo: "CgManfiest" should be "CgManifest" (missing 'i'). This parameter name is inconsistent with the variable name used on line 53.
| # Read version from cgmanifest.json. If ignored the currently installed or | ||
| # "latest" version is used. | ||
| $versionParams = @() | ||
| if (!$IgnoreCgManfiestVersion) { |
There was a problem hiding this comment.
Typo: "CgManfiest" should be "CgManifest" (missing 'i'). This references the parameter from line 10 which also has the typo.
| $versionParams = @() | ||
| if (!$IgnoreCgManfiestVersion) { | ||
| $versionParams += '--version' | ||
| $cgManfiest = Get-Content ([System.IO.Path]::Combine($PSScriptRoot, '..', 'cgmanifest.json')) ` |
There was a problem hiding this comment.
Typo: "CgManfiest" should be "CgManifest" (missing 'i').
eng/cgmanfiest.jsonto track installation of a dev tool in the build systemTest-Semver.ps1uses the version specified ineng/cgmanifest.jsonTested:
Test performance
SemVer checks add a non-trivial amount of time in some OS configurations (Windows, MacOS) because
rust-semver-checksis built.rust-semver-checkbuild times (look at logs for builds of stable in this PR run):Caching considerations
Azure DevOps caches are scoped according to several factors. In the case of a PR where speed is most important, we would need a job that runs against
mainto populate a cache that PRs could read. We don't currently schedule PR pipelines to run. In fact, vcpkg, uses its own cache mechanism with relevant rules.If we needed to semver check across OS quickly, we could use a storage account to hold pre-built binaries (similar to vcpkg). The binaries would be built in a Rust pipeline and uploaded to the storage account with a path prefix like
carge-semver-checks/ubuntu-24/0.45.0/. Something would need to update the cache and cgmanifes.json file and proper arrangements would need to be made locally so that callingcargo semver-checkswould use the correct binary.Similar work could also be done to cache source analysis binaries
Other considerations
Checking "nightly" and "msrv" versions of Rust is more complicated. Generally,
cargo-semver-checkssupports "stable" so SemVer checking focuses there.