Add a notion of "physical" sysroot, use for remote writing

(Note this PR was reverted in <https://github.com/ostreedev/ostree/pull/902>;
 this version should be better)

Using `${sysroot}` to mean the physical storage root: We don't want to write to
`${sysroot}/etc/ostree/remotes.d`, since nothing will read it, and really
`${sysroot}` should just have `/ostree` (ideally). Today the Anaconda rpmostree
code ends up writing there. Fix this by adding a notion of "physical" sysroot.
We determine whether the path is physical by checking for `/sysroot`, which
exists in deployment roots (and there shouldn't be a `${sysroot}/sysroot`).

In order to unit test this, I added a `--sysroot` argument to `remote add`.
However, doing this better would require reworking the command line parsing for
the `remote` argument to support specifying `--repo` or `--sysroot`, and I
didn't quite want to do that yet in this patch.

This second iteration of this patch fixes the bug we hit the first time;
embarassingly enough I broke `ostree remote list` finding system remotes.
The fix is to have `ostree_repo_open()` figure out whether it's the same
as `/ostree/repo` for now.

Down the line...we might consider having the `ostree remote` command line itself
instatiate an `OstreeSysroot` by default, but this maximizes compatibility; we
just have to pay a small cost that `ostree` usage outside of that case like
`ostree static-delta` in a releng Jenkins job or whatever will do this `stat()`
too.

Closes: https://github.com/ostreedev/ostree/issues/892

Closes: #1008
Approved by: mbarnes

src/libostree/ostree-repo.c

@@ -32,6 +32,7 @@
 #include <glnx-console.h>
 
 #include "ostree-core-private.h"
+#include "ostree-sysroot-private.h"
 #include "ostree-remote-private.h"
 #include "ostree-repo-private.h"
 #include "ostree-repo-file.h"
@@ -125,6 +126,10 @@ G_DEFINE_TYPE (OstreeRepo, ostree_repo, G_TYPE_OBJECT)
 
 #define SYSCONF_REMOTES SHORTENED_SYSCONFDIR "/ostree/remotes.d"
 
+static GFile *
+get_remotes_d_dir (OstreeRepo          *self,
+                   GFile               *sysroot);
+
 OstreeRemote *
 _ostree_repo_get_remote (OstreeRepo  *self,
                          const char  *name,
@@ -467,6 +472,7 @@ ostree_repo_finalize (GObject *object)
   if (self->uncompressed_objects_dir_fd != -1)
     (void) close (self->uncompressed_objects_dir_fd);
   g_clear_object (&self->sysroot_dir);
+  g_weak_ref_clear (&self->sysroot);
   g_free (self->remotes_config_dir);
 
   if (self->loose_object_devino_hash)
@@ -547,10 +553,6 @@ ostree_repo_constructed (GObject *object)
 
   g_assert (self->repodir != NULL);
 
-  /* Ensure the "sysroot-path" property is set. */
-  if (self->sysroot_dir == NULL)
-    self->sysroot_dir = g_object_ref (_ostree_get_default_sysroot_path ());
-
   G_OBJECT_CLASS (ostree_repo_parent_class)->constructed (object);
 }
 
@@ -645,6 +647,7 @@ ostree_repo_init (OstreeRepo *self)
   self->objects_dir_fd = -1;
   self->uncompressed_objects_dir_fd = -1;
   self->commit_stagedir_lock = empty_lockfile;
+  self->sysroot_kind = OSTREE_REPO_SYSROOT_KIND_UNKNOWN;
 }
 
 /**
@@ -728,18 +731,20 @@ ostree_repo_new_default (void)
 gboolean
 ostree_repo_is_system (OstreeRepo   *repo)
 {
-  g_autoptr(GFile) default_repo_path = NULL;
-
   g_return_val_if_fail (OSTREE_IS_REPO (repo), FALSE);
 
   /* If we were created via ostree_sysroot_get_repo(), we know the answer is yes
    * without having to compare file paths.
    */
-  if (repo->is_system)
+  if (repo->sysroot_kind == OSTREE_REPO_SYSROOT_KIND_VIA_SYSROOT ||
+      repo->sysroot_kind == OSTREE_REPO_SYSROOT_KIND_IS_SYSROOT_OSTREE)
     return TRUE;
 
-  default_repo_path = get_default_repo_path (repo->sysroot_dir);
+  /* No sysroot_dir set?  Not a system repo then. */
+  if (!repo->sysroot_dir)
+    return FALSE;
 
+  g_autoptr(GFile) default_repo_path = get_default_repo_path (repo->sysroot_dir);
   return g_file_equal (repo->repodir, default_repo_path);
 }
 
@@ -916,24 +921,11 @@ impl_repo_remote_add (OstreeRepo     *self,
 
   remote = ostree_remote_new (name);
 
-  /* The OstreeRepo maintains its own internal system root path,
-   * so we need to not only check if a "sysroot" argument was given
-   * but also whether it's actually different from OstreeRepo's.
-   *
-   * XXX Having API regret about the "sysroot" argument now.
-   */
-  gboolean different_sysroot = FALSE;
-  if (sysroot != NULL)
-    different_sysroot = !g_file_equal (sysroot, self->sysroot_dir);
-
-  if (different_sysroot || ostree_repo_is_system (self))
+  g_autoptr(GFile) etc_ostree_remotes_d = get_remotes_d_dir (self, sysroot);
+  if (etc_ostree_remotes_d)
     {
       g_autoptr(GError) local_error = NULL;
 
-      if (sysroot == NULL)
-        sysroot = self->sysroot_dir;
-
-      g_autoptr(GFile) etc_ostree_remotes_d = g_file_resolve_relative_path (sysroot, SYSCONF_REMOTES);
       if (!g_file_make_directory_with_parents (etc_ostree_remotes_d,
                                                cancellable, &local_error))
         {
@@ -1874,14 +1866,55 @@ append_one_remote_config (OstreeRepo      *self,
 }
 
 static GFile *
-get_remotes_d_dir (OstreeRepo          *self)
+get_remotes_d_dir (OstreeRepo          *self,
+                   GFile               *sysroot)
 {
-  if (self->remotes_config_dir != NULL)
+  /* Support explicit override */
+  if (self->sysroot_dir != NULL && self->remotes_config_dir != NULL)
     return g_file_resolve_relative_path (self->sysroot_dir, self->remotes_config_dir);
-  else if (ostree_repo_is_system (self))
-    return g_file_resolve_relative_path (self->sysroot_dir, SYSCONF_REMOTES);
 
-  return NULL;
+  g_autoptr(GFile) sysroot_owned = NULL;
+  /* Very complicated sysroot logic; this bit breaks the otherwise mostly clean
+   * layering between OstreeRepo and OstreeSysroot. First, If a sysroot was
+   * provided, use it. Otherwise, check to see whether we reference
+   * /ostree/repo, or if not that, see if we have a ref to a sysroot (and it's
+   * physical).
+   */
+  g_autoptr(OstreeSysroot) sysroot_ref = NULL;
+  if (sysroot == NULL)
+    {
+      /* No explicit sysroot?  Let's see if we have a kind */
+      switch (self->sysroot_kind)
+        {
+        case OSTREE_REPO_SYSROOT_KIND_UNKNOWN:
+          g_assert_not_reached ();
+          break;
+        case OSTREE_REPO_SYSROOT_KIND_NO:
+          break;
+        case OSTREE_REPO_SYSROOT_KIND_IS_SYSROOT_OSTREE:
+          sysroot = sysroot_owned = g_file_new_for_path ("/");
+          break;
+        case OSTREE_REPO_SYSROOT_KIND_VIA_SYSROOT:
+          sysroot_ref = (OstreeSysroot*)g_weak_ref_get (&self->sysroot);
+          /* Only write to /etc/ostree/remotes.d if we are pointed at a deployment */
+          if (sysroot_ref != NULL && !sysroot_ref->is_physical)
+            sysroot = ostree_sysroot_get_path (sysroot_ref);
+          break;
+        }
+    }
+  /* For backwards compat, also fall back to the sysroot-path variable, which we
+   * don't set anymore internally, and I hope no one else uses.
+   */
+  if (sysroot == NULL && sysroot_ref == NULL)
+    sysroot = self->sysroot_dir;
+
+  /* Did we find a sysroot? If not, NULL means use the repo config, otherwise
+   * return the path in /etc.
+   */
+  if (sysroot == NULL)
+    return NULL;
+  else
+    return g_file_resolve_relative_path (sysroot, SYSCONF_REMOTES);
 }
 
 static gboolean
@@ -2036,7 +2069,7 @@ reload_remote_config (OstreeRepo          *self,
   if (!add_remotes_from_keyfile (self, self->config, NULL, error))
     return FALSE;
 
-  g_autoptr(GFile) remotes_d = get_remotes_d_dir (self);
+  g_autoptr(GFile) remotes_d = get_remotes_d_dir (self, NULL);
   if (remotes_d == NULL)
     return TRUE;
 
@@ -2101,6 +2134,7 @@ ostree_repo_open (OstreeRepo    *self,
                   GCancellable  *cancellable,
                   GError       **error)
 {
+  struct stat self_stbuf;
   struct stat stbuf;
 
   g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
@@ -2137,6 +2171,9 @@ ostree_repo_open (OstreeRepo    *self,
       return FALSE;
     }
 
+  if (!glnx_fstat (self->repo_dir_fd, &self_stbuf, error))
+    return FALSE;
+
   if (!glnx_opendirat (self->repo_dir_fd, "objects", TRUE,
                        &self->objects_dir_fd, error))
     {
@@ -2178,6 +2215,27 @@ ostree_repo_open (OstreeRepo    *self,
         return FALSE;
     }
 
+  /* If we weren't created via ostree_sysroot_get_repo(), for backwards
+   * compatibility we need to figure out now whether or not we refer to the
+   * system repo.  See also ostree-sysroot.c.
+   */
+  if (self->sysroot_kind == OSTREE_REPO_SYSROOT_KIND_UNKNOWN)
+    {
+      struct stat system_stbuf;
+      /* Ignore any errors if we can't access /ostree/repo */
+      if (fstatat (AT_FDCWD, "/ostree/repo", &system_stbuf, 0) == 0)
+        {
+          /* Are we the same as /ostree/repo? */
+          if (self_stbuf.st_dev == system_stbuf.st_dev &&
+              self_stbuf.st_ino == system_stbuf.st_ino)
+            self->sysroot_kind = OSTREE_REPO_SYSROOT_KIND_IS_SYSROOT_OSTREE;
+          else
+            self->sysroot_kind = OSTREE_REPO_SYSROOT_KIND_NO;
+        }
+      else
+        self->sysroot_kind = OSTREE_REPO_SYSROOT_KIND_NO;
+    }
+
   if (!ostree_repo_reload_config (self, cancellable, error))
     return FALSE;
 
@@ -4144,7 +4202,7 @@ find_keyring (OstreeRepo          *self,
       return g_steal_pointer (&file);
     }
 
-  g_autoptr(GFile) remotes_d = get_remotes_d_dir (self);
+  g_autoptr(GFile) remotes_d = get_remotes_d_dir (self, NULL);
   if (remotes_d)
     {
       g_autoptr(GFile) file2 = g_file_get_child (remotes_d, remote->keyring);