Skip to content

Conversation

@henrywang
Copy link
Collaborator

@henrywang henrywang commented Dec 2, 2025

Pin bcvk to 0.8.0.

@bootc-bot bootc-bot bot requested a review from jmarrero December 2, 2025 07:07
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the hardcoded GitHub Actions run ID to download the bcvk binary, fixing a CI failure due to an expired artifact. While this fixes the immediate issue, I've suggested a more robust approach to prevent this from happening again. My feedback focuses on replacing the hardcoded run ID with a dynamic command to fetch the latest artifact, which will improve the reliability and maintainability of the CI workflow.

Comment on lines 88 to 89
# Install bcvk from PR 163
gh run download 19682860308 --name bcvk-binary --repo bootc-dev/bcvk
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Hardcoding the run_id for downloading the bcvk binary is brittle because workflow run artifacts expire (as noted in the PR description). This will cause the CI to fail again in the future.

To make this more robust, you can dynamically fetch the artifact from the latest successful workflow run on the main branch. This avoids tying the CI to a specific run that will eventually expire.

A more permanent solution would be for the bootc-dev/bcvk repository to publish bcvk as a GitHub Release asset. You could then use gh release download to fetch it, which is the standard practice for distributing binaries and is more stable than relying on workflow run artifacts.

        # Install bcvk from the latest successful CI run on the main branch (adjust workflow name if needed)
        gh run download $(gh run list --repo bootc-dev/bcvk --workflow 'ci.yml' --branch main --status success --limit 1 --json id -q '.[0].id') --name bcvk-binary --repo bootc-dev/bcvk

@henrywang
Copy link
Collaborator Author

Hi @cgwalters, I think we need a new bcvk release. This archives will be expired in two days.

@cgwalters
Copy link
Collaborator

Hi @cgwalters, I think we need a new bcvk release. This archives will be expired in two days.

There's a PR over in bootc-dev/bcvk#160 that just needs approval/merge at this point

@cgwalters
Copy link
Collaborator

Let's do bootc-dev/infra#51 instead

Signed-off-by: Xiaofeng Wang <henrywangxf@me.com>
@henrywang henrywang changed the title ci: Update bcvk binary download source ci: Pin bcvk to 0.8.0 Dec 2, 2025
@henrywang
Copy link
Collaborator Author

Already to pin bcvk to 0.8.0.

@cgwalters cgwalters enabled auto-merge (rebase) December 2, 2025 15:46
tar xzf ${target}.tar.gz
sudo install -T ${target} /usr/bin/bcvk
# Install bcvk
gh run download 19682860308 --name bcvk-binary-tests --repo bootc-dev/bcvk
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this part is unnecesary now.

BTW, the next run of https://github.com/bootc-dev/infra/actions/workflows/sync-common.yml should have done a PR to sync, but it failed. I'm looking at that.

In this PR you can just do it manually - update this action.yml to exactly what's in the infra repo.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OH, I know.

@henrywang henrywang closed this Dec 2, 2025
auto-merge was automatically disabled December 2, 2025 15:57

Pull request was closed

@henrywang henrywang deleted the fix_bcvk branch December 2, 2025 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants