New upstream version 2018.7

src/libostree/libostree-released.sym

@@ -527,6 +527,13 @@ global:
   ostree_validate_collection_id;
 } LIBOSTREE_2018.5;
 
+LIBOSTREE_2018.7 {
+global:
+    ostree_mutable_tree_fill_empty_from_dirtree;
+    ostree_mutable_tree_new_from_checksum;
+    ostree_mutable_tree_check_error;
+} LIBOSTREE_2018.6;
+
 /* NOTE: Only add more content here in release commits!  See the
  * comments at the top of this file.
  */

src/libostree/ostree-mutable-tree.c

@@ -26,6 +26,8 @@
 #include "otutil.h"
 #include "ostree.h"
 
+#include "ostree-core-private.h"
+
 /**
  * SECTION:ostree-mutable-tree
  * @title: In-memory modifiable filesystem tree
@@ -38,6 +40,15 @@
  * programmatically.
  */
 
+typedef enum {
+    MTREE_STATE_WHOLE,
+
+    /* MTREE_STATE_LAZY allows us to not read files and subdirs from the objects
+     * on disk until they're actually needed - often they won't be needed at
+     * all. */
+    MTREE_STATE_LAZY
+} OstreeMutableTreeState;
+
 /**
  * OstreeMutableTree:
  *
@@ -47,10 +58,45 @@ struct OstreeMutableTree
 {
   GObject parent_instance;
 
+  /* The parent directory to this one.  We don't hold a ref because this mtree
+   * is owned by the parent.  We can be certain that any mtree only has one
+   * parent because external users can't set this, it's only set when we create
+   * a child from within this file (see insert_child_mtree). We ensure that the
+   * parent pointer is either valid or NULL because when the parent is destroyed
+   * it sets parent = NULL on all its children (see remove_child_mtree) */
+  OstreeMutableTree *parent;
+
+  OstreeMutableTreeState state;
+
+  /* This is the checksum of the Dirtree object that corresponds to the current
+   * contents of this directory.  contents_checksum can be NULL if the SHA was
+   * never calculated or contents of this mtree or any subdirectory has been
+   * modified.  If a contents_checksum is NULL then all the parent's checksums
+   * will be NULL (see `invalidate_contents_checksum`).
+   *
+   * Note: This invariant is partially maintained externally - we
+   * rely on the callers of `ostree_mutable_tree_set_contents_checksum` to have
+   * first ensured that the mtree contents really does correspond to this
+   * checksum */
   char *contents_checksum;
+
+  /* This is the checksum of the DirMeta object that holds the uid, gid, mode
+   * and xattrs of this directory.  This can be NULL. */
   char *metadata_checksum;
 
+  /* ======== Valid for state LAZY: =========== */
+
+  /* The repo so we can look up the checksums. */
+  OstreeRepo *repo;
+
+  GError *cached_error;
+
+  /* ======== Valid for state WHOLE: ========== */
+
+  /* const char* filename -> const char* checksum. */
   GHashTable *files;
+
+  /* const char* filename -> OstreeMutableTree* subtree */
   GHashTable *subdirs;
 };
 
@@ -66,9 +112,12 @@ ostree_mutable_tree_finalize (GObject *object)
   g_free (self->contents_checksum);
   g_free (self->metadata_checksum);
 
+  g_clear_pointer (&self->cached_error, g_error_free);
   g_hash_table_destroy (self->files);
   g_hash_table_destroy (self->subdirs);
 
+  g_clear_object (&self->repo);
+
   G_OBJECT_CLASS (ostree_mutable_tree_parent_class)->finalize (object);
 }
 
@@ -80,19 +129,133 @@ ostree_mutable_tree_class_init (OstreeMutableTreeClass *klass)
   gobject_class->finalize = ostree_mutable_tree_finalize;
 }
 
+/* This must not be made public or we can't maintain the invariant that any
+ * OstreeMutableTree has only one parent.
+ *
+ * Ownership of @child is transferred from the caller to @self */
+static void
+insert_child_mtree (OstreeMutableTree *self, const gchar* name,
+                    OstreeMutableTree *child)
+{
+  g_assert_null (child->parent);
+  g_hash_table_insert (self->subdirs, g_strdup (name), child);
+  child->parent = self;
+}
+
+static void
+remove_child_mtree (gpointer data)
+{
+  /* Each mtree has shared ownership of its children and each child has a
+   * non-owning reference back to parent.  If the parent goes out of scope the
+   * children may still be alive because they're reference counted. This
+   * removes the reference to the parent before it goes stale. */
+  OstreeMutableTree *child = (OstreeMutableTree*) data;
+  child->parent = NULL;
+  g_object_unref (child);
+}
+
 static void
 ostree_mutable_tree_init (OstreeMutableTree *self)
 {
   self->files = g_hash_table_new_full (g_str_hash, g_str_equal,
                                        g_free, g_free);
   self->subdirs = g_hash_table_new_full (g_str_hash, g_str_equal,
-                                         g_free, (GDestroyNotify)g_object_unref);
+                                         g_free, remove_child_mtree);
+  self->state = MTREE_STATE_WHOLE;
+}
+
+static void
+invalidate_contents_checksum (OstreeMutableTree *self)
+{
+  while (self) {
+    if (!self->contents_checksum)
+      break;
+
+    g_clear_pointer (&self->contents_checksum, g_free);
+    self = self->parent;
+  }
+}
+
+/* Go from state LAZY to state WHOLE by reading the tree from disk */
+static gboolean
+_ostree_mutable_tree_make_whole (OstreeMutableTree           *self,
+                                 GCancellable                *cancellable,
+                                 GError                     **error)
+{
+  if (self->state == MTREE_STATE_WHOLE)
+    return TRUE;
+
+  g_assert_cmpuint (self->state, ==, MTREE_STATE_LAZY);
+  g_assert_nonnull (self->repo);
+  g_assert_nonnull (self->contents_checksum);
+  g_assert_nonnull (self->metadata_checksum);
+  g_assert_cmpuint (g_hash_table_size (self->files), ==, 0);
+  g_assert_cmpuint (g_hash_table_size (self->subdirs), ==, 0);
+
+  g_autoptr(GVariant) dirtree = NULL;
+  if (!ostree_repo_load_variant (self->repo, OSTREE_OBJECT_TYPE_DIR_TREE,
+                                 self->contents_checksum, &dirtree, error))
+    return FALSE;
+
+  {
+    g_autoptr(GVariant) dir_file_contents = g_variant_get_child_value (dirtree, 0);
+    GVariantIter viter;
+    g_variant_iter_init (&viter, dir_file_contents);
+    const char *fname;
+    GVariant *contents_csum_v = NULL;
+    while (g_variant_iter_loop (&viter, "(&s@ay)", &fname, &contents_csum_v))
+      {
+        char tmp_checksum[OSTREE_SHA256_STRING_LEN + 1];
+        _ostree_checksum_inplace_from_bytes_v (contents_csum_v, tmp_checksum);
+        g_hash_table_insert (self->files, g_strdup (fname),
+            g_strdup (tmp_checksum));
+      }
+  }
+
+  /* Process subdirectories */
+  {
+    g_autoptr(GVariant) dir_subdirs = g_variant_get_child_value (dirtree, 1);
+    const char *dname;
+    GVariant *subdirtree_csum_v = NULL;
+    GVariant *subdirmeta_csum_v = NULL;
+    GVariantIter viter;
+    g_variant_iter_init (&viter, dir_subdirs);
+    while (g_variant_iter_loop (&viter, "(&s@ay@ay)", &dname,
+                                &subdirtree_csum_v, &subdirmeta_csum_v))
+      {
+        char subdirtree_checksum[OSTREE_SHA256_STRING_LEN+1];
+        _ostree_checksum_inplace_from_bytes_v (subdirtree_csum_v, subdirtree_checksum);
+        char subdirmeta_checksum[OSTREE_SHA256_STRING_LEN+1];
+        _ostree_checksum_inplace_from_bytes_v (subdirmeta_csum_v, subdirmeta_checksum);
+        insert_child_mtree (self, dname, ostree_mutable_tree_new_from_checksum (
+            self->repo, subdirtree_checksum, subdirmeta_checksum));
+      }
+  }
+
+  g_clear_object (&self->repo);
+  self->state = MTREE_STATE_WHOLE;
+  return TRUE;
+}
+
+/* _ostree_mutable_tree_make_whole can fail if state == MTREE_STATE_LAZY, but
+ * we have getters that preceed the existence of MTREE_STATE_LAZY which can't
+ * return errors.  So instead this function will fail and print a warning. */
+static gboolean
+_assert_ostree_mutable_tree_make_whole (OstreeMutableTree *self)
+{
+  if (self->cached_error)
+    return FALSE;
+  return _ostree_mutable_tree_make_whole (self, NULL, &self->cached_error);
 }
 
 void
 ostree_mutable_tree_set_metadata_checksum (OstreeMutableTree *self,
                                            const char        *checksum)
 {
+  if (g_strcmp0 (checksum, self->metadata_checksum) == 0)
+    return;
+
+  invalidate_contents_checksum (self->parent);
   g_free (self->metadata_checksum);
   self->metadata_checksum = g_strdup (checksum);
 }
@@ -107,6 +270,16 @@ void
 ostree_mutable_tree_set_contents_checksum (OstreeMutableTree *self,
                                            const char        *checksum)
 {
+  if (g_strcmp0 (checksum, self->contents_checksum) == 0)
+    return;
+
+  if (checksum && self->contents_checksum)
+    g_warning ("Setting a contents checksum on an OstreeMutableTree that "
+        "already has a checksum set.  Old checksum %s, new checksum %s",
+        self->contents_checksum, checksum);
+
+  _assert_ostree_mutable_tree_make_whole (self);
+
   g_free (self->contents_checksum);
   self->contents_checksum = g_strdup (checksum);
 }
@@ -114,26 +287,6 @@ ostree_mutable_tree_set_contents_checksum (OstreeMutableTree *self,
 const char *
 ostree_mutable_tree_get_contents_checksum (OstreeMutableTree *self)
 {
-  if (!self->contents_checksum)
-    return NULL;
-
-  /* Ensure the cache is valid; this implementation is a bit
-   * lame in that we walk the whole tree every time this
-   * getter is called; a better approach would be to invalidate
-   * all of the parents whenever a child is modified.
-   *
-   * However, we only call this function once right now.
-   */
-  GLNX_HASH_TABLE_FOREACH_V (self->subdirs, OstreeMutableTree*, subdir)
-    {
-      if (!ostree_mutable_tree_get_contents_checksum (subdir))
-        {
-          g_free (self->contents_checksum);
-          self->contents_checksum = NULL;
-          return NULL;
-        }
-    }
-
   return self->contents_checksum;
 }
 
@@ -159,6 +312,9 @@ ostree_mutable_tree_replace_file (OstreeMutableTree *self,
   if (!ot_util_filename_validate (name, error))
     goto out;
 
+  if (!_ostree_mutable_tree_make_whole (self, NULL, error))
+    goto out;
+
   if (g_hash_table_lookup (self->subdirs, name))
     {
       g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
@@ -166,7 +322,7 @@ ostree_mutable_tree_replace_file (OstreeMutableTree *self,
       goto out;
     }
 
-  ostree_mutable_tree_set_contents_checksum (self, NULL);
+  invalidate_contents_checksum (self);
   g_hash_table_replace (self->files,
                         g_strdup (name),
                         g_strdup (checksum));
@@ -176,6 +332,16 @@ ostree_mutable_tree_replace_file (OstreeMutableTree *self,
   return ret;
 }
 
+/**
+ * ostree_mutable_tree_ensure_dir:
+ * @self: Tree
+ * @name: Name of subdirectory of self to retrieve/creates
+ * @out_subdir: (out) (transfer full): the subdirectory
+ * @error: a #GError
+ *
+ * Returns the subdirectory of self with filename @name, creating an empty one
+ * it if it doesn't exist.
+ */
 gboolean
 ostree_mutable_tree_ensure_dir (OstreeMutableTree *self,
                                 const char        *name,
@@ -190,6 +356,9 @@ ostree_mutable_tree_ensure_dir (OstreeMutableTree *self,
   if (!ot_util_filename_validate (name, error))
     goto out;
 
+  if (!_ostree_mutable_tree_make_whole (self, NULL, error))
+    goto out;
+
   if (g_hash_table_lookup (self->files, name))
     {
       g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
@@ -201,8 +370,8 @@ ostree_mutable_tree_ensure_dir (OstreeMutableTree *self,
   if (!ret_dir)
     {
       ret_dir = ostree_mutable_tree_new ();
-      ostree_mutable_tree_set_contents_checksum (self, NULL);
-      g_hash_table_insert (self->subdirs, g_strdup (name), g_object_ref (ret_dir));
+      invalidate_contents_checksum (self);
+      insert_child_mtree (self, name, g_object_ref (ret_dir));
     }
   
   ret = TRUE;
@@ -222,6 +391,9 @@ ostree_mutable_tree_lookup (OstreeMutableTree   *self,
   g_autoptr(OstreeMutableTree) ret_subdir = NULL;
   g_autofree char *ret_file_checksum = NULL;
   
+  if (!_ostree_mutable_tree_make_whole (self, NULL, error))
+    goto out;
+
   ret_subdir = ot_gobject_refz (g_hash_table_lookup (self->subdirs, name));
   if (!ret_subdir)
     {
@@ -263,6 +435,9 @@ ostree_mutable_tree_ensure_parent_dirs (OstreeMutableTree  *self,
   OstreeMutableTree *subdir = self; /* nofree */
   g_autoptr(OstreeMutableTree) ret_parent = NULL;
 
+  if (!_ostree_mutable_tree_make_whole (self, NULL, error))
+    goto out;
+
   g_assert (metadata_checksum != NULL);
 
   if (!self->metadata_checksum)
@@ -283,9 +458,10 @@ ostree_mutable_tree_ensure_parent_dirs (OstreeMutableTree  *self,
       next = g_hash_table_lookup (subdir->subdirs, name);
       if (!next) 
         {
+          invalidate_contents_checksum (subdir);
           next = ostree_mutable_tree_new ();
           ostree_mutable_tree_set_metadata_checksum (next, metadata_checksum);
-          g_hash_table_insert (subdir->subdirs, g_strdup (name), next);
+          insert_child_mtree (subdir, g_strdup (name), next);
         }
       
       subdir = next;
@@ -299,6 +475,71 @@ ostree_mutable_tree_ensure_parent_dirs (OstreeMutableTree  *self,
   return ret;
 }
 
+const char empty_tree_csum[] = "6e340b9cffb37a989ca544e6bb780a2c78901d3fb33738768511a30617afa01d";
+
+/**
+ * ostree_mutable_tree_fill_empty_from_dirtree:
+ *
+ * Merges @self with the tree given by @contents_checksum and
+ * @metadata_checksum, but only if it's possible without writing new objects to
+ * the @repo.  We can do this if either @self is empty, the tree given by
+ * @contents_checksum is empty or if both trees already have the same
+ * @contents_checksum.
+ *
+ * Returns: @TRUE if merge was successful, @FALSE if it was not possible.
+ *
+ * This function enables optimisations when composing trees.  The provided
+ * checksums are not loaded or checked when this function is called.  Instead
+ * the contents will be loaded only when needed.
+ */
+gboolean
+ostree_mutable_tree_fill_empty_from_dirtree (OstreeMutableTree *self,
+                                             OstreeRepo        *repo,
+                                             const char        *contents_checksum,
+                                             const char        *metadata_checksum)
+{
+  g_return_val_if_fail (repo, FALSE);
+  g_return_val_if_fail (contents_checksum, FALSE);
+  g_return_val_if_fail (metadata_checksum, FALSE);
+
+  switch (self->state)
+    {
+    case MTREE_STATE_LAZY:
+      {
+        if (g_strcmp0 (contents_checksum, self->contents_checksum) == 0 ||
+            g_strcmp0 (empty_tree_csum, self->contents_checksum) == 0)
+          break;
+
+        if (g_strcmp0 (empty_tree_csum, contents_checksum) == 0)
+          {
+            /* Adding an empty tree to a full one - stick with the old contents */
+            contents_checksum = self->contents_checksum;
+            break;
+          }
+        else
+          return FALSE;
+      }
+    case MTREE_STATE_WHOLE:
+      if (g_hash_table_size (self->files) == 0 &&
+          g_hash_table_size (self->subdirs) == 0)
+        break;
+      /* We're not empty - can't convert to a LAZY tree */
+      return FALSE;
+    default:
+      g_assert_not_reached ();
+    }
+
+  self->state = MTREE_STATE_LAZY;
+  g_set_object (&self->repo, repo);
+  ostree_mutable_tree_set_metadata_checksum (self, metadata_checksum);
+  if (g_strcmp0 (self->contents_checksum, contents_checksum) != 0)
+    {
+      invalidate_contents_checksum (self);
+      self->contents_checksum = g_strdup (contents_checksum);
+    }
+  return TRUE;
+}
+
 /**
  * ostree_mutable_tree_walk:
  * @self: Tree
@@ -327,6 +568,8 @@ ostree_mutable_tree_walk (OstreeMutableTree     *self,
   else
     {
       OstreeMutableTree *subdir;
+      if (!_ostree_mutable_tree_make_whole (self, NULL, error))
+        return FALSE;
 
       subdir = g_hash_table_lookup (self->subdirs, split_path->pdata[start]);
       if (!subdir)
@@ -345,6 +588,7 @@ ostree_mutable_tree_walk (OstreeMutableTree     *self,
 GHashTable *
 ostree_mutable_tree_get_subdirs (OstreeMutableTree *self)
 {
+  _assert_ostree_mutable_tree_make_whole (self);
   return self->subdirs;
 }
 
@@ -357,9 +601,35 @@ ostree_mutable_tree_get_subdirs (OstreeMutableTree *self)
 GHashTable *
 ostree_mutable_tree_get_files (OstreeMutableTree *self)
 {
+  _assert_ostree_mutable_tree_make_whole (self);
   return self->files;
 }
 
+/**
+ * ostree_mutable_tree_check_error:
+ * @self: Tree
+ *
+ * In some cases, a tree may be in a "lazy" state that loads
+ * data in the background; if an error occurred during a non-throwing
+ * API call, it will have been cached.  This function checks for a
+ * cached error.  The tree remains in error state.
+ *
+ * Since: 2018.7
+ * Returns: `TRUE` on success
+ */
+gboolean
+ostree_mutable_tree_check_error (OstreeMutableTree     *self,
+                                 GError               **error)
+{
+  if (self->cached_error)
+    {
+      if (error)
+        *error = g_error_copy (self->cached_error);
+      return FALSE;
+    }
+  return TRUE;
+}
+
 /**
  * ostree_mutable_tree_new:
  *
@@ -370,3 +640,27 @@ ostree_mutable_tree_new (void)
 {
   return (OstreeMutableTree*)g_object_new (OSTREE_TYPE_MUTABLE_TREE, NULL);
 }
+
+/**
+ * ostree_mutable_tree_new_from_checksum:
+ * @repo: The repo which contains the objects refered by the checksums.
+ * @contents_checksum: dirtree checksum
+ * @metadata_checksum: dirmeta checksum
+ *
+ * Creates a new OstreeMutableTree with the contents taken from the given repo
+ * and checksums.  The data will be loaded from the repo lazily as needed.
+ *
+ * Returns: (transfer full): A new tree
+ */
+OstreeMutableTree *
+ostree_mutable_tree_new_from_checksum (OstreeRepo *repo,
+                                       const char *contents_checksum,
+                                       const char *metadata_checksum)
+{
+  OstreeMutableTree* out = (OstreeMutableTree*)g_object_new (OSTREE_TYPE_MUTABLE_TREE, NULL);
+  out->state = MTREE_STATE_LAZY;
+  out->repo = g_object_ref (repo);
+  out->contents_checksum = g_strdup (contents_checksum);
+  out->metadata_checksum = g_strdup (metadata_checksum);
+  return out;
+}

src/libostree/ostree-mutable-tree.h

@@ -52,6 +52,11 @@ GType   ostree_mutable_tree_get_type (void) G_GNUC_CONST;
 _OSTREE_PUBLIC
 OstreeMutableTree *ostree_mutable_tree_new (void);
 
+_OSTREE_PUBLIC
+OstreeMutableTree * ostree_mutable_tree_new_from_checksum (OstreeRepo *repo,
+                                                           const char *contents_checksum,
+                                                           const char *metadata_checksum);
+
 _OSTREE_PUBLIC
 void ostree_mutable_tree_set_metadata_checksum (OstreeMutableTree *self,
                                                 const char        *checksum);
@@ -100,6 +105,17 @@ gboolean ostree_mutable_tree_walk (OstreeMutableTree   *self,
                                    OstreeMutableTree  **out_subdir,
                                    GError             **error);
 
+_OSTREE_PUBLIC
+gboolean ostree_mutable_tree_fill_empty_from_dirtree (OstreeMutableTree *self,
+                                                      OstreeRepo        *repo,
+                                                      const char        *contents_checksum,
+                                                      const char        *metadata_checksum);
+
+_OSTREE_PUBLIC
+gboolean
+ostree_mutable_tree_check_error (OstreeMutableTree     *self,
+                                 GError               **error);
+
 _OSTREE_PUBLIC
 GHashTable * ostree_mutable_tree_get_subdirs (OstreeMutableTree *self);
 _OSTREE_PUBLIC

src/libostree/ostree-repo-commit.c

@@ -429,6 +429,28 @@ add_size_index_to_metadata (OstreeRepo        *self,
   return g_variant_ref_sink (g_variant_builder_end (builder));
 }
 
+static gboolean
+throw_min_free_space_error (OstreeRepo  *self,
+                            guint64      bytes_required,
+                            GError     **error)
+{
+  const char *err_msg = NULL;
+  g_autofree char *err_msg_owned = NULL;
+
+  if (bytes_required > 0)
+    {
+      g_autofree char *formatted_required = g_format_size (bytes_required);
+      err_msg = err_msg_owned = g_strdup_printf ("would be exceeded, at least %s requested", formatted_required);
+    }
+  else
+    err_msg  = "would be exceeded";
+
+  if (self->min_free_space_percent > 0)
+    return glnx_throw (error, "min-free-space-percent '%u%%' %s", self->min_free_space_percent, err_msg);
+  else
+    return glnx_throw (error, "min-free-space-size %" G_GUINT64_FORMAT "MB %s", self->min_free_space_mb, err_msg);
+}
+
 typedef struct {
   gboolean initialized;
   GLnxTmpfile tmpf;
@@ -509,6 +531,28 @@ _ostree_repo_bare_content_commit (OstreeRepo                 *self,
 {
   OstreeRealRepoBareContent *real = (OstreeRealRepoBareContent*) barewrite;
   g_assert (real->initialized);
+
+  if ((self->min_free_space_percent > 0 || self->min_free_space_mb > 0) && self->in_transaction)
+    {
+      struct stat st_buf;
+      if (!glnx_fstat (real->tmpf.fd, &st_buf, error))
+        return FALSE;
+
+      g_mutex_lock (&self->txn_lock);
+      g_assert_cmpint (self->txn.blocksize, >, 0);
+
+      const fsblkcnt_t object_blocks = (st_buf.st_size / self->txn.blocksize) + 1;
+      if (object_blocks > self->txn.max_blocks)
+        {
+          self->cleanup_stagedir = TRUE;
+          g_mutex_unlock (&self->txn_lock);
+          return throw_min_free_space_error (self, st_buf.st_size, error);
+        }
+      /* This is the main bit that needs mutex protection */
+      self->txn.max_blocks -= object_blocks;
+      g_mutex_unlock (&self->txn_lock);
+    }
+
   ot_checksum_get_hexdigest (&real->checksum, checksum_buf, buflen);
 
   if (real->expected_checksum &&
@@ -897,14 +941,9 @@ write_content_object (OstreeRepo         *self,
       if (object_blocks > self->txn.max_blocks)
         {
           guint64 bytes_required = (guint64)object_blocks * self->txn.blocksize;
+          self->cleanup_stagedir = TRUE;
           g_mutex_unlock (&self->txn_lock);
-          g_autofree char *formatted_required = g_format_size (bytes_required);
-          if (self->min_free_space_percent > 0)
-            return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s more required",
-                               self->min_free_space_percent, formatted_required);
-          else
-            return glnx_throw (error, "min-free-space-size %luMB would be exceeded, %s more required",
-                               self->min_free_space_mb, formatted_required);
+          return throw_min_free_space_error (self, bytes_required, error);
         }
       /* This is the main bit that needs mutex protection */
       self->txn.max_blocks -= object_blocks;
@@ -1585,9 +1624,15 @@ ostree_repo_prepare_transaction (OstreeRepo     *self,
                                  GCancellable   *cancellable,
                                  GError        **error)
 {
+  g_autoptr(_OstreeRepoAutoTransaction) txn = NULL;
 
   g_return_val_if_fail (self->in_transaction == FALSE, FALSE);
 
+  g_debug ("Preparing transaction in repository %p", self);
+
+  /* Set up to abort the transaction if we return early from this function. */
+  txn = self;
+
   memset (&self->txn.stats, 0, sizeof (OstreeRepoTransactionStats));
 
   self->txn_locked = _ostree_repo_lock_push (self, OSTREE_REPO_LOCK_SHARED,
@@ -1596,32 +1641,26 @@ ostree_repo_prepare_transaction (OstreeRepo     *self,
     return FALSE;
 
   self->in_transaction = TRUE;
-  if (self->min_free_space_percent >= 0 || self->min_free_space_mb >= 0)
+  self->cleanup_stagedir = FALSE;
+
+  struct statvfs stvfsbuf;
+  if (TEMP_FAILURE_RETRY (fstatvfs (self->repo_dir_fd, &stvfsbuf)) < 0)
+    return glnx_throw_errno_prefix (error, "fstatvfs");
+
+  g_mutex_lock (&self->txn_lock);
+  self->txn.blocksize = stvfsbuf.f_bsize;
+  guint64 reserved_blocks = min_free_space_calculate_reserved_blocks (self, &stvfsbuf);
+  /* Use the appropriate free block count if we're unprivileged */
+  guint64 bfree = (getuid () != 0 ? stvfsbuf.f_bavail : stvfsbuf.f_bfree);
+  if (bfree > reserved_blocks)
+    self->txn.max_blocks = bfree - reserved_blocks;
+  else
     {
-      struct statvfs stvfsbuf;
-      if (TEMP_FAILURE_RETRY (fstatvfs (self->repo_dir_fd, &stvfsbuf)) < 0)
-        return glnx_throw_errno_prefix (error, "fstatvfs");
-      g_mutex_lock (&self->txn_lock);
-      self->txn.blocksize = stvfsbuf.f_bsize;
-      guint64 reserved_blocks = min_free_space_calculate_reserved_blocks (self, &stvfsbuf);
-      /* Use the appropriate free block count if we're unprivileged */
-      guint64 bfree = (getuid () != 0 ? stvfsbuf.f_bavail : stvfsbuf.f_bfree);
-      if (bfree > reserved_blocks)
-        self->txn.max_blocks = bfree - reserved_blocks;
-      else
-        {
-          guint64 bytes_required = bfree * self->txn.blocksize;
-          g_mutex_unlock (&self->txn_lock);
-          g_autofree char *formatted_free = g_format_size (bytes_required);
-          if (self->min_free_space_percent > 0)
-            return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s available",
-                               self->min_free_space_percent, formatted_free);
-          else
-            return glnx_throw (error, "min-free-space-size %luMB would be exceeded, %s available",
-                               self->min_free_space_mb, formatted_free);
-        }
+      self->cleanup_stagedir = TRUE;
       g_mutex_unlock (&self->txn_lock);
+      return throw_min_free_space_error (self, 0, error);
     }
+  g_mutex_unlock (&self->txn_lock);
 
   gboolean ret_transaction_resume = FALSE;
   if (!_ostree_repo_allocate_tmpdir (self->tmp_dir_fd,
@@ -1632,6 +1671,9 @@ ostree_repo_prepare_transaction (OstreeRepo     *self,
                                      cancellable, error))
     return FALSE;
 
+  /* Success: do not abort the transaction when returning. */
+  txn = NULL;
+
   if (out_transaction_resume)
     *out_transaction_resume = ret_transaction_resume;
   return TRUE;
@@ -1756,9 +1798,10 @@ cleanup_txn_dir (OstreeRepo   *self,
 
   /* If however this is the staging directory for the *current*
    * boot, then don't delete it now - we may end up reusing it, as
-   * is the point.
+   * is the point. Delete *only if* we have hit min-free-space* checks
+   * as we don't want to hold onto caches in that case.
    */
-  if (g_str_has_prefix (path, self->stagedir_prefix))
+  if (g_str_has_prefix (path, self->stagedir_prefix) && !self->cleanup_stagedir)
     return TRUE; /* Note early return */
 
   /* But, crucially we can now clean up staging directories
@@ -2118,6 +2161,8 @@ ostree_repo_commit_transaction (OstreeRepo                  *self,
 {
   g_return_val_if_fail (self->in_transaction == TRUE, FALSE);
 
+  g_debug ("Committing transaction in repository %p", self);
+
   if ((self->test_error_flags & OSTREE_REPO_TEST_ERROR_PRE_COMMIT) > 0)
     return glnx_throw (error, "OSTREE_REPO_TEST_ERROR_PRE_COMMIT specified");
 
@@ -2202,9 +2247,7 @@ ostree_repo_abort_transaction (OstreeRepo     *self,
   if (!self->in_transaction)
     return TRUE;
 
-  /* Do not propagate failures from cleanup_tmpdir() immediately, as we want
-   * to clean up the rest of the internal transaction state first. */
-  cleanup_tmpdir (self, cancellable, &cleanup_error);
+  g_debug ("Aborting transaction in repository %p", self);
 
   if (self->loose_object_devino_hash)
     g_hash_table_remove_all (self->loose_object_devino_hash);
@@ -2215,6 +2258,10 @@ ostree_repo_abort_transaction (OstreeRepo     *self,
   glnx_tmpdir_unset (&self->commit_stagedir);
   glnx_release_lock_file (&self->commit_stagedir_lock);
 
+  /* Do not propagate failures from cleanup_tmpdir() immediately, as we want
+   * to clean up the rest of the internal transaction state first. */
+  cleanup_tmpdir (self, cancellable, &cleanup_error);
+
   self->in_transaction = FALSE;
 
   if (self->txn_locked)
@@ -3555,6 +3602,14 @@ write_directory_to_mtree_internal (OstreeRepo                  *self,
       if (!ostree_repo_file_ensure_resolved (repo_dir, error))
         return FALSE;
 
+      /* ostree_mutable_tree_fill_from_dirtree returns FALSE if mtree isn't
+       * empty: in which case we're responsible for merging the trees. */
+      if (ostree_mutable_tree_fill_empty_from_dirtree (mtree,
+            ostree_repo_file_get_repo (repo_dir),
+            ostree_repo_file_tree_get_contents_checksum (repo_dir),
+            ostree_repo_file_get_checksum (repo_dir)))
+        return TRUE;
+
       ostree_mutable_tree_set_metadata_checksum (mtree, ostree_repo_file_tree_get_metadata_checksum (repo_dir));
 
       filter_result = OSTREE_REPO_COMMIT_FILTER_ALLOW;
@@ -3868,6 +3923,9 @@ ostree_repo_write_mtree (OstreeRepo           *self,
   const char *contents_checksum, *metadata_checksum;
   g_autoptr(GFile) ret_file = NULL;
 
+  if (!ostree_mutable_tree_check_error (mtree, error))
+    return glnx_prefix_error (error, "mtree");
+
   metadata_checksum = ostree_mutable_tree_get_metadata_checksum (mtree);
   if (!metadata_checksum)
     return glnx_throw (error, "Can't commit an empty tree");

src/libostree/ostree-repo-finder-avahi.c

@@ -1432,8 +1432,7 @@ ostree_repo_finder_avahi_start (OstreeRepoFinderAvahi  *self,
 
   g_assert (self->client == NULL);
 
-  client = avahi_client_new (avahi_glib_poll_get (self->poll),
-                             AVAHI_CLIENT_NO_FAIL,
+  client = avahi_client_new (avahi_glib_poll_get (self->poll), 0,
                              client_cb, self, &failure);
 
   if (client == NULL)

src/libostree/ostree-repo-finder-config.c

@@ -96,7 +96,7 @@ ostree_repo_finder_config_resolve_async (OstreeRepoFinder                  *find
   GHashTableIter iter;
   const gchar *remote_name;
   g_auto(GStrv) remotes = NULL;
-  gsize n_remotes = 0;
+  guint n_remotes = 0;
 
   task = g_task_new (finder, cancellable, callback, user_data);
   g_task_set_source_tag (task, ostree_repo_finder_config_resolve_async);
@@ -106,9 +106,9 @@ ostree_repo_finder_config_resolve_async (OstreeRepoFinder                  *find
 
   /* List all remotes in this #OstreeRepo and see which of their ref lists
    * intersect with @refs. */
-  remotes = ostree_repo_remote_list (parent_repo, (guint *) &n_remotes);
+  remotes = ostree_repo_remote_list (parent_repo, &n_remotes);
 
-  g_debug ("%s: Checking %" G_GSIZE_FORMAT " remotes", G_STRFUNC, n_remotes);
+  g_debug ("%s: Checking %u remotes", G_STRFUNC, n_remotes);
 
   for (i = 0; i < n_remotes; i++)
     {

src/libostree/ostree-repo-private.h

@@ -152,6 +152,7 @@ struct OstreeRepo {
   gid_t target_owner_gid;
   guint min_free_space_percent; /* See the min-free-space-percent config option */
   guint64 min_free_space_mb; /* See the min-free-space-size config option */
+  gboolean cleanup_stagedir;
 
   guint test_error_flags; /* OstreeRepoTestErrorFlags */
 

src/libostree/ostree-repo.c

@@ -2670,7 +2670,7 @@ min_free_space_size_validate_and_convert (OstreeRepo    *self,
 
   g_autoptr(GMatchInfo) match = NULL;
   if (!g_regex_match (regex, min_free_space_size_str, 0, &match))
-    return glnx_prefix_error (error, "Failed to parse min-free-space-size parameter: '%s'", min_free_space_size_str);
+    return glnx_throw (error, "It should be of the format '123MB', '123GB' or '123TB'");
 
   g_autofree char *size_str = g_match_info_fetch (match, 1);
   g_autofree char *unit = g_match_info_fetch (match, 2);
@@ -2691,7 +2691,11 @@ min_free_space_size_validate_and_convert (OstreeRepo    *self,
         g_assert_not_reached ();
     }
 
-  self->min_free_space_mb = g_ascii_strtoull (size_str, NULL, 10) << shifts;
+  guint64 min_free_space = g_ascii_strtoull (size_str, NULL, 10);
+  if (shifts > 0 && g_bit_nth_lsf (min_free_space, 63 - shifts) != -1)
+    return glnx_throw (error, "Value was too high");
+
+  self->min_free_space_mb = min_free_space << shifts;
 
   return TRUE;
 }
@@ -2814,12 +2818,10 @@ reload_core_config (OstreeRepo          *self,
   }
 
   {
-    if (g_key_file_has_key (self->config, "core", "min-free-space-size", NULL) &&
-        g_key_file_has_key (self->config, "core", "min-free-space-percent", NULL))
-      {
-        return glnx_throw (error, "min-free-space-percent and min-free-space-size are mutually exclusive");
-      }
-    else if (g_key_file_has_key (self->config, "core", "min-free-space-size", NULL))
+    /* Try to parse both min-free-space-* config options first. If both are absent, fallback on 3% free space.
+     * If both are present and are non-zero, use min-free-space-size unconditionally and display a warning.
+     */
+    if (g_key_file_has_key (self->config, "core", "min-free-space-size", NULL))
       {
         g_autofree char *min_free_space_size_str = NULL;
 
@@ -2829,23 +2831,33 @@ reload_core_config (OstreeRepo          *self,
 
         /* Validate the string and convert the size to MBs */
         if (!min_free_space_size_validate_and_convert (self, min_free_space_size_str, error))
-          return glnx_throw (error, "Invalid min-free-space-size '%s'", min_free_space_size_str);
+          return glnx_prefix_error (error, "Invalid min-free-space-size '%s'", min_free_space_size_str);
       }
-    else
+
+    if (g_key_file_has_key (self->config, "core", "min-free-space-percent", NULL))
       {
         g_autofree char *min_free_space_percent_str = NULL;
-        /* If changing this, be sure to change the man page too */
-        const char *default_min_free_space = "3";
 
         if (!ot_keyfile_get_value_with_default (self->config, "core", "min-free-space-percent",
-                                                default_min_free_space,
-                                                &min_free_space_percent_str, error))
+                                                NULL, &min_free_space_percent_str, error))
           return FALSE;
 
         self->min_free_space_percent = g_ascii_strtoull (min_free_space_percent_str, NULL, 10);
         if (self->min_free_space_percent > 99)
           return glnx_throw (error, "Invalid min-free-space-percent '%s'", min_free_space_percent_str);
       }
+    else if (!g_key_file_has_key (self->config, "core", "min-free-space-size", NULL))
+      {
+        /* Default fallback of 3% free space. If changing this, be sure to change the man page too */
+        self->min_free_space_percent = 3;
+        self->min_free_space_mb = 0;
+      }
+
+    if (self->min_free_space_percent != 0 && self->min_free_space_mb != 0)
+      {
+        self->min_free_space_percent = 0;
+        g_debug ("Both min-free-space-percent and -size are mentioned in config. Enforcing min-free-space-size check only.");
+      }
   }
 
   {

src/libostree/ostree-sysroot-deploy.c

@@ -1628,8 +1628,9 @@ install_deployment_kernel (OstreeSysroot   *sysroot,
   g_assert_cmpstr (kernel_layout->bootcsum, ==, bootcsum);
   g_autofree char *bootcsumdir = g_strdup_printf ("ostree/%s-%s", osname, bootcsum);
   g_autofree char *bootconfdir = g_strdup_printf ("loader.%d/entries", new_bootversion);
-  g_autofree char *bootconf_name = g_strdup_printf ("ostree-%s-%d.conf", osname,
-                                   ostree_deployment_get_index (deployment));
+  g_autofree char *bootconf_name = g_strdup_printf ("ostree-%d-%s.conf",
+                                   n_deployments - ostree_deployment_get_index (deployment),
+                                   osname);
   if (!glnx_shutil_mkdir_p_at (boot_dfd, bootcsumdir, 0775, cancellable, error))
     return FALSE;
 
@@ -2181,39 +2182,56 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot     *self,
 {
   g_assert (self->loaded);
 
-  /* It dramatically simplifies a lot of the logic below if we
-   * drop the staged deployment from both the source deployment list,
-   * as well as the target list.  We don't want to write it to the bootloader
-   * now, which is mostly what this function is concerned with.
-   * In the future we though should probably adapt things to keep it.
+  /* Dealing with the staged deployment is quite tricky here. This function is
+   * primarily concerned with writing out "finalized" deployments which have
+   * bootloader entries. Originally, we simply dropped the staged deployment
+   * here unconditionally. Now, the high level strategy is to retain it, but
+   * *only* if it's the first item in the new deployment list - otherwise, it's
+   * silently dropped.
    */
-  gboolean removed_staged = FALSE;
-  if (self->staged_deployment)
+
+  g_autoptr(GPtrArray) new_deployments_copy = g_ptr_array_new ();
+  gboolean removed_staged = (self->staged_deployment != NULL);
+  if (new_deployments->len > 0)
     {
+      OstreeDeployment *first = new_deployments->pdata[0];
+      /* If the first deployment is the staged, we filter it out for now */
+      g_assert (first);
+      if (first == self->staged_deployment)
+        {
+          g_assert (ostree_deployment_is_staged (first));
+
+          /* In this case note staged was retained */
+          removed_staged = FALSE;
+        }
+
+      /* Create a copy without any staged deployments */
+      for (guint i = 0; i < new_deployments->len; i++)
+        {
+          OstreeDeployment *deployment = new_deployments->pdata[i];
+          if (!ostree_deployment_is_staged (deployment))
+            g_ptr_array_add (new_deployments_copy, deployment);
+        }
+      new_deployments = new_deployments_copy;
+    }
+
+  /* Take care of removing the staged deployment's on-disk state if we should */
+  if (removed_staged)
+    {
+      g_assert (self->staged_deployment);
+      g_assert (self->staged_deployment == self->deployments->pdata[0]);
+
       if (!glnx_unlinkat (AT_FDCWD, _OSTREE_SYSROOT_RUNSTATE_STAGED, 0, error))
         return FALSE;
 
       if (!_ostree_sysroot_rmrf_deployment (self, self->staged_deployment, cancellable, error))
         return FALSE;
 
-      g_assert (self->staged_deployment == self->deployments->pdata[0]);
+      /* Clear it out of the *current* deployments list to maintain invariants */
+      self->staged_deployment = NULL;
       g_ptr_array_remove_index (self->deployments, 0);
-      removed_staged = TRUE;
     }
-  /* First new deployment; we'll see if it's staged */
-  OstreeDeployment *first_new =
-    (new_deployments->len > 0 ? new_deployments->pdata[0] : NULL);
-  g_autoptr(GPtrArray) new_deployments_copy = NULL;
-  if (first_new && ostree_deployment_is_staged (first_new))
-    {
-      g_assert_cmpint (new_deployments->len, >, 0);
-      new_deployments_copy = g_ptr_array_sized_new (new_deployments->len - 1);
-      for (guint i = 1; i < new_deployments->len; i++)
-        g_ptr_array_add (new_deployments_copy, new_deployments->pdata[i]);
-    }
-  else
-    new_deployments_copy = g_ptr_array_ref (new_deployments);
-  new_deployments = new_deployments_copy;
+  const guint nonstaged_current_len = self->deployments->len - (self->staged_deployment ? 1 : 0);
 
   /* Assign a bootserial to each new deployment.
    */
@@ -2225,7 +2243,8 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot     *self,
    * subbootversion bootlinks.
    */
   gboolean requires_new_bootversion = FALSE;
-  if (new_deployments->len != self->deployments->len)
+
+  if (new_deployments->len != nonstaged_current_len)
     requires_new_bootversion = TRUE;
   else
     {
@@ -2834,7 +2853,7 @@ _ostree_sysroot_finalize_staged (OstreeSysroot *self,
   if (!glnx_unlinkat (AT_FDCWD, _OSTREE_SYSROOT_RUNSTATE_STAGED, 0, error))
     return FALSE;
 
-  if (!sysroot_finalize_deployment (self, self->staged_deployment, NULL, merge_deployment,
+  if (!sysroot_finalize_deployment (self, self->staged_deployment, kargs, merge_deployment,
                                     cancellable, error))
     return FALSE;
 

src/libostree/ostree-sysroot.c

@@ -1894,7 +1894,8 @@ ostree_sysroot_deployment_unlock (OstreeSysroot     *self,
  * for e.g. older versions of libostree unaware of pinning to GC the deployment.
  *
  * This function does nothing and returns successfully if the deployment
- * is already in the desired pinning state.
+ * is already in the desired pinning state.  It is an error to try to pin
+ * the staged deployment (as it's not in the bootloader entries).
  *
  * Since: 2018.3
  */
@@ -1908,6 +1909,9 @@ ostree_sysroot_deployment_set_pinned (OstreeSysroot     *self,
   if (is_pinned == current_pin)
     return TRUE;
 
+  if (ostree_deployment_is_staged (deployment))
+    return glnx_throw (error, "Cannot pin staged deployment");
+
   g_autoptr(OstreeDeployment) deployment_clone = ostree_deployment_clone (deployment);
   GKeyFile *origin_clone = ostree_deployment_get_origin (deployment_clone);
 

src/libostree/ostree-version.h

@@ -43,7 +43,7 @@
  *
  * Since: 2017.4
  */
-#define OSTREE_RELEASE_VERSION (6)
+#define OSTREE_RELEASE_VERSION (7)
 
 /**
  * OSTREE_VERSION
@@ -52,7 +52,7 @@
  *
  * Since: 2017.4
  */
-#define OSTREE_VERSION (2018.6)
+#define OSTREE_VERSION (2018.7)
 
 /**
  * OSTREE_VERSION_S:
@@ -62,7 +62,7 @@
  *
  * Since: 2017.4
  */
-#define OSTREE_VERSION_S "2018.6"
+#define OSTREE_VERSION_S "2018.7"
 
 #define OSTREE_ENCODE_VERSION(year,release) \
         ((year) << 16 | (release))