Skip to content

cli: Add a --check flag for update #105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Required dependencies

In order to build `bootc` you will need the following dependencies.

Fedora:
```
sudo dnf install ostree-libs ostree-devel
```

# Pre flight checks

Makes sure you commented your code additions, then run
```
cargo fmt
cargo clippy
```
Make sure to apply any relevant suggestions.

42 changes: 34 additions & 8 deletions lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,13 @@ pub(crate) struct UpgradeOpts {

#[clap(long)]
pub(crate) touch_if_changed: Option<Utf8PathBuf>,

/// Check if an update is available without applying it
#[clap(long)]
pub(crate) check: bool,
}

/// Perform an upgrade operation
/// Perform an switch operation
#[derive(Debug, Parser)]
pub(crate) struct SwitchOpts {
/// Don't display progress
Expand All @@ -57,7 +61,7 @@ pub(crate) struct SwitchOpts {
pub(crate) target: String,
}

/// Perform an upgrade operation
/// Perform a status operation
#[derive(Debug, Parser)]
pub(crate) struct StatusOpts {
/// Output in JSON format.
Expand Down Expand Up @@ -281,14 +285,36 @@ async fn upgrade(opts: UpgradeOpts) -> Result<()> {
let commit = booted_deployment.csum();
let state = ostree_container::store::query_image_commit(repo, &commit)?;
let digest = state.manifest_digest.as_str();
let fetched = pull(repo, &imgref, opts.quiet).await?;

if fetched.merge_commit.as_str() == commit.as_str() {
println!("Already queued: {digest}");
return Ok(());
}
if opts.check {
// pull the image manifest without the layers
let config = Default::default();
let mut imp = ostree_container::store::ImageImporter::new(repo, &imgref, config).await?;
match imp.prepare().await? {
PrepareResult::AlreadyPresent(c) => {
println!(
"No changes available for {}. Latest digest: {}",
imgref, c.manifest_digest
);
return Ok(());
}
PrepareResult::Ready(p) => {
println!(
"New manifest available for {}. Digest {}",
Copy link
Collaborator

Choose a reason for hiding this comment

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

One less than ideal thing here is that it leaves open the problem of how programmatic access would work - if I'm scripting bootc, I'd have to parse this text designed for human readability.

We already have a --touch-if-changed CLI option which is designed for the non-check case, so let's honor it with --check too.

imgref, p.manifest_digest
);
}
}
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we have a return above I don't think we need to indent everything here, right? (Though if you follow my above suggestion, the touch_if_changed handling would be outside the else)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the PR with a change in the return statement and moved the touch_if_changed handling outside of the else. Now it will carry on and honor the option.
However that keeps the indentation.

let fetched = pull(repo, &imgref, opts.quiet).await?;

if fetched.merge_commit.as_str() == commit.as_str() {
println!("Already queued: {digest}");
return Ok(());
}

stage(sysroot, &osname, &imgref, fetched, &origin).await?;
stage(sysroot, &osname, &imgref, fetched, &origin).await?;
}

if let Some(path) = opts.touch_if_changed {
std::fs::write(&path, "").with_context(|| format!("Writing {path}"))?;
Expand Down