Build both Linux libc flavours per architecture on one agent - #394
Draft
Saurabh Singh (saurabh500) wants to merge 6 commits into
Draft
Build both Linux libc flavours per architecture on one agent#394Saurabh Singh (saurabh500) wants to merge 6 commits into
Saurabh Singh (saurabh500) wants to merge 6 commits into
Conversation
cargo records a dependency that has a version but no registry as coming from crates.io. mssql-tds is not on crates.io, so every published mssql-mock-tds version resolves to "no matching package named mssql-tds found, location searched: crates.io index" for anyone consuming the feed. CI never caught this because .cargo/config.ci.toml source-replaces crates.io with the feed, which silently redirects the lookup back to a place the crate does exist. Pass the feed's sparse index through to the pin script and emit it as registry-index on the dependency. Use the plain index URL rather than the ~force-auth variant so anonymous readers of the public feed still resolve, and reject ~force-auth outright since that mistake would only surface for consumers long after the publish succeeded. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7e506d0f-cc6b-451c-a641-095c469480d6
The sandbox publish pipeline fanned out into seven concurrent build jobs, six of which wanted self-hosted RUST pool agents. The pools could not satisfy that burst: run 169925 spent roughly 46 minutes waiting for an agent after preflight finished, longer than any individual build step. The manylinux and musllinux jobs for a given architecture already wanted the same pool, the same demands and the same setup steps, and they call one build script that is parameterized by container image. Merge each same-architecture pair into a single job that runs both container builds, taking concurrent build agents from 7 to 5 and self-hosted ones from 6 to 4. Each flavour now writes to its own output subdirectory, and the build script asserts on a wheel matching an expected platform tag instead of counting wheels. Without that, the second invocation in a merged job would count the first invocation's wheels and could never fail. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 611abe0f-df0d-4345-8d2a-396068346e16
The governed Linux job template permits a single published artifact per job, named from ob_artifactSuffix. Gather both libc flavours into one artifact directory instead of publishing them separately. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 611abe0f-df0d-4345-8d2a-396068346e16
…istry' into dev/saurabh/pipeline-agent-consolidation
Run 170165 failed with 'No mock wheel matching platform tag manylinux' while the container build had succeeded. The in-container build passes --auditwheel skip, so maturin never retags glibc wheels manylinux_*; they keep the default linux_<arch> tag. The assertion expected a string that never appears and so fired on healthy output. Assert on the emitted tags instead: linux_x86_64 / linux_aarch64 for the manylinux containers, musllinux_1_2_<arch> for the musllinux ones. The two are mutually exclusive, so a musl wheel cannot satisfy the glibc check and the second invocation still cannot pass on the first's output. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 611abe0f-df0d-4345-8d2a-396068346e16
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changesNo lines with coverage information in this diff. 🔗 Quick Links |
…agent-consolidation # Conflicts: # .pipeline/scripts/pin-mssql-tds-dep-version.ps1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The sandbox publish pipeline fanned out into seven concurrent build jobs, six of which wanted self-hosted RUST pool agents. The pools could not satisfy that burst — run 169925 spent roughly 46 minutes waiting for an agent after preflight finished, longer than any individual build step in the run.
The manylinux and musllinux jobs for a given architecture already wanted the same pool, the same demands and the same setup steps, and they call one build script parameterized by container image. This merges each same-architecture pair into a single job that runs both container builds.
RUST-X64-WUS3(self-hosted)RUST-ARM64-WUS3(self-hosted)Self-hosted demand drops by a third. Wheel output is unchanged: still seven wheels, now from five build jobs.
The check that would have gone blind
build-mock-wheel-linux-host.shended withfind … -name '*.whl' | wc -land failed only on zero. With two invocations in one job, the second would count the first invocation's wheels and pass even if it produced nothing — a check that cannot fail.It now takes a required
EXPECTED_PLATFORM_TAGand asserts a wheel matching that tag exists. Required rather than optional, so a future caller cannot silently opt out. Each flavour also writes to its own output subdirectory, so the two builds cannot see each other's wheels at all.Deviation from the original plan
The plan called for two published artifacts per merged job, keeping the existing artifact names. previewRun rejected that: the governed Linux job template permits exactly one artifact per job, named from
ob_artifactSuffix. Both flavours are now gathered into one artifact directory before publishing.collect-wheels.ps1recurses for*.whl, soUploadPythonis unaffected.Trade-off
The two container builds run serially instead of side by side, offset by dropping one duplicated agent setup pass per merged pair. Against ~46 minutes of queue wait, that is worth it whenever the pools are contended.
A
strategy: matrixwas considered and rejected — it spawns one agent per leg, so it reduces nothing.Related Issues
Fixes #392
Follow-up tracked separately in #393 (cross-compiling the Windows ARM64 wheel from the x64 agent, which would take peak to 4). Deliberately not attempted here: it needs a real run to confirm the
RUST-W22-SQL25image hasVC.Tools.ARM64, and cross-compiling removes the accidental smoke test that building natively gives us.Validation
What I actually exercised:
The platform-tag assertion, against the real script with the docker wrapper stubbed. Six cases, all as expected:
EXPECTED_PLATFORM_TAGunsetI broke the mechanism rather than just reverting the fix: on the third case the old count-based check passes (blind) while the new one fails. That is precisely the blind spot merging introduces. The healthy cases confirm it does not fire on good input.
previewRunon definition 2293 against this branch — returnsruns/-1/buildId=-1, so no real run was created. The expanded YAML confirms all four Linux builds are present with correct tags, andUploadPython'sdependsOnresolves to the five surviving job names with no stale entries. The first previewRun attempt is what caught the one-artifact-per-job constraint.Caller audit. Grepped the whole repo for
build-mock-wheel-linux-host— exactly four call sites, all in the two merged jobs, all passEXPECTED_PLATFORM_TAG. No caller missed by making it required.What I did not exercise: the container builds themselves. A green previewRun proves the YAML expands and says nothing about whether the wheels build. This pipeline has publish side effects, so I did not queue a real run. First real run should be watched for the serial-build wall clock and for disk headroom, since a merged agent now holds two cargo target trees (gnu and musl land in different subdirectories, so they do not collide, but they do both occupy disk).
Checklist
cargo bfmtpasses — no Rust changed; pre-commit hook ran cleancargo bclippypasses — no Rust changedcargo btestpasses — no Rust changedUpdate: platform-tag assertion corrected after run 170165
The first real run of this branch (170165) failed in the merged Linux ARM64 job. The container
build succeeded and produced a wheel, then the new assertion rejected it:
Cause:
scripts/build-mock-python-wheel-in-container.shpasses--auditwheel skip, so maturinnever retags glibc wheels as
manylinux_*. They keep the defaultlinux_<arch>tag. Theassertion expected a string that never appears, so it fired on healthy output. The x64 job had the
same defect and would have failed too; ARM64 just got there first.
This was my error, and it is worth naming how it got through: I validated the assertion against
wheel filenames I made up, rather than against real maturin output. The test double did not
reproduce real behaviour, so the passing case proved nothing.
Fixed by asserting on the tags actually emitted:
BuildLinux_x64linux_x86_64BuildLinux_x64musllinux_1_2_x86_64BuildLinux_arm64linux_aarch64BuildLinux_arm64musllinux_1_2_aarch64The two tags are mutually exclusive —
musllinux_1_2_x86_64does not containlinux_x86_64(the
linuxin it is followed by_1_2_) — so a musl wheel cannot satisfy the glibc check. Thatkeeps the property the assertion exists for: the second invocation in a merged job still cannot
pass on the first invocation's wheels.
Verified against the real filenames recorded in runs 170138 and 170165, in both directions:
linux_x86_64musllinux_1_2_x86_64previewRunon definition 2293 re-validated against this branch: expands clean,UploadPythonstill lists the five merged job names.
Note that
previewRuncannot catch this class of bug at all — it proves YAML expansion and nothingabout what the containers produce. Only a real run confirms the fix.