refactor(xtask): unify release packaging and install lifecycle E2E - #139
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
rominf
force-pushed
the
refactor-xtask-release-lifecycle
branch
2 times, most recently
from
July 22, 2026 13:32
1cff742 to
6b800a6
Compare
rominf
marked this pull request as ready for review
July 22, 2026 15:07
rominf
marked this pull request as draft
July 23, 2026 14:25
rominf
force-pushed
the
ci-shared-windows-strix-runners
branch
from
July 24, 2026 12:23
bb89ce9 to
cf10418
Compare
Replace the platform-specific `scripts/package-{linux,windows}-release`
shell/PowerShell scripts with a single `cargo xtask package` command that
builds the release distribution bundle in pure Rust: a `.tar.gz` on Unix
and a `.zip` on Windows, each with a SHA-256 sidecar and, when a signing
key is configured, a detached signature. Signing inputs are read from the
environment for parity with the old scripts, and the external artifact
contract (bundle layout, checksum syntax, signature presence, installer
compatibility) is preserved.
Extract the duplicated workspace-path resolution (workspace root, active
target dir, release binary dir, platform binary name) that had drifted
across `e2e.rs`, `demos.rs`, and the packaging scripts into a shared
`paths` module with a unit-testable pure core, and build `rocmd` alongside
`rocm` so the packaged bundle carries both binaries.
Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
Replace the standalone `acceptance-install-upgrade-tui-uninstall` shell/PowerShell acceptance scripts with an opt-in `@lifecycle` set in the cucumber-rs E2E suite, so the package + real-installer + install / upgrade / TUI / uninstall flow is exercised through one cross-platform harness with per-scenario isolation and teardown. The scenarios drive `cargo xtask package`, run the actual `install.sh` / `install.ps1`, and assert the external contract: checksum and detached signature verification (including rejection of bad checksums, bad or missing signatures, and required-signature mode without a public key), first-install PATH setup, isolated-config smoke checks, and clean uninstall. A `LifecycleState` roots all side effects under the scenario temp dir and restores mutated OS state (notably the Windows user PATH) on drop, even if a step panics. The set is gated behind `E2E_INCLUDE_LIFECYCLE` / `E2E_ONLY_LIFECYCLE` so the default run stays fast. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
Point the release, nightly, and CI workflows at `cargo xtask package` and the `@lifecycle` E2E set instead of the removed packaging and acceptance scripts, widen the CI change-detection paths to trigger the heavy job on `xtask/` and the lifecycle sources, and update the release-trust and testing docs to describe the new packaging command and lifecycle suite. Refresh the dependency manifest and third-party notices for the archive dependencies the packaging command pulls in, and fix a stale script reference in a rocm-core doc comment. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
Resolve the clippy failures in the new lifecycle steps that broke the CI clippy gate (const fn for the state accessors, map_or_else, writeln! in place of push_str(format!), and flatten over the Option loop), and drop the needless format! in the Windows PATH-restore helper that would have tripped the Windows clippy gate. Restore three assertions that the shell/PowerShell acceptance scripts had but the cucumber port lost: the negative isolation check that examine never reads the real user ~/.rocm, the non-running rocmd binary being removed after a keep-config Windows uninstall, and that an interactive chat session does not clobber the default-engine config. Also surface a failed Windows user-PATH restore on stderr instead of silently swallowing it (it runs in Drop, so it warns rather than panics), and de-duplicate the @lifecycle / @requires-os rows in the tags table. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
The install-lifecycle scenarios invoked `cargo xtask package` and
`cargo xtask keygen` from inside the running test. On Windows that
re-enters cargo, which tries to rebuild `xtask.exe` and fails to replace
it while the harness is still executing that same binary ("Access is
denied", os error 5) — every lifecycle scenario then failed at keygen.
The harness now exports the path of the already-built xtask executable
(its own `current_exe`) as `ROCM_XTASK_BINARY`, and the steps run that
binary directly instead of shelling back through cargo. Running the
existing executable needs no rebuild and takes no lock. A standalone
`cargo test` run (no harness) falls back to `cargo xtask`, where no
xtask process is running so the rebuild is safe.
Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
Static analysis flagged the release-packaging and demo tasks for using paths derived from environment overrides (`ROCM_BIN_DIR`, `CARGO_TARGET_DIR`, the signing-key path) and CLI arguments (the bundle output dir and name) in filesystem operations without validation, so a `..` segment could point the copy/read outside the intended tree. Validate the path's string form for a `..` traversal segment and rebuild the path from the checked string before every such filesystem access. Absolute overrides and target dirs stay valid — only `..` traversal is refused — so legitimate `CARGO_TARGET_DIR`/`ROCM_BIN_DIR` values keep working. Adds unit tests covering the rejection for both the copy and require paths. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
…erver
The Windows install-lifecycle loopback download server (lifecycle_steps.rs
LoopbackServer, used by the lifecycle-windows-http-install scenario) read
the incoming HTTP request with a single read() call and treated whatever
bytes arrived as the whole request. Nothing in TCP/HTTP guarantees a
request lands in one read(): a client or the OS network stack can split it
across multiple segments/writes. When that happened, the server captured
only a prefix of the request line (e.g. "GET " with no path yet),
defaulted the parsed path to "/", and 404'd a legitimate download.
Reproduced locally (outside this fix) by forcing a raw socket to send
"GET " and the rest of the request line in two separate writes: the
server returned 404 for tests/e2e-cucumber-windows-amd64.zip instead of
200. This plausibly explains a real Windows CI failure on
lifecycle-windows-http-install ("failed to download ... An error occurred
... while sending the request"), since fragmenting a request across
writes is more a property of the client/OS TCP stack (here PowerShell's
Invoke-WebRequest on Windows) than of loopback distance.
Extract the request-head reading and parsing into a small, unit-tested
e2e_cucumber::loopback_http module: read_request_head() loops across as
many read() calls as needed until it sees the header terminator (or EOF,
or a size cap), instead of trusting a single fixed-size read(). Verified
the fix end-to-end against a real TCP loopback socket with the request
split byte-by-byte, in addition to the new unit tests covering split and
malformed requests.
Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
rominf
force-pushed
the
refactor-xtask-release-lifecycle
branch
from
July 28, 2026 10:03
0d82a01 to
2831271
Compare
rominf
marked this pull request as ready for review
July 29, 2026 06:48
7 tasks
5 tasks
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
3 participants
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.
Summary
Consolidates the release packaging and install-lifecycle verification onto a single cross-platform Rust harness, replacing four platform-specific shell/PowerShell scripts.
cargo xtask package— a new pure-Rust command that builds the release distribution bundle (a.tar.gzon Unix, a.zipon Windows), each with a SHA-256 sidecar and, when a signing key is configured, a detached signature. It packages both therocmandrocmdbinaries and preserves the external artifact contract (bundle layout, checksum syntax, signature presence, installer compatibility) of the scripts it replaces.pathsmodule — extracts the workspace-path resolution (workspace root, active target dir, release binary dir, platform binary name) that had drifted acrosse2e.rs,demos.rs, and the old packaging scripts into one module with a unit-testable pure core.@lifecycleE2E set — replaces the standaloneacceptance-install-upgrade-tui-uninstallscripts with an opt-in scenario set in the cucumber E2E suite. The scenarios drivecargo xtask package, run the realinstall.sh/install.ps1, and assert the install / upgrade / TUI / uninstall contract: checksum and detached-signature verification (including rejection of bad checksums, bad or missing signatures, and required-signature mode without a public key), first-install PATH setup, isolated-config smoke checks, and clean uninstall. All side effects are rooted under a per-scenario temp dir and mutated OS state is restored on teardown even if a step panics.cargo xtask packageand the@lifecycleset instead of the removed scripts, and widens CI change-detection so the heavy job triggers onxtask/and the lifecycle sources.Why
Packaging and install-acceptance logic was duplicated across separate Linux (shell) and Windows (PowerShell) scripts that had drifted apart and duplicated path-resolution logic already present in the xtask crate. A single cross-platform harness removes that drift, makes the packaging and lifecycle contract unit- and scenario-testable, and keeps the two platforms behaviorally in step.
Design notes
@lifecycleset is gated behindE2E_INCLUDE_LIFECYCLE/E2E_ONLY_LIFECYCLEso the defaultcargo xtask e2erun stays fast; CI opts in explicitly.pathsmodule splits a pure, unit-testable core from filesystem lookups so the resolution logic can be tested without a built workspace.Test plan
Verified on Linux:
cargo test -p xtask— 62 passed, including the newpackagetests (bundle layout, two-space lowercase-hex checksum syntax, stage-checksum-sign round trip, required-signature-without-key failure) and thepathsunit tests.cargo test -p e2e-cucumber --no-run— the lifecycle steps and installer fixture compile cleanly.E2E_INCLUDE_LIFECYCLE=1 E2E_ONLY_LIFECYCLE=1 cargo xtask e2eDeferred / not yet verified
Risk
Medium. Touches release/nightly/CI packaging paths and removes the previous acceptance scripts; the archive/layout contract is preserved and covered by unit tests, but the Windows path needs the CI runner to confirm.