Add support for creating ostree commits#343
Conversation
Replace the lossy `Vec<(String, String)>` metadata representation with `Vec<(String, MetadataValue)>` where MetadataValue is a typed enum covering the common GVariant types used in ostree commit metadata (s, b, u, t, as, ay) with a raw `Owned<Variant>` fallback for unrecognized types. This preserves type information needed for serialization (to compute correct checksums when generating commits), while the Display impl reproduces the same output as before for inspect/display purposes. Signed-off-by: Alexander Larsson <alexl@redhat.com>
Add serialize() methods to OstreeDirMeta, OstreeDirTree, and
OstreeCommit, using gv!().serialize_to_vec() — the same pattern
already used by OstreeFileHeader.
Add SerializeTo<Variant> for MetadataValue so OstreeCommit can
serialize its a{sv} metadata dict with correct variant type tags,
using VariantWrap for typed values and raw Variant passthrough for
the Other fallback.
Signed-off-by: Alexander Larsson <alexl@redhat.com>
alexlarsson
force-pushed
the
ostree-commit
branch
from
d5f31e4 to
9df369b
Compare
Johan-Liebert1
left a comment
Johan-Liebert1
left a comment
There was a problem hiding this comment.
Logic looks okay at a first glance. Some comments
| ) -> Result<Sha256Digest> { | ||
| let leaf = fs.leaf(leaf_id); | ||
|
|
||
| let (symlink_target, content_owned, external_obj) = match &leaf.content { |
There was a problem hiding this comment.
How about making symlink_target Option<String>?
There was a problem hiding this comment.
It will eventually end up in OstreeFileHeader.symlink_target, which is kind of a direct mapping of an ostree GVariant where the format for nothing is "", because gvariant doesn't have optional types. We could introduce one in the rust mapping, but I'm not sure its worth that.
| gid: leaf.stat.st_gid, | ||
| mode: (leaf.stat.st_mode & !S_IFMT) | file_type_bits, | ||
| symlink_target, | ||
| xattrs: stat_xattrs_to_vec(&leaf.stat), |
There was a problem hiding this comment.
I'm not sure how much it's applicable here, but worth taking a look at
There was a problem hiding this comment.
It is not strictly related, as it works correctly if your Filesystem has the correct xattrs (i.e. matching on-disk format for selinux labels), which they would if you created one from an actual filesystem. However, given that existing code like selabel() gets this wrong I added some canonicalization to avoid running into this again.
Add CommitWriter::from_filesystem() which walks a FileSystem tree bottom-up, producing ostree dirtree/dirmeta/file objects with correct checksums. Hardlinks are handled via a LeafId cache so shared leaves produce the same ostree checksum. The public commit_filesystem() function wraps this with commit metadata (subject, timestamp, ostree.ref-binding), serializes the splitstream, and generates the linked EROFS image. This enables creating ostree commits from any composefs FileSystem, regardless of its origin (OCI image, local directory, etc.). Signed-off-by: Alexander Larsson <alexl@redhat.com>
Validates that commiting an image created by composefs-ostree return the identical ostree commit id. Signed-off-by: Alexander Larsson <alexl@redhat.com>
Creates an ostree commit from an existing composefs image in the repository. The image is specified by its object ID or refs/ name. Example workflow to create an ostree commit from an OCI image: IMAGE_ID=$(cfsctl oci compute-id myimage:latest) cfsctl ostree commit $IMAGE_ID --reference fedora/40/x86_64 Signed-off-by: Alexander Larsson <alexl@redhat.com>
alexlarsson
force-pushed
the
ostree-commit
branch
from
9df369b to
5657488
Compare
| RegularFile::External(obj_id, _) | RegularFile::ExternalNoVerity(obj_id, _), | ||
| ) => Some(repo.read_object(obj_id)?), | ||
| LeafContent::Regular(RegularFile::Sparse(size)) => { | ||
| Some(vec![0u8; *size as usize]) |
There was a problem hiding this comment.
This shouldn't happen - honestly I'd rather just error out on sparse files in this case because again they're just a leftover bogosity from the composefs-c project I think.
But, if we're going ot handle them we should not potentially heap allocate gigabytes of zeroes...
| /// Use [`ostree::CommitMetadata::default()`] for a minimal commit, the | ||
| /// builder methods to set fields, or [`ostree::OstreeCommit::commit_metadata()`] | ||
| /// to round-trip an existing commit. | ||
| pub fn commit_filesystem<ObjectID: FsVerityHashValue>( |
There was a problem hiding this comment.
The challenge I see here is that now composefs holds the GC root for the ostree commit...and actually since we aren't writing to an ostree repo at all, I am not sure how we'd sanely do in-place upgrades.
IOW a goal of the bootc unified storage is that (for compatibility) every aspect of ostree continues to work unchanged (including e.g. rpm-ostree package layering), we just share storage. ostree handles GC on its side, etc.
There was a problem hiding this comment.
Are you worried about a gc in the ostree repo removing a commit, and but the ostree commit (as well as the oci i guess) is not GC:ed in the composefs repo?
3 participants
This lets us create a local ostree commit in the composefs repo based on an Filesystem (which could come from an image, which in turn could come from an oci image).
This lets us integrate composefs and ostree easily, as mentioned in #141