diff --git a/.github/workflows/nightly-performance.yml b/.github/workflows/nightly-performance.yml index 6727c97..14548e1 100644 --- a/.github/workflows/nightly-performance.yml +++ b/.github/workflows/nightly-performance.yml @@ -18,6 +18,22 @@ on: # 02:00 UTC daily. - cron: '0 2 * * *' workflow_dispatch: + inputs: + dryrun: + type: boolean + default: false + common-ci-ref: + type: string + description: "common-ci branch/tag/sha to use (default: main)" + default: '' + dd-cxx-ref: + type: string + description: "device-detection-cxx branch/tag/sha to use (default: main)" + default: '' + ipi-cxx-ref: + type: string + description: "ip-intelligence-cxx branch/tag/sha to use (default: main)" + default: '' jobs: performance: @@ -41,16 +57,19 @@ jobs: path: rust submodules: recursive - # 2. The shared comparison step, which reads the results of earlier runs - # and charts them. The performance adapter itself lives in this - # repository, in ci/, because it serves only this repository and a - # folder named rust in common-ci collides with the clone of a - # repository named rust (51Degrees/rust#50). + # 2. common-ci, checked out for the shared comparison step + # (steps/compare-performance.ps1). The Rust performance adapter itself + # now lives in this repo at ci/run-performance-tests.ps1 (see + # ci/missing-ci-scripts.md), because it serves only this repository and + # a folder named rust in common-ci collides with the clone of a + # repository named rust (51Degrees/rust#50); only the comparison step + # is shared. - name: Checkout common-ci uses: actions/checkout@v4 with: repository: 51Degrees/common-ci path: common-ci + ref: ${{ inputs.common-ci-ref }} # 3. The C/C++ source repositories the -sys crates compile, plus the data # submodules the performance examples read. They are checked out as @@ -65,6 +84,7 @@ jobs: with: repository: 51Degrees/device-detection-cxx path: device-detection-cxx + ref: ${{ inputs.dd-cxx-ref }} submodules: recursive - name: Checkout ip-intelligence-cxx @@ -72,6 +92,7 @@ jobs: with: repository: 51Degrees/ip-intelligence-cxx path: ip-intelligence-cxx + ref: ${{ inputs.ipi-cxx-ref }} submodules: recursive # 3a. The data files (.hash, .ipi) and the User-Agent / evidence records are @@ -128,15 +149,27 @@ jobs: # `main`, commit them to the `gh-images` branch (other branches use a # `perf-images/` prefix). The documentation benchmarks page # embeds those images by raw URL, so without `-Publish` the Rust graphs - # never appear. Best-effort: on a repository without run history the - # comparison has too few points and simply records the current figures, - # so a failure here must not fail the job. + # never appear. `-DryRun` still renders and commits the graphs locally + # (so the full render path is exercised) but suppresses the final + # `git push`, so a manually dispatched dry run leaves nothing outside + # this CI job. The `dryrun` input is empty on scheduled runs, which + # parse to `false`, so the nightly cron always publishes. + # + # This step is allowed to fail the job. The shared + # compare-performance.ps1 exits 0 when it succeeds (including the benign + # "not enough history yet" case) and non-zero otherwise, whether that is + # a genuine performance regression or an infrastructure failure such as + # an inability to switch to the images branch. Neither should be + # silently swallowed, so `continue-on-error` is false. - name: Compare performance and render graphs - continue-on-error: true + continue-on-error: false shell: pwsh env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | + # The dryrun input is only set on manual dispatch; default it to + # false so scheduled runs publish normally. + $DryRun = [bool]::Parse( "${{ inputs.dryrun || 'false' }}" ) # Committing the rendered graphs onto the gh-images branch needs a git # identity, which a bare runner checkout does not configure; use the # github-actions bot identity so the commit succeeds. @@ -150,19 +183,37 @@ jobs: @{ Name = "DeviceDetection-OnPremise"; RunPerformance = $true }, @{ Name = "IpIntelligence-OnPremise"; RunPerformance = $true } ) + # The performance run resolves examples/Cargo.lock against source.toml + # (the local-path patch), which rewrites this tracked lockfile. The + # shared compare step publishes graphs by switching to an orphan images + # branch, and git aborts that switch while a tracked file has + # uncommitted changes. Restore the committed lockfile so the tree is + # clean before the switch. Only this one file is touched by the run; + # git's "changes would be overwritten" error enumerates every colliding + # tracked file, and it lists only examples/Cargo.lock. + git -C rust checkout -- examples/Cargo.lock ./common-ci/steps/compare-performance.ps1 ` -RepoName 'rust' ` -OrgName '51Degrees' ` -AllOptions $options ` -Branch '${{ github.ref_name }}' ` + -DryRun $DryRun ` -Publish + # Runs even when the compare step failed. That upload is the only source + # of future baselines, so skipping it on a failing compare would drop the + # run's own figure out of history and wedge the trend (see below). - name: Escape Branch Name id: escape_branch + if: always() shell: pwsh run: '"name=" + ($env:GITHUB_REF_NAME -replace ''[":<>|*?/\\\r\n]'', ''-'') | Out-File $env:GITHUB_OUTPUT -Append' + # Runs even when the compare step failed (see above), but not on a dry + # run: this artifact is the performance history, and a dry run must leave + # nothing outside its own CI job, so it must not inject a figure here. - name: Upload Graphed Performance Results + if: always() && github.event.inputs.dryrun != 'true' uses: actions/upload-artifact@v4 with: name: publish_performance_results@${{ steps.escape_branch.outputs.name }} diff --git a/AGENTS.md b/AGENTS.md index 4652688..a1e56d9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -119,6 +119,12 @@ manifest's `include`, so `cargo publish` packages it. a real drift is still reported. 51Degrees/rust#52 lifts the pin, by moving the bundled asset and `UPSTREAM_PINNED_REF` to the same commit in one pull request. +- `nightly-performance.yml` — nightly (and on-demand) on-premise throughput + run that feeds the documentation performance graphs. Its Rust adapter lives + in this repo at `ci/run-performance-tests.ps1`; only the shared comparison + step (`compare-performance.ps1`) comes from common-ci. See + [ci/missing-ci-scripts.md](ci/missing-ci-scripts.md) for how this relates to + the org-standard reusable-workflow contract and the planned direction. ## Conventions and gotchas diff --git a/ci/missing-ci-scripts.md b/ci/missing-ci-scripts.md new file mode 100644 index 0000000..e750d22 --- /dev/null +++ b/ci/missing-ci-scripts.md @@ -0,0 +1,97 @@ +# Missing CI scripts and the road to the shared reusable workflows + +This note records why the Rust repository's CI diverges from the rest of the +51Degrees organisation today, what the organisation's standard contract looks +like, and the incremental direction for closing the gap. It exists so the next +person (or agent) does not have to re-derive the layout from scratch, and so the +docs that must be kept current (see `AGENTS.md`) point at a single source of +truth. + +## The organisation standard (how the other repos work) + +Consuming repositories keep their GitHub Actions YAML deliberately thin. They do +not `run:` common-ci PowerShell directly; instead they `uses:` common-ci's +*reusable workflows*. For example, `device-detection-cxx/.github/workflows`: + +``` +nightly-pipeline.yml + └─ uses: 51Degrees/common-ci/.github/workflows/nightly-pull-requests.yml@main + └─ uses: nightly-pull-request.yml (per PR: Configure → BuildAndTest + │ → ComparePerformance → Complete) + └─ runs: nightly-pull-request.build-and-test.ps1 +``` + +The orchestrator `nightly-pull-request.build-and-test.ps1` then calls back into +**repo-local** hook scripts by a fixed naming convention: + +``` +.//ci/fetch-assets.ps1 +.//ci/setup-environment.ps1 +.//ci/build-project.ps1 +.//ci/run-unit-tests.ps1 +.//ci/run-integration-tests.ps1 +.//ci/run-performance-tests.ps1 # only when Options.RunPerformance +``` + +plus a `ci/options.json` describing the build matrix. In short: common-ci owns +the *orchestration*; each repo owns a set of `ci/.ps1` *customization +entry points*. The only common-ci script a repo's YAML runs directly is the +generic linter `scripts/utm-lint.ps1`. + +## Where the Rust repo stands today + +The Rust repo has **not** joined the shared orchestration. It carries its own +self-contained `.github/workflows/nightly-performance.yml`, which the workflow +header itself notes is "kept self-contained here (rather than calling the shared +reusable workflow) until the repository joins the shared nightly orchestration." + +Historically its performance adapter also lived in the wrong repository: +`common-ci/rust/run-performance-tests.ps1`. That was an anomaly — common-ci +exists to hold code shared *across* repositories, and the Rust repo is the only +Rust consumer, so there is nothing to share. Every other language keeps its +`run-performance-tests.ps1` under its own `/ci/`. + +## What this change does (Phase 1) + +- Adds the Rust performance adapter to this repo at + `ci/run-performance-tests.ps1`, matching the org's `/ci/.ps1` + convention. This is the customization entry point that further repo-specific + work will build on. +- Repoints `nightly-performance.yml` to run `./rust/ci/run-performance-tests.ps1` + (the workflow checks this repo out into a `rust/` subdirectory) instead of + `./common-ci/rust/run-performance-tests.ps1`. +- Keeps the `Checkout common-ci` step, because the comparison step + `steps/compare-performance.ps1` is genuinely shared and still comes from + common-ci. + +Moving the adapter's home is the structural part of the change. Alongside it, +this repo's copy has since diverged from the common-ci original with +repository-specific fixes (the ASN data fetch, the `dryrun` input, the +`common-ci-ref`/`dd-cxx-ref`/`ipi-cxx-ref` inputs, and letting the compare step +fail the job), so the behaviour of the nightly run is not identical to the old +`common-ci/rust` path. + +The copy in `common-ci/rust/run-performance-tests.ps1` is intentionally **left +in place** for now and will be removed in a separate, manually raised common-ci +PR. This repo's copy is the one CI uses, and it is now the source of truth: the +two copies are no longer identical, so the common-ci copy should not be edited +in place — it is only awaiting deletion. + +## The remaining gap (Phase 2, deferred) + +Fully aligning with the organisation standard would mean: + +- Providing the complete `ci/` verb set (`build-project.ps1`, + `run-unit-tests.ps1`, `run-integration-tests.ps1`, `setup-environment.ps1`, + `fetch-assets.ps1`, `run-performance-tests.ps1`) and a `ci/options.json` + build matrix. +- Replacing the bespoke `nightly-performance.yml` with a thin + `nightly-pipeline.yml` that `uses:` the common reusable workflows, letting + common-ci drive build, test, performance and publish uniformly with the other + languages. + +This is a much larger change that alters how the Rust repo is built and tested +in CI across the whole organisation, and it carries open-ended maintenance until +the shared workflows fully accommodate a Cargo-workspace consumer. It is +recorded here as a direction, not scheduled work. Weigh that cost explicitly +before starting it. diff --git a/ci/run-performance-tests.ps1 b/ci/run-performance-tests.ps1 index 781f70e..990185a 100644 --- a/ci/run-performance-tests.ps1 +++ b/ci/run-performance-tests.ps1 @@ -10,6 +10,35 @@ param( $ErrorActionPreference = "Stop" $PSNativeCommandUseErrorActionPreference = $true +# The ip-intelligence-cxx checkout is a sibling of the repo directory (CI +# checks this repo out into $RepoName next to it; a local run passes "." from +# the repo root, whose parent is the workspace holding the sibling checkout). +# Resolve the ASN data directory against $RepoName rather than the caller's +# current directory, so both invocations find it. +$AsnDataDir = Join-Path $RepoName "../ip-intelligence-cxx/ip-intelligence-data" +Push-Location $AsnDataDir +try { + Write-Host "Entering $PWD" + # Remove old Asn file (if exists) + $AsnFilePath = "51Degrees-IPIV4AsnIpiV41.ipi" + if (Test-Path -Type Leaf -Path $AsnFilePath) { + Remove-Item -Path $AsnFilePath + Write-Host "Deleted $AsnFilePath" + } + + Write-Host "Loading free IPI data files..." + # -Force re-downloads the .gz archive rather than re-extracting whatever is + # already on disk: the Azure script gates its download on the .gz, not the + # .ipi, so on a persisted workspace a stale archive would otherwise be + # silently re-extracted and the delete above would refresh nothing. -Asn + # fetches the ASN file the performance example reads + # (51DEGREES_IPI_PATH). + & ./get-lite-file-from-azure.ps1 -Force -Asn +} finally { + Write-Host "Leaving $PWD" + Pop-Location +} + # Runs the 51Degrees Rust on-premise performance examples in release and writes # their throughput figures into results_.json files, in the same # `{ HigherIsBetter = @{ metric = value } }` shape the shared