Re: Extending ostree's functionality (adding a metadat index)

[Colin Walters <walters verbum org>]

On Fri, 2013-08-30 at 15:13 +0100, Vivek Dasmohapatra wrote:

> http://cgit.collabora.com/git/user/vivek/ostree.git/log/?h=commit-index-ii

Ok, I should be able to have a chance to review this more closely Sunday
or early next week.  Some brief comments:

+ path = ostree_get_relative_file_path (csum, SIZES_EXTENSTION);
+
+ if (!path)
+ {
+ g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+ "Could not create file path for ." SIZES_EXTENSTION " file");
+ goto out;
+ }

ostree_get_relative_file_path can't throw an error; we (like all of
GLib) don't attempt to handle OOM.  You have a number of code paths like
this.

+ if (indexable)
+ {
+ gs_unref_object GFileInfo *info =
+ g_file_query_info (temp_file, G_FILE_ATTRIBUTE_STANDARD_SIZE, 0, NULL,
NULL);
+ gsize archived_size = g_file_info_get_size (info);

This bit however *can* throw (really the only possibility is I/O errors
since the file shouldn't have gone away but) should they occur we'd
segfault.  So this needs to be:

 gs_unref_object GFileInfo *info =
   g_file_query_info (temp_file, G_FILE_ATTRIBUTE_STANDARD_SIZE, 0,
                      cancellable, error))
   if (!info)
     goto out;

Can you audit further commits for stuff like this?

Also with the first change:
http://cgit.collabora.com/git/user/vivek/ostree.git/commit/?h=commit-index-ii&id=b7924c99243ade253b26317a77b4449d380a39c5
I think it'd be best if we compute the sizes just before writing out the
commit object, by traversing the tree again.  As opposed to gathering
the information in stage_object().   The problem is
that if say you do a commit like this:

$ mkdir overlay
$ touch overlay/usr/bin/foo
$ ostree commit -b --tree=ref=gnome-ostree/buildmaster/x86_64-runtime --tree=dir=overlay

We'll only end up writing .sizes that contains the object usr/bin/foo, because
the commit logic there won't call stage_object() for everything that's already in
gnome-ostree/buildmaster/x86_64-runtime.

This kind of "layering" commit is intended to be a relatively efficient
operation.  Let me give you an example of their use.  Say you're running
an OSTree-enabled system, and you're hacking on gnome-shell.  You can do
the equivalent of "make install DESTDIR=/tmp/gnome-shell-overlay" from
git, then:

commit=$(ostree commit -b local --tree=ref=0c5d9934a5b... --tree=dir=/tmp/gnome-shell-overlay)
ostree admin deploy $commit

Now what happened here is we did something like "sudo make install",
except it will only take effect on the *next* boot.  We can reboot
and see our changes, then reboot back.  And we didn't re-checksum/commit
all of /usr.