system-reinstall-bootc: add progress indication during install steps#2091
system-reinstall-bootc: add progress indication during install steps#2091judavi wants to merge 9 commits intobootc-dev:mainfrom
Conversation
github-actions
bot
added
the
area/system-reinstall-bootc
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the user experience by adding progress indicators for long-running operations. A spinner is now shown during the bootc_has_clean check, and a message is printed before starting the installation. The changes are well-implemented. I have one suggestion to improve robustness by replacing unwrap() with expect() to avoid potential panics.
Signed-off-by: Juan <[email protected]>
Signed-off-by: Juan <[email protected]>
Signed-off-by: Juan <[email protected]>
Signed-off-by: Juan <[email protected]>
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
Sorry for the delay in review and thank you so much for your contribution!
crates/tests-integration/Cargo.toml
Outdated
| oci-spec = "0.9.0" | ||
| rand = "0.10" | ||
| rexpect = "0.7" | ||
| rexpect = "0.6" |
There was a problem hiding this comment.
Hmm, this looks like a likely unintentional change?
There was a problem hiding this comment.
That's correct. Thank you! I'm not that used to use Codespaces for development
|
|
||
| prompt::temporary_developer_protection_prompt()?; | ||
|
|
||
| println!("Starting bootc installation. This may take several minutes..."); |
| ); | ||
| spinner.set_message("Checking image capabilities..."); | ||
| spinner.enable_steady_tick(Duration::from_millis(150)); | ||
| let has_clean = bootc_has_clean(&opts.image)?; |
There was a problem hiding this comment.
Hmm wait wait...did we end up pulling the image here? I think we did...if so that seems like the real bug. I think we should break out a clearly distinct pull phase.
There was a problem hiding this comment.
You're right bootc_has_clean was calling podman run <image> which would have implicitly pulled the image if it wasn't already cached locally. Fixed in the latest push: bootc_has_clean is now pub(crate) and called explicitly from run() in main.rs, after pull_if_not_present completes. reinstall_command is now a pure command builder that takes has_clean: bool as a parameter with no I/O side effects. The flow is now three clearly distinct phases: pull → capability check (spinner) → install
Signed-off-by: Juan Gomez <[email protected]>
Signed-off-by: Juan Gomez <[email protected]>
…tc change Assisted-by: Claude Code (claude-sonnet-4-6) Signed-off-by: Juan Gomez <[email protected]>
main.rs, so the pull and capability-check steps are clearly distinct and sequenced Signed-off-by: Juan Gomez <[email protected]>
….lock Fix missing newline at EOF in podman.rs flagged by cargo fmt. Also restore the indicatif 0.18.3 entry in Cargo.lock which was accidentally dropped when reverting unrelated Cargo.lock changes; indicatif is a real dependency of system-reinstall-bootc. Assisted-by: Claude Code (claude-sonnet-4-6) Signed-off-by: Juan Gomez <[email protected]>
2 participants
Add progress indication to the long-running steps of system-reinstall-bootc to address #1270.
This also fixes a latent bug raised in review:
bootc_has_cleanwas callingpodman run <image>which could implicitly pull the image if it wasn't already cached locally, with no clear indication to the user.The flow is now three clearly distinct phases:
bootc_has_cleanis called after the image is guaranteed to be local, with a spinner labeled "Checking image capabilities...".