Store container manifest

[Johan-Liebert1]

Unfortunately, this will break any older composefs booted systems

Until now, when doing a bootc status for a compoesfs booted system, we
were reaching out a container registry to fetch image manifest and
config, which is pretty suboptimal as the command took upwards of 1.5s
to execute, sometimes.

Instead, now we store the manifest + config as a JSON structure inside
an .imginfo file alongside the .origin file

Now that we store the current deployment's manifest locally, we can
replace the ugly stuff with a simple call to ManifestDiff::new

[gemini-code-assist]

Code Review

This pull request introduces a significant performance improvement for bootc status by storing the container manifest and config locally in an .imginfo file, avoiding slow network requests. The implementation also refactors the code to use ManifestDiff::new for a cleaner diffing logic.

The changes are well-structured and achieve the intended goal. However, I've found a couple of critical issues related to memory safety and type mismatches that need to be addressed. I've also included a suggestion to improve backward compatibility for older systems.

Overall, this is a great enhancement. Once the identified issues are resolved, this will be a solid contribution.

[gemini-code-assist]

The current implementation has a lifetime issue. In the None case, get_container_manifest_and_config returns an owned ImgConfigManifest, and you're taking a reference to this temporary value. This reference will dangle after the match statement, leading to undefined behavior. This is a critical bug that the compiler should have caught.

A good way to fix this is to use std::borrow::Cow to handle both the borrowed and owned cases gracefully. You may need to add use std::borrow::Cow; at the top of the file if it's not already there.

With this change, you'll also need to update the serde_json::to_vec call on line 247 to serde_json::to_vec(&*img_config)? to dereference the Cow before serialization.

Suggested change

	let img_config = match container_details {
	Some(val) => val,
	None => &get_container_manifest_and_config(&imgref).await?,
	};
	let img_config: std::borrow::Cow<'_, ImgConfigManifest> = match container_details {
	Some(val) => std::borrow::Cow::Borrowed(val),
	None => std::borrow::Cow::Owned(get_container_manifest_and_config(&imgref).await?),
	};

[Johan-Liebert1]

We're not really assigning it as let x = ..., thus not tying it to the None block's lifetime, so this should be fine, unless there's something else I'm missing?

[cgwalters]

The current implementation has a lifetime issue. In the None case, get_container_manifest_and_config returns an owned ImgConfigManifest, and you're taking a reference to this temporary value. This reference will dangle after the match statement, leading to undefined behavior. This is a critical bug that the compiler should have caught.

It really surprises me but it seems that both Gemini Pro and Sonnet 4.5 at least don't understand the Rust lifetime extension rules. This also just came up in
bootc-dev/bcvk#170 (comment)

Again if the compiler accepts it, we can trust it.

[gemini-code-assist]

This change removes the fallback mechanism that was present before. If get_imginfo fails (e.g., for an older deployment without an .imginfo file), this will cause bootc status and other commands to fail. While the PR description mentions breaking older systems, a status command should ideally be more resilient.

Consider adding a fallback to the previous behavior of fetching the manifest from the registry if the local .imginfo file is not available. This would provide backward compatibility and a more robust user experience.

[cgwalters]

Consider adding a fallback to the previous behavior of fetching the manifest from the registry if the local .imginfo file is not available.

Yeah I also lean towards keeping the previous code for now if it's not too hard

[Johan-Liebert1]

It's definitely not difficult, but we did not want to make a network request on every bootc status so I left it out. Guess we won't make the request anyway after this change. But, also, the previous code had a bug where it would show the "latest" shasum of the image on bootc status even when the image was not pulled into the composefs repo, because we were just fetching everything from a remote repository