[ostree] Add more flexible _remote_change() API , expose via 'ostree remote'

[Colin Walters <walters src gnome org>]

commit f6a6e68412c9f1be2b5de1ded79b92aa340ab22c
Author: Colin Walters <walters verbum org>
Date:   Mon Dec 15 16:21:15 2014 -0500

    Add more flexible _remote_change() API , expose via 'ostree remote'
    
    For Anaconda, I needed OSTREE_REPO_REMOTE_CHANGE_ADD_IF_NOT_EXISTS,
    with the GFile *sysroot argument to avoid ugly hacks.  We want to
    write the content provided via "ostreesetup" as a remote to the target
    chroot only in the case where it isn't provided as part of the tree
    content itself.
    
    This is also potentially useful in idempotent systems management tools
    like Ansible.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=741577

 src/libostree/ostree-repo.c    |  165 +++++++++++++++++++++++++++++++---------
 src/libostree/ostree-repo.h    |   16 ++++
 src/ostree/ot-builtin-remote.c |   24 +++++-
 tests/test-remote-add.sh       |   32 +++++++-
 4 files changed, 193 insertions(+), 44 deletions(-)
---
diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index b627046..fb4ff91 100644
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -616,33 +616,16 @@ keyfile_set_from_vardict (GKeyFile     *keyfile,
     }
 }
 
-/**
- * ostree_repo_remote_add:
- * @self: Repo
- * @name: Name of remote
- * @url: URL for remote (if URL begins with metalink=, it will be used as such)
- * @options: (allow-none): GVariant of type a{sv}
- * @cancellable: Cancellable
- * @error: Error
- *
- * Create a new remote named @name pointing to @url.  If @options is
- * provided, then it will be mapped to #GKeyFile entries, where the
- * GVariant dictionary key is an option string, and the value is
- * mapped as follows:
- *   * s: g_key_file_set_string()
- *   * b: g_key_file_set_boolean()
- *   * as: g_key_file_set_string_list()
- *
- */
-gboolean
-ostree_repo_remote_add (OstreeRepo     *self,
-                        const char     *name,
-                        const char     *url,
-                        GVariant       *options,
-                        GCancellable   *cancellable,
-                        GError        **error)
+static gboolean
+impl_repo_remote_add (OstreeRepo     *self,
+                      GFile          *sysroot,
+                      gboolean        if_not_exists,
+                      const char     *name,
+                      const char     *url,
+                      GVariant       *options,
+                      GCancellable   *cancellable,
+                      GError        **error)
 {
-  gs_unref_object GFile *etc_ostree_remotes_d = g_file_new_for_path (SYSCONFDIR "/ostree/remotes.d");
   local_cleanup_remote OstreeRemote *remote = NULL;
   gboolean ret = FALSE;
 
@@ -659,8 +642,12 @@ ostree_repo_remote_add (OstreeRepo     *self,
     }
 
   remote = ost_repo_get_remote (self, name, NULL);
-
-  if (remote != NULL)
+  if (remote != NULL && if_not_exists)
+    {
+      ret = TRUE;
+      goto out;
+    }
+  else if (remote != NULL)
     {
       GFile *file;
 
@@ -680,9 +667,17 @@ ostree_repo_remote_add (OstreeRepo     *self,
   remote->name = g_strdup (name);
   remote->group = g_strdup_printf ("remote \"%s\"", name);
 
-  if (ostree_repo_is_system (self))
+  if (sysroot != NULL || ostree_repo_is_system (self))
     {
+      const char *sysconf_remotes = SYSCONFDIR "/ostree/remotes.d";
       gs_free char *basename = g_strconcat (name, ".conf", NULL);
+      gs_unref_object GFile *etc_ostree_remotes_d = NULL;
+
+      if (sysroot == NULL)
+        etc_ostree_remotes_d = g_file_new_for_path (sysconf_remotes);
+      else
+        etc_ostree_remotes_d = g_file_resolve_relative_path (sysroot, sysconf_remotes + 1);
+
       remote->file = g_file_get_child (etc_ostree_remotes_d, basename);
     }
 
@@ -727,21 +722,42 @@ ostree_repo_remote_add (OstreeRepo     *self,
 }
 
 /**
- * ostree_repo_remote_delete:
+ * ostree_repo_remote_add:
  * @self: Repo
  * @name: Name of remote
+ * @url: URL for remote (if URL begins with metalink=, it will be used as such)
+ * @options: (allow-none): GVariant of type a{sv}
  * @cancellable: Cancellable
  * @error: Error
  *
- * Delete the remote named @name.  It is an error if the provided
- * remote does not exist.
+ * Create a new remote named @name pointing to @url.  If @options is
+ * provided, then it will be mapped to #GKeyFile entries, where the
+ * GVariant dictionary key is an option string, and the value is
+ * mapped as follows:
+ *   * s: g_key_file_set_string()
+ *   * b: g_key_file_set_boolean()
+ *   * as: g_key_file_set_string_list()
  *
  */
 gboolean
-ostree_repo_remote_delete (OstreeRepo     *self,
-                           const char     *name,
-                           GCancellable   *cancellable,
-                           GError        **error)
+ostree_repo_remote_add (OstreeRepo     *self,
+                        const char     *name,
+                        const char     *url,
+                        GVariant       *options,
+                        GCancellable   *cancellable,
+                        GError        **error)
+{
+  return impl_repo_remote_add (self, NULL, FALSE, name, url, options,
+                               cancellable, error);
+}
+
+static gboolean
+impl_repo_remote_delete (OstreeRepo     *self,
+                         GFile          *sysroot,
+                         gboolean        if_exists,
+                         const char     *name,
+                         GCancellable   *cancellable,
+                         GError        **error)
 {
   local_cleanup_remote OstreeRemote *remote = NULL;
   gboolean ret = FALSE;
@@ -756,7 +772,17 @@ ostree_repo_remote_delete (OstreeRepo     *self,
       goto out;
     }
 
-  remote = ost_repo_get_remote (self, name, error);
+  if (if_exists)
+    {
+      remote = ost_repo_get_remote (self, name, NULL);
+      if (!remote)
+        {
+          ret = TRUE;
+          goto out;
+        }
+    }
+  else
+    remote = ost_repo_get_remote (self, name, error);
 
   if (remote == NULL)
     goto out;
@@ -790,6 +816,71 @@ ostree_repo_remote_delete (OstreeRepo     *self,
 }
 
 /**
+ * ostree_repo_remote_delete:
+ * @self: Repo
+ * @name: Name of remote
+ * @cancellable: Cancellable
+ * @error: Error
+ *
+ * Delete the remote named @name.  It is an error if the provided
+ * remote does not exist.
+ *
+ */
+gboolean
+ostree_repo_remote_delete (OstreeRepo     *self,
+                           const char     *name,
+                           GCancellable   *cancellable,
+                           GError        **error)
+{
+  return impl_repo_remote_delete (self, NULL, FALSE, name, cancellable, error);
+}
+
+/**
+ * ostree_repo_remote_change:
+ * @self: Repo
+ * @sysroot: (allow-none): System root
+ * @changeop: Operation to perform
+ * @name: Name of remote
+ * @url: URL for remote (if URL begins with metalink=, it will be used as such)
+ * @options: (allow-none): GVariant of type a{sv}
+ * @cancellable: Cancellable
+ * @error: Error
+ *
+ * A combined function handling the equivalent of
+ * ostree_repo_remote_add(), ostree_repo_remote_delete(), with more
+ * options.
+ *
+ *
+ */
+gboolean
+ostree_repo_remote_change (OstreeRepo     *self,
+                           GFile          *sysroot,
+                           OstreeRepoRemoteChange changeop,
+                           const char     *name,
+                           const char     *url,
+                           GVariant       *options,
+                           GCancellable   *cancellable,
+                           GError        **error)
+{
+  switch (changeop)
+    {
+    case OSTREE_REPO_REMOTE_CHANGE_ADD:
+      return impl_repo_remote_add (self, sysroot, FALSE, name, url, options,
+                                   cancellable, error);
+    case OSTREE_REPO_REMOTE_CHANGE_ADD_IF_NOT_EXISTS:
+      return impl_repo_remote_add (self, sysroot, TRUE, name, url, options,
+                                   cancellable, error);
+    case OSTREE_REPO_REMOTE_CHANGE_DELETE:
+      return impl_repo_remote_delete (self, sysroot, FALSE, name,
+                                      cancellable, error);
+    case OSTREE_REPO_REMOTE_CHANGE_DELETE_IF_EXISTS:
+      return impl_repo_remote_delete (self, sysroot, TRUE, name,
+                                      cancellable, error);
+    }
+  g_assert_not_reached ();
+}
+
+/**
  * ostree_repo_remote_get_url:
  * @self: Repo
  * @name: Name of remote
diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h
index 8ba0930..7320f2c 100644
--- a/src/libostree/ostree-repo.h
+++ b/src/libostree/ostree-repo.h
@@ -79,6 +79,22 @@ gboolean      ostree_repo_remote_delete (OstreeRepo     *self,
                                          GCancellable   *cancellable,
                                          GError        **error);
 
+typedef enum {
+  OSTREE_REPO_REMOTE_CHANGE_ADD,
+  OSTREE_REPO_REMOTE_CHANGE_ADD_IF_NOT_EXISTS,
+  OSTREE_REPO_REMOTE_CHANGE_DELETE,
+  OSTREE_REPO_REMOTE_CHANGE_DELETE_IF_EXISTS
+} OstreeRepoRemoteChange;
+
+gboolean      ostree_repo_remote_change (OstreeRepo     *self,
+                                         GFile          *sysroot,
+                                         OstreeRepoRemoteChange changeop,
+                                         const char     *name,
+                                         const char     *url,
+                                         GVariant       *options,
+                                         GCancellable   *cancellable,
+                                         GError        **error);
+
 gboolean      ostree_repo_remote_get_url (OstreeRepo   *self,
                                           const char   *name,
                                           char        **out_url,
diff --git a/src/ostree/ot-builtin-remote.c b/src/ostree/ot-builtin-remote.c
index b28dc5a..fc7f93e 100644
--- a/src/ostree/ot-builtin-remote.c
+++ b/src/ostree/ot-builtin-remote.c
@@ -55,10 +55,12 @@ parse_keyvalue (const char  *keyvalue,
 
 static char **opt_set;
 static gboolean opt_no_gpg_verify;
+static gboolean opt_if_not_exists;
 
 static GOptionEntry add_option_entries[] = {
   { "set", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_set, "Set config option KEY=VALUE for remote", "KEY=VALUE" 
},
   { "no-gpg-verify", 0, 0, G_OPTION_ARG_NONE, &opt_no_gpg_verify, "Disable GPG verification", NULL },
+  { "if-not-exists", 0, 0, G_OPTION_ARG_NONE, &opt_if_not_exists, "Do nothing if the provided remote 
exists", NULL },
   { NULL }
 };
 
@@ -124,17 +126,25 @@ ostree_remote_builtin_add (int argc, char **argv, GCancellable *cancellable, GEr
                            "gpg-verify",
                            g_variant_new_variant (g_variant_new_boolean (FALSE)));
 
-  ret = ostree_repo_remote_add (repo, remote_name, remote_url,
-                                g_variant_builder_end (optbuilder),
-                                cancellable, error);
+  if (!ostree_repo_remote_change (repo, NULL,
+                                  opt_if_not_exists ? OSTREE_REPO_REMOTE_CHANGE_ADD_IF_NOT_EXISTS : 
+                                  OSTREE_REPO_REMOTE_CHANGE_ADD,
+                                  remote_name, remote_url,
+                                  g_variant_builder_end (optbuilder),
+                                  cancellable, error))
+    goto out;
 
+  ret = TRUE;
  out:
   g_option_context_free (context);
 
   return ret;
 }
 
+gboolean opt_if_exists = FALSE;
+
 static GOptionEntry delete_option_entries[] = {
+  { "if-exists", 0, 0, G_OPTION_ARG_NONE, &opt_if_exists, "Do nothing if the provided remote does not 
exist", NULL },
   { NULL }
 };
 
@@ -160,8 +170,14 @@ ostree_remote_builtin_delete (int argc, char **argv, GCancellable *cancellable,
 
   remote_name = argv[1];
 
-  ret = ostree_repo_remote_delete (repo, remote_name, cancellable, error);
+  if (!ostree_repo_remote_change (repo, NULL,
+                                  opt_if_exists ? OSTREE_REPO_REMOTE_CHANGE_DELETE_IF_EXISTS : 
+                                  OSTREE_REPO_REMOTE_CHANGE_DELETE,
+                                  remote_name, NULL, NULL,
+                                  cancellable, error))
+    goto out;
 
+  ret = TRUE;
  out:
   g_option_context_free (context);
 
diff --git a/tests/test-remote-add.sh b/tests/test-remote-add.sh
index 65efdc1..5e1a0ee 100755
--- a/tests/test-remote-add.sh
+++ b/tests/test-remote-add.sh
@@ -25,14 +25,26 @@ echo '1..3'
 
 setup_test_repository "bare"
 $OSTREE remote add origin http://example.com/ostree/gnome
-echo "ok remote add"
-assert_file_has_content $test_tmpdir/repo/config "example.com"
+$OSTREE remote show-url origin >/dev/null
 echo "ok config"
 
 $OSTREE remote add --no-gpg-verify another http://another.com/repo
-assert_file_has_content $test_tmpdir/repo/config "gpg-verify=false"
+$OSTREE remote show-url another >/dev/null
 echo "ok remote no gpg-verify"
 
+if $OSTREE remote add --no-gpg-verify another http://another.example.com/anotherrepo 2>err.txt; then
+    assert_not_reached "Adding duplicate remote unexpectedly succeeded"
+fi
+echo "ok"
+
+$OSTREE remote add --if-not-exists --no-gpg-verify another http://another.example.com/anotherrepo
+$OSTREE remote show-url another >/dev/null
+echo "ok"
+
+$OSTREE remote add --if-not-exists --no-gpg-verify another-noexist 
http://another-noexist.example.com/anotherrepo
+$OSTREE remote show-url another-noexist >/dev/null
+echo "ok"
+
 $OSTREE remote delete another
 echo "ok remote delete"
 
@@ -41,4 +53,18 @@ if $OSTREE remote delete nosuchremote 2>err.txt; then
 fi
 assert_file_has_content err.txt "error: "
 
+$OSTREE remote delete --if-exists nosuchremote
+echo "ok"
 
+if $OSTREE remote show-url nosuchremote 2>/dev/null; then
+    assert_not_reached "Deleting remote unexpectedly failed"
+fi
+echo "ok"
+
+$OSTREE remote delete --if-exists origin
+echo "ok"
+
+if $OSTREE remote show-url origin 2>/dev/null; then
+    assert_not_reached "Deleting remote unexpectedly failed"
+fi
+echo "ok"