Move ref writing to be transaction-based

Rather than having separate write_ref calls, make clients start a
transaction, add some refs, and then commit it. While this doesn't
make it 100% atomic, it makes it easier for us to use an atomic
model, and it means we don't do as much I/O updating the summary
file and such.

https://bugzilla.gnome.org/show_bug.cgi?id=707644

doc/ostree-sections.txt

@@ -68,6 +68,8 @@ ostree_repo_scan_hardlinks
 ostree_repo_prepare_transaction
 ostree_repo_commit_transaction
 ostree_repo_abort_transaction
+ostree_repo_transaction_set_ref
+ostree_repo_transaction_set_refspec
 ostree_repo_has_object
 ostree_repo_write_metadata
 ostree_repo_write_metadata_async
@@ -78,8 +80,6 @@ ostree_repo_write_content_trusted
 ostree_repo_write_content_async
 ostree_repo_write_content_finish
 ostree_repo_resolve_rev
-ostree_repo_write_ref
-ostree_repo_write_refspec
 ostree_repo_list_refs
 ostree_repo_load_variant
 ostree_repo_load_variant_if_exists

src/libostree/ostree-repo-commit.c

@@ -652,6 +652,74 @@ cleanup_tmpdir (OstreeRepo        *self,
   return ret;
 }
 
+static void
+ensure_txn_refs (OstreeRepo *self)
+{
+  if (self->txn_refs == NULL)
+    self->txn_refs = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
+}
+
+/**
+ * ostree_repo_transaction_set_refspec:
+ * @self: An #OstreeRepo
+ * @refspec: The refspec to write
+ * @checksum: The checksum to point it to
+ *
+ * Like ostree_repo_transaction_set__ref(), but takes concatenated
+ * @refspec format as input instead of separate remote and name
+ * arguments.
+ */
+void
+ostree_repo_transaction_set_refspec (OstreeRepo *self,
+                                     const char *refspec,
+                                     const char *checksum)
+{
+  g_return_if_fail (self->in_transaction == TRUE);
+
+  ensure_txn_refs (self);
+
+  g_hash_table_replace (self->txn_refs, g_strdup (refspec), g_strdup (checksum));
+}
+
+/**
+ * ostree_repo_transaction_set_ref:
+ * @self: An #OstreeRepo
+ * @remote: (allow-none): A remote for the ref
+ * @ref: The ref to write
+ * @checksum: The checksum to point it to
+ *
+ * If @checksum is not %NULL, then record it as the target of ref named
+ * @ref; if @remote is provided, the ref will appear to originate from that
+ * remote.
+ *
+ * Otherwise, if @checksum is %NULL, then record that the ref should
+ * be deleted.
+ *
+ * The change will not be written out immediately, but when the transaction
+ * is completed with ostree_repo_complete_transaction(). If the transaction
+ * is instead aborted with ostree_repo_abort_transaction(), no changes will
+ * be made to the repository.
+ */
+void
+ostree_repo_transaction_set_ref (OstreeRepo *self,
+                                 const char *remote,
+                                 const char *ref,
+                                 const char *checksum)
+{
+  char *refspec;
+
+  g_return_if_fail (self->in_transaction == TRUE);
+
+  ensure_txn_refs (self);
+
+  if (remote)
+    refspec = g_strdup_printf ("%s:%s", remote, ref);
+  else
+    refspec = g_strdup (ref);
+
+  g_hash_table_replace (self->txn_refs, refspec, g_strdup (checksum));
+}
+
 gboolean
 ostree_repo_commit_transaction (OstreeRepo                  *self,
                                 OstreeRepoTransactionStats  *out_stats,
@@ -668,6 +736,11 @@ ostree_repo_commit_transaction (OstreeRepo                  *self,
   if (self->loose_object_devino_hash)
     g_hash_table_remove_all (self->loose_object_devino_hash);
 
+  if (self->txn_refs)
+    if (!_ostree_repo_update_refs (self, self->txn_refs, cancellable, error))
+      goto out;
+  g_clear_pointer (&self->txn_refs, g_hash_table_destroy);
+
   self->in_transaction = FALSE;
 
   if (!ot_gfile_ensure_unlinked (self->transaction_lock_path, cancellable, error))
@@ -697,6 +770,8 @@ ostree_repo_abort_transaction (OstreeRepo     *self,
   if (self->loose_object_devino_hash)
     g_hash_table_remove_all (self->loose_object_devino_hash);
 
+  g_clear_pointer (&self->txn_refs, g_hash_table_destroy);
+
   self->in_transaction = FALSE;
 
   ret = TRUE;

src/libostree/ostree-repo-private.h

@@ -39,6 +39,7 @@ struct OstreeRepo {
   GFile *config_file;
 
   GFile *transaction_lock_path;
+  GHashTable *txn_refs;
   GMutex txn_stats_lock;
   OstreeRepoTransactionStats txn_stats;
 
@@ -92,6 +93,11 @@ _ostree_repo_write_directory_meta (OstreeRepo   *self,
                                    guchar      **out_csum,
                                    GCancellable *cancellable,
                                    GError      **error);
+gboolean
+_ostree_repo_update_refs (OstreeRepo        *self,
+                          GHashTable        *refs,
+                          GCancellable      *cancellable,
+                          GError           **error);
 
 G_END_DECLS
 

src/libostree/ostree-repo-pull.c

@@ -1332,8 +1332,6 @@ ostree_repo_pull (OstreeRepo               *self,
   if (!run_mainloop_monitor_fetcher (pull_data))
     goto out;
   
-  if (!ostree_repo_commit_transaction (pull_data->repo, NULL, cancellable, error))
-    goto out;
 
   g_hash_table_iter_init (&hash_iter, updated_refs);
   while (g_hash_table_iter_next (&hash_iter, &key, &value))
@@ -1354,13 +1352,15 @@ ostree_repo_pull (OstreeRepo               *self,
         }
       else
         {
-          if (!ostree_repo_write_ref (pull_data->repo, pull_data->remote_name, ref, checksum, error))
+          ostree_repo_transaction_set_ref (pull_data->repo, pull_data->remote_name, ref, checksum);
-            goto out;
+
-          
           g_print ("remote %s is now %s\n", remote_ref, checksum);
         }
     }
-      
+
+  if (!ostree_repo_commit_transaction (pull_data->repo, NULL, cancellable, error))
+    goto out;
+
   end_time = g_get_monotonic_time ();
 
   bytes_transferred = ostree_fetcher_bytes_transferred (pull_data->fetcher);

src/libostree/ostree-repo-refs.c

@@ -63,6 +63,7 @@ static gboolean
 write_checksum_file (GFile *parentdir,
                      const char *name,
                      const char *sha256,
+                     GCancellable *cancellable,
                      GError **error)
 {
   gboolean ret = FALSE;
@@ -98,7 +99,7 @@ write_checksum_file (GFile *parentdir,
     {
       child = g_file_get_child (parent, (char*)components->pdata[i]);
 
-      if (!gs_file_ensure_directory (child, FALSE, NULL, error))
+      if (!gs_file_ensure_directory (child, FALSE, cancellable, error))
         goto out;
 
       g_clear_object (&parent);
@@ -107,13 +108,13 @@ write_checksum_file (GFile *parentdir,
     }
 
   child = g_file_get_child (parent, components->pdata[components->len - 1]);
-  if ((out = (GOutputStream*)g_file_replace (child, NULL, FALSE, 0, NULL, error)) == NULL)
+  if ((out = (GOutputStream*)g_file_replace (child, NULL, FALSE, 0, cancellable, error)) == NULL)
     goto out;
-  if (!g_output_stream_write_all (out, sha256, strlen (sha256), &bytes_written, NULL, error))
+  if (!g_output_stream_write_all (out, sha256, strlen (sha256), &bytes_written, cancellable, error))
     goto out;
-  if (!g_output_stream_write_all (out, "\n", 1, &bytes_written, NULL, error))
+  if (!g_output_stream_write_all (out, "\n", 1, &bytes_written, cancellable, error))
     goto out;
-  if (!g_output_stream_close (out, NULL, error))
+  if (!g_output_stream_close (out, cancellable, error))
     goto out;
 
   ret = TRUE;
@@ -613,33 +614,21 @@ write_ref_summary (OstreeRepo      *self,
   return ret;
 }
 
-/**
+static gboolean      
- * ostree_repo_write_ref:
+write_refspec (OstreeRepo    *self,
- * @self: Repo
+               const char    *refspec,
- * @remote: (allow-none): Optional remote name
+               const char    *rev,
- * @name: Name of ref, e.g. foo/bar/baz
+               GCancellable  *cancellable,
- * @rev: (allow-none); ASCII SHA256 checksum; if %NULL, then delete @name
+               GError       **error)
- * @error: Error
- *
- * If @rev is not %NULL, then it as the target of ref named @name; if
- * @remote is provided, the ref will appear originate from that
- * remote.
- *
- * Otherwise, if @rev is %NULL, then delete the ref @name if it exists.
- *
- * This function merely changes the ref target; it is possible to use
- * it to target earlier commits.
- */
-gboolean      
-ostree_repo_write_ref (OstreeRepo  *self,
-                       const char  *remote,
-                       const char  *name,
-                       const char  *rev,
-                       GError     **error)
 {
   gboolean ret = FALSE;
+  gs_free char *remote = NULL;
+  gs_free char *name = NULL;
   gs_unref_object GFile *dir = NULL;
 
+  if (!ostree_parse_refspec (refspec, &remote, &name, error))
+    goto out;
+
   if (remote == NULL)
     dir = g_object_ref (self->local_heads_dir);
   else
@@ -648,7 +637,7 @@ ostree_repo_write_ref (OstreeRepo  *self,
       
       if (rev != NULL)
         {
-          if (!gs_file_ensure_directory (dir, FALSE, NULL, error))
+          if (!gs_file_ensure_directory (dir, FALSE, cancellable, error))
             goto out;
         }
     }
@@ -656,26 +645,17 @@ ostree_repo_write_ref (OstreeRepo  *self,
   if (rev == NULL)
     {
       gs_unref_object GFile *child = g_file_resolve_relative_path (dir, name);
-      
+
-      if (g_file_query_exists (child, NULL))
+      if (g_file_query_exists (child, cancellable))
         {
-          if (!gs_file_unlink (child, NULL, error))
+          if (!gs_file_unlink (child, cancellable, error))
             goto out;
         }
     }
   else
     {
-      if (!write_checksum_file (dir, name, rev, error))
+      if (!write_checksum_file (dir, name, rev, cancellable, error))
         goto out;
-      
-      if (rev != NULL)
-        {
-          if (self->mode == OSTREE_REPO_MODE_ARCHIVE_Z2)
-            {
-              if (!write_ref_summary (self, NULL, error))
-                goto out;
-            }
-        }
     }
 
   ret = TRUE;
@@ -683,31 +663,31 @@ ostree_repo_write_ref (OstreeRepo  *self,
   return ret;
 }
 
-/**
- * ostree_repo_write_refspec:
- * @self: Repo
- * @refspec: Optional remote with name of ref, e.g. remotename:foo/bar/baz
- * @rev: (allow-none); ASCII SHA256 checksum; if %NULL, then delete @refspec
- * @error: Error
- *
- * Like ostree_repo_write_ref(), but takes concatenated @refspec
- * format as input instead of separate remote and name arguments.
- */
 gboolean
-ostree_repo_write_refspec (OstreeRepo  *self,
+_ostree_repo_update_refs (OstreeRepo        *self,
-                           const char  *refspec,
+                          GHashTable        *refs,
-                           const char  *rev,
+                          GCancellable      *cancellable,
-                           GError     **error)
+                          GError           **error)
 {
   gboolean ret = FALSE;
-  gs_free char *remote = NULL;
+  GHashTableIter hash_iter;
-  gs_free char *ref = NULL;
+  gpointer key, value;
 
-  if (!ostree_parse_refspec (refspec, &remote, &ref, error))
+  g_hash_table_iter_init (&hash_iter, refs);
-    goto out;
+  while (g_hash_table_iter_next (&hash_iter, &key, &value))
+    {
+      const char *refspec = key;
+      const char *rev = value;
 
-  if (!ostree_repo_write_ref (self, remote, ref, rev, error))
+      if (!write_refspec (self, refspec, rev, cancellable, error))
-    goto out;
+        goto out;
+    }
+
+  if (self->mode == OSTREE_REPO_MODE_ARCHIVE_Z2)
+    {
+      if (!write_ref_summary (self, cancellable, error))
+        goto out;
+    }
 
   ret = TRUE;
  out:

src/libostree/ostree-repo.c

@@ -100,6 +100,7 @@ ostree_repo_finalize (GObject *object)
     g_hash_table_destroy (self->updated_uncompressed_dirs);
   if (self->config)
     g_key_file_free (self->config);
+  g_clear_pointer (&self->txn_refs, g_hash_table_destroy);
   g_clear_pointer (&self->cached_meta_indexes, (GDestroyNotify) g_ptr_array_unref);
   g_clear_pointer (&self->cached_content_indexes, (GDestroyNotify) g_ptr_array_unref);
   g_mutex_clear (&self->cache_lock);

src/libostree/ostree-repo.h

@@ -130,6 +130,15 @@ gboolean      ostree_repo_abort_transaction (OstreeRepo     *self,
                                              GCancellable   *cancellable,
                                              GError        **error);
 
+void          ostree_repo_transaction_set_refspec (OstreeRepo *self,
+                                                   const char *refspec,
+                                                   const char *checksum);
+
+void          ostree_repo_transaction_set_ref     (OstreeRepo *self,
+                                                   const char *remote,
+                                                   const char *ref,
+                                                   const char *checksum);
+
 gboolean      ostree_repo_has_object (OstreeRepo           *self,
                                       OstreeObjectType      objtype,
                                       const char           *checksum,
@@ -199,17 +208,6 @@ gboolean      ostree_repo_resolve_rev (OstreeRepo  *self,
                                        char       **out_rev,
                                        GError     **error);
 
-gboolean      ostree_repo_write_ref (OstreeRepo  *self,
-                                     const char  *remote,
-                                     const char  *name,
-                                     const char  *rev,
-                                     GError     **error);
-
-gboolean      ostree_repo_write_refspec (OstreeRepo  *self,
-                                         const char  *refspec,
-                                         const char  *rev,
-                                         GError     **error);
-
 gboolean      ostree_repo_list_refs (OstreeRepo       *self,
                                      const char       *refspec_prefix,
                                      GHashTable      **out_all_refs,

src/ostree/ot-admin-cleanup.c

@@ -400,17 +400,23 @@ cleanup_ref_prefix (OstreeRepo         *repo,
   if (!ostree_repo_list_refs (repo, prefix, &refs, cancellable, error))
     goto out;
 
+  if (!ostree_repo_prepare_transaction (repo, NULL, cancellable, error))
+    goto out;
+
   g_hash_table_iter_init (&hashiter, refs);
   while (g_hash_table_iter_next (&hashiter, &hashkey, &hashvalue))
     {
       const char *suffix = hashkey;
       gs_free char *ref = g_strconcat (prefix, "/", suffix, NULL);
-      if (!ostree_repo_write_refspec (repo, ref, NULL, error))
+      ostree_repo_transaction_set_refspec (repo, ref, NULL);
-        goto out;
     }
 
+  if (!ostree_repo_commit_transaction (repo, NULL, cancellable, error))
+    goto out;
+
   ret = TRUE;
  out:
+  ostree_repo_abort_transaction (repo, cancellable, NULL);
   return ret;
 }
 
@@ -451,8 +457,13 @@ generate_deployment_refs_and_prune (GFile               *sysroot,
       gs_free char *refname = g_strdup_printf ("ostree/%d/%d/%u",
                                                bootversion, subbootversion,
                                                i);
-      if (!ostree_repo_write_refspec (repo, refname, ot_deployment_get_csum (deployment),
+
-                                      error))
+      if (!ostree_repo_prepare_transaction (repo, NULL, cancellable, error))
+        goto out;
+
+      ostree_repo_transaction_set_refspec (repo, refname, ot_deployment_get_csum (deployment));
+
+      if (!ostree_repo_commit_transaction (repo, NULL, cancellable, error))
         goto out;
     }
 
@@ -468,6 +479,7 @@ generate_deployment_refs_and_prune (GFile               *sysroot,
 
   ret = TRUE;
  out:
+  ostree_repo_abort_transaction (repo, cancellable, NULL);
   return ret;
 }
   

src/ostree/ot-builtin-commit.c

@@ -420,11 +420,10 @@ ostree_builtin_commit (int argc, char **argv, OstreeRepo *repo, GCancellable *ca
                                      &commit_checksum, cancellable, error))
         goto out;
 
+      ostree_repo_transaction_set_ref (repo, NULL, opt_branch, commit_checksum);
+
       if (!ostree_repo_commit_transaction (repo, &stats, cancellable, error))
         goto out;
-      
-      if (!ostree_repo_write_ref (repo, NULL, opt_branch, commit_checksum, error))
-        goto out;
     }
   else
     {

src/ostree/ot-builtin-pull-local.c

@@ -290,9 +290,6 @@ ostree_builtin_pull_local (int argc, char **argv, OstreeRepo *repo, GCancellable
         gs_console_end_status_line (data->console, NULL, NULL);
     }
 
-  if (!ostree_repo_commit_transaction (data->dest_repo, NULL, NULL, error))
-    goto out;
-
   g_print ("Writing %u refs\n", g_hash_table_size (refs_to_clone));
 
   g_hash_table_iter_init (&hash_iter, refs_to_clone);
@@ -301,11 +298,12 @@ ostree_builtin_pull_local (int argc, char **argv, OstreeRepo *repo, GCancellable
       const char *name = key;
       const char *checksum = value;
         
-      if (!ostree_repo_write_ref (data->dest_repo, opt_remote, name, checksum,
+      ostree_repo_transaction_set_ref (data->dest_repo, opt_remote, name, checksum);
-                                  error))
-        goto out;
     }
 
+  if (!ostree_repo_commit_transaction (data->dest_repo, NULL, NULL, error))
+    goto out;
+
   ret = TRUE;
  out:
   g_clear_pointer (&data->threadpool, (GDestroyNotify) g_thread_pool_free);
@@ -315,5 +313,6 @@ ostree_builtin_pull_local (int argc, char **argv, OstreeRepo *repo, GCancellable
     g_object_unref (data->dest_repo);
   if (context)
     g_option_context_free (context);
+  ostree_repo_abort_transaction (repo, cancellable, NULL);
   return ret;
 }

src/ostree/ot-builtin-refs.c

@@ -67,18 +67,24 @@ ostree_builtin_refs (int argc, char **argv, OstreeRepo *repo, GCancellable *canc
     }
   else
     {
+      if (!ostree_repo_prepare_transaction (repo, NULL, cancellable, error))
+        goto out;
+
       g_hash_table_iter_init (&hashiter, refs);
       while (g_hash_table_iter_next (&hashiter, &hashkey, &hashvalue))
         {
           const char *refspec = hashkey;
-          if (!ostree_repo_write_refspec (repo, refspec, NULL, error))
+          ostree_repo_transaction_set_refspec (repo, refspec, NULL);
-            goto out;
         }
+
+      if (!ostree_repo_commit_transaction (repo, NULL, cancellable, error))
+        goto out;
     }
- 
+
   ret = TRUE;
  out:
   if (context)
     g_option_context_free (context);
+  ostree_repo_abort_transaction (repo, cancellable, NULL);
   return ret;
 }

src/ostree/ot-builtin-reset.c

@@ -118,12 +118,18 @@ ostree_builtin_reset (int           argc,
   if (!check_revision_is_parent (repo, current, checksum, cancellable, error))
     goto out;
 
-  if (!ostree_repo_write_ref (repo, NULL, ref, checksum, error))
+  if (!ostree_repo_prepare_transaction (repo, NULL, cancellable, error))
-     goto out;
+    goto out;
+
+  ostree_repo_transaction_set_ref (repo, NULL, ref, checksum);
+
+  if (!ostree_repo_commit_transaction (repo, NULL, cancellable, error))
+    goto out;
 
   ret = TRUE;
  out:
   if (context)
     g_option_context_free (context);
+  ostree_repo_abort_transaction (repo, cancellable, NULL);
   return ret;
 }

src/ostree/ot-builtin-write-refs.c

@@ -53,6 +53,9 @@ ostree_builtin_write_refs (int argc, char **argv, OstreeRepo *repo, GCancellable
   instream = (GInputStream*)g_unix_input_stream_new (0, FALSE);
   datastream = g_data_input_stream_new (instream);
 
+  if (!ostree_repo_prepare_transaction (repo, NULL, cancellable, error))
+    goto out;
+
   while ((line = g_data_input_stream_read_line (datastream, &len,
                                                 cancellable, &temp_error)) != NULL)
     {
@@ -70,8 +73,7 @@ ostree_builtin_write_refs (int argc, char **argv, OstreeRepo *repo, GCancellable
       if (!ostree_validate_structureof_checksum_string (spc + 1, error))
         goto out;
 
-      if (!ostree_repo_write_ref (repo, NULL, ref, spc + 1, error))
+      ostree_repo_transaction_set_ref (repo, NULL, ref, spc + 1);
-        goto out;
 
       g_free (line);
     }
@@ -81,9 +83,13 @@ ostree_builtin_write_refs (int argc, char **argv, OstreeRepo *repo, GCancellable
       goto out;
     }
 
+  if (!ostree_repo_commit_transaction (repo, NULL, cancellable, error))
+    goto out;
+
   ret = TRUE;
  out:
   if (context)
     g_option_context_free (context);
+  ostree_repo_abort_transaction (repo, cancellable, NULL);
   return ret;
 }