fix(release): package runnable binary archives #175
No reviewers
Labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Slipstream/auto_review!175
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/issue-163-runnable-release-artifact"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Verification
nix develop -c cargo test -p ar-gateway embedded_oci_gatewaybash tests/release_tooling/publish_workflow_test.shbash tests/release_tooling/ci_pr_artifacts_test.shnix build .#checks.x86_64-linux.release-tooling --no-linknix develop -c cargo fmt --all -- --checknix develop -c cargo clippy -p ar-gateway --all-targets -- -D warningsauto-review --version,bin/youki --version./auto-review --version,./bin/youki --versionCloses #163
PR artifacts for
980cbc576f:Docker image: git.johnwilger.com/jwilger/auto_review/ar-gateway-pr:pr-175-980cbc576fae13890ee04c502a5cef8c3b851a47
binary download: https://git.johnwilger.com/api/packages/jwilger/generic/auto-review-pr/175-980cbc576fae13890ee04c502a5cef8c3b851a47/auto-review-pr-175-980cbc576fae13890ee04c502a5cef8c3b851a47-linux-x86_64.tar.gz
SHA256SUMS: https://git.johnwilger.com/api/packages/jwilger/generic/auto-review-pr/175-980cbc576fae13890ee04c502a5cef8c3b851a47/SHA256SUMS
The PR refactors the packaging process for Linux binary archives, ensuring they include the necessary runtime components. It updates the CI and release workflows to accommodate these changes and adds tests to verify the new packaging logic. The changes appear safe to merge, but ensure that all paths and variables are correctly set and validated.
Walkthrough
CI Workflow Changes:
x86_release_rootfor packaging, ensuring the correct components are included in the archive.x86_release_rootvariable is correctly set and used.Release Workflow Changes:
x86_release_rootvariable is correctly set and used.Startup Validation:
validate_packaged_nix_store_pathwithvalidate_packaged_oci_input_pathsto handle new validation logic.Flake Configuration:
ar-cli-portable-release-rootto the flake configuration to support the new packaging structure.Testing:
🟡 Warning: Ensure that the
x86_release_rootvariable is correctly set and used throughout the script. Verify that the path is valid and accessible in the context where it's used.Reflection: I relied on the existing workflow checks to cover the new variable, but they only proved the portable package marker existed; they did not assert that
x86_release_rootwas assigned before it was used for archiving. Classification: one-off missing test pressure, addressed with a focused release-tooling assertion. Remediation: added CI workflow coverage that verifiesx86_release_rootis assigned from.#packages.x86_64-linux.ar-cli-portable-release-root, and that the archive is created from that variable after assignment. Pushed in990dcfd. Verified withbash tests/release_tooling/ci_pr_artifacts_test.shandnix build .#checks.x86_64-linux.release-tooling --no-link.🟡 Warning: Check that the
x86_release_rootvariable is correctly set and used throughout the script. Ensure that the path is valid and accessible in the context where it's used.Reflection: I validated the portable package with smoke tests, but the release workflow contract test did not explicitly couple
x86_release_rootassignment to every later use. Classification: one-off missing test pressure rather than a new guardrail gap. Remediation: tightened the publish workflow test to requirex86_release_rootto be assigned from the portable x86 release package before both archiving and metadata generation use it. Pushed in990dcfd. Verified withbash tests/release_tooling/publish_workflow_test.shandnix build .#checks.x86_64-linux.release-tooling --no-link.New commits pushed, approval review dismissed automatically according to repository settings
The PR enhances the release process by packaging Linux binary archives from a portable release root and updating CI tests to ensure proper artifact generation. The changes appear safe to merge, with added checks for CI workflow steps.
Walkthrough
Δ since
62012b0:x86_release_rootis correctly assigned and used in the CI scripts to prevent errors.🟡 Warning: Lines 6–26: Ensure that the CI workflow includes a PR binary artifact build step. This is crucial for verifying that the build process is correctly generating the necessary artifacts for PRs.
Reflection: I let the follow-up review rediscover release-tooling expectations that are already enforced by the updated contract test instead of calling that out in the PR context. Classification: one-off stale/duplicate feedback rather than a guardrail gap. Remediation: verified the CI workflow contract already requires the PR artifact build job and PR binary artifact publication markers; no code change needed. Verification:
bash tests/release_tooling/ci_pr_artifacts_test.shpassed.🟡 Warning: Lines 9–32: Check that the CI PR binary artifact step assigns
x86_release_rootfrom the portable x86 release package build. This ensures that the correct build output is being archived.🟡 Warning: Lines 9–32: Verify that the CI PR binary artifact step archives from the
x86_release_rootpath. This is necessary to ensure that the correct files are included in the archive.🟡 Warning: Lines 9–32: Ensure that
x86_release_rootis set before archiving from it in the CI PR binary artifact step. This prevents potential errors due to uninitialized variables.Reflection: The first remediation tightened this exact invariant, but the new review comment did not see that the assertion lives in the release-tooling test rather than only in the workflow diff. Classification: one-off duplicate feedback. Remediation: confirmed the test requires
x86_release_rootto be assigned from.#packages.x86_64-linux.ar-cli-portable-release-root. Verification:bash tests/release_tooling/ci_pr_artifacts_test.shpassed.Reflection: I previously focused the assertion on assignment ordering, but the same test also needed to make clear that archiving uses the resolved release-root path. Classification: one-off duplicate feedback already covered by the focused test. Remediation: confirmed the contract test requires
tar -C "$x86_release_root"in the PR binary artifact step. Verification:bash tests/release_tooling/ci_pr_artifacts_test.shpassed.Reflection: The original issue was missing ordering pressure around
x86_release_root; this follow-up correctly points at the same failure mode, but the pushed remediation already added that ordering assertion. Classification: one-off duplicate feedback. Remediation: confirmed the CI contract test fails if archiving appears before thex86_release_rootassignment. Verification:bash tests/release_tooling/ci_pr_artifacts_test.shpassed.🟡 Warning: Lines 9–32: Ensure that
x86_release_rootis set before metadata generation uses it in the binary artifact step. This prevents potential errors due to uninitialized variables.Reflection: I initially under-specified the release publish contract around metadata generation, which let the same variable-ordering concern recur. Classification: one-off missing test pressure, already remediated in the updated publish workflow test. Remediation: confirmed the test requires
x86_release_rootassignment before both archive creation and metadata generation use it. Verification:bash tests/release_tooling/publish_workflow_test.shpassed.New commits pushed, approval review dismissed automatically according to repository settings
The PR updates documentation to specify that replies to inline comments should begin with an @-mention of the original comment author. This change ensures that the reviewer is notified and the thread remains attributable. The changes appear safe to merge.