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

4 files changed, 193 insertions, 44 deletions

diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index b627046b..fb4ff91c 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 8ba09303..7320f2c7 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 b28dc5a6..fc7f93ec 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 65efdc1e..5e1a0eed 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"