lib/repo-refs: Include remote refs when using collections

When working with collections it can be useful to see remote refs rather
than just local and mirrored ones. This commit changes the "ostree refs
-c" output to include remote refs, and includes remote refs with
collection IDs in summary file generation as well. The former behavior
is consistent with how "ostree refs" works, and the latter behavior is
useful in facilitating P2P updates even when mirrors haven't been
configured.

To accomplish this, OstreeRepoListRefsExtFlags was extended with an
EXCLUDE_REMOTES flag. This was done rather than an INCLUDE_REMOTES flag
so that existing calls to ostree_repo_list_refs_ext continue to have the
same behavior. This flag was added to ostree_repo_list_collection_refs
(which is an experimental API break).

Also, add unit tests for the "refs -c" and summary file behavior, and
update relevant tests.

Closes: #1069
Approved by: cgwalters

12 files changed, 175 insertions, 72 deletions

diff --git a/src/libostree/ostree-repo-finder-mount.c b/src/libostree/ostree-repo-finder-mount.c
index 7f257f68..b7b15bf0 100644
--- a/src/libostree/ostree-repo-finder-mount.c
+++ b/src/libostree/ostree-repo-finder-mount.c
@@ -314,7 +314,9 @@ ostree_repo_finder_mount_resolve_async (OstreeRepoFinder                  *finde
 
           g_autoptr(GHashTable) repo_refs = NULL;  /* (element-type OstreeCollectionRef utf8) */
 
-          if (!ostree_repo_list_collection_refs (repo, refs[i]->collection_id, &repo_refs, cancellable, &local_error))
+          if (!ostree_repo_list_collection_refs (repo, refs[i]->collection_id, &repo_refs,
+                                                 OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES,
+                                                 cancellable, &local_error))
             {
               g_debug ("Ignoring ref (%s, %s) on mount ‘%s’ as its refs could not be listed: %s",
                        refs[i]->collection_id, refs[i]->ref_name, mount_name, local_error->message);
diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h
index e38e48bc..121c2d52 100644
--- a/src/libostree/ostree-repo-private.h
+++ b/src/libostree/ostree-repo-private.h
@@ -396,11 +396,12 @@ gboolean      ostree_repo_set_collection_id (OstreeRepo   *self,
                                              const gchar  *collection_id,
                                              GError      **error);
 
-gboolean      ostree_repo_list_collection_refs (OstreeRepo    *self,
-                                                const char    *match_collection_id,
-                                                GHashTable   **out_all_refs,
-                                                GCancellable  *cancellable,
-                                                GError       **error);
+gboolean      ostree_repo_list_collection_refs (OstreeRepo                  *self,
+                                                const char                  *match_collection_id,
+                                                GHashTable                 **out_all_refs,
+                                                OstreeRepoListRefsExtFlags   flags,
+                                                GCancellable                *cancellable,
+                                                GError                     **error);
 
 void          ostree_repo_transaction_set_collection_ref (OstreeRepo                *self,
                                                           const OstreeCollectionRef *ref,
diff --git a/src/libostree/ostree-repo-prune.c b/src/libostree/ostree-repo-prune.c
index 7cd9eb2c..2b596ecb 100644
--- a/src/libostree/ostree-repo-prune.c
+++ b/src/libostree/ostree-repo-prune.c
@@ -335,7 +335,7 @@ ostree_repo_prune (OstreeRepo        *self,
       g_autoptr(GHashTable) all_collection_refs = NULL;  /* (element-type OstreeChecksumRef utf8) */
 
       if (!ostree_repo_list_collection_refs (self, NULL, &all_collection_refs,
-                                             cancellable, error))
+                                             OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES, cancellable, error))
         return FALSE;
 
       GLNX_HASH_TABLE_FOREACH_V (all_collection_refs, const char*, checksum)
diff --git a/src/libostree/ostree-repo-refs.c b/src/libostree/ostree-repo-refs.c
index 6c113fb6..04c99a03 100644
--- a/src/libostree/ostree-repo-refs.c
+++ b/src/libostree/ostree-repo-refs.c
@@ -560,7 +560,7 @@ _ostree_repo_list_refs_internal (OstreeRepo       *self,
       if (!ostree_parse_refspec (refspec_prefix, &remote, &ref_prefix, error))
         return FALSE;
 
-      if (remote)
+      if (!(flags & OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES) && remote)
         {
           prefix_path = glnx_strjoina ("refs/remotes/", remote, "/");
           path = glnx_strjoina (prefix_path, ref_prefix);
@@ -620,32 +620,35 @@ _ostree_repo_list_refs_internal (OstreeRepo       *self,
                                    ret_all_refs, cancellable, error))
         return FALSE;
 
-      g_string_truncate (base_path, 0);
-
-      if (!glnx_dirfd_iterator_init_at (self->repo_dir_fd, "refs/remotes", TRUE, &dfd_iter, error))
-        return FALSE;
-
-      while (TRUE)
+      if (!(flags & OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES))
         {
-          struct dirent *dent;
-          glnx_fd_close int remote_dfd = -1;
+          g_string_truncate (base_path, 0);
 
-          if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent, cancellable, error))
+          if (!glnx_dirfd_iterator_init_at (self->repo_dir_fd, "refs/remotes", TRUE, &dfd_iter, error))
             return FALSE;
-          if (!dent)
-            break;
 
-          if (dent->d_type != DT_DIR)
-            continue;
+          while (TRUE)
+            {
+              struct dirent *dent;
+              glnx_fd_close int remote_dfd = -1;
 
-          if (!glnx_opendirat (dfd_iter.fd, dent->d_name, TRUE, &remote_dfd, error))
-            return FALSE;
+              if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent, cancellable, error))
+                return FALSE;
+              if (!dent)
+                break;
 
-          if (!enumerate_refs_recurse (self, dent->d_name, flags, NULL, remote_dfd, base_path,
-                                       remote_dfd, ".",
-                                       ret_all_refs,
-                                       cancellable, error))
-            return FALSE;
+              if (dent->d_type != DT_DIR)
+                continue;
+
+              if (!glnx_opendirat (dfd_iter.fd, dent->d_name, TRUE, &remote_dfd, error))
+                return FALSE;
+
+              if (!enumerate_refs_recurse (self, dent->d_name, flags, NULL, remote_dfd, base_path,
+                                           remote_dfd, ".",
+                                           ret_all_refs,
+                                           cancellable, error))
+                return FALSE;
+            }
         }
     }
 
@@ -1101,27 +1104,33 @@ _ostree_repo_update_collection_refs (OstreeRepo        *self,
  * @self: Repo
  * @match_collection_id: (nullable): If non-%NULL, only list refs from this collection
  * @out_all_refs: (out) (element-type OstreeCollectionRef utf8): Mapping from collection–ref to checksum
+ * @flags: Options controlling listing behavior
  * @cancellable: Cancellable
  * @error: Error
  *
- * List all local and mirrored refs, mapping them to the commit checksums they
- * currently point to in @out_all_refs. If @match_collection_id is specified,
- * the results will be limited to those with an equal collection ID.
+ * List all local, mirrored, and remote refs, mapping them to the commit
+ * checksums they currently point to in @out_all_refs. If @match_collection_id
+ * is specified, the results will be limited to those with an equal collection
+ * ID.
  *
  * #OstreeCollectionRefs are guaranteed to be returned with their collection ID
  * set to a non-%NULL value; so no refs from `refs/heads` will be listed if no
  * collection ID is configured for the repository
  * (ostree_repo_get_collection_id()).
  *
+ * If you want to exclude refs from `refs/remotes`, use
+ * %OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES in @flags.
+ *
  * Returns: %TRUE on success, %FALSE otherwise
  * Since: 2017.8
  */
 gboolean
-ostree_repo_list_collection_refs (OstreeRepo    *self,
-                                  const char    *match_collection_id,
-                                  GHashTable   **out_all_refs,
-                                  GCancellable  *cancellable,
-                                  GError       **error)
+ostree_repo_list_collection_refs (OstreeRepo                 *self,
+                                  const char                 *match_collection_id,
+                                  GHashTable                 **out_all_refs,
+                                  OstreeRepoListRefsExtFlags flags,
+                                  GCancellable               *cancellable,
+                                  GError                     **error)
 {
   g_return_val_if_fail (OSTREE_IS_REPO (self), FALSE);
   g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), FALSE);
@@ -1130,6 +1139,10 @@ ostree_repo_list_collection_refs (OstreeRepo    *self,
   if (match_collection_id != NULL && !ostree_validate_collection_id (match_collection_id, error))
     return FALSE;
 
+  const gchar *refs_dirs[] = { "refs/mirrors", "refs/remotes", NULL };
+  if (flags & OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES)
+    refs_dirs[1] = NULL;
+
   g_autoptr(GHashTable) ret_all_refs = NULL;
 
   ret_all_refs = g_hash_table_new_full (ostree_collection_ref_hash,
@@ -1150,7 +1163,7 @@ ostree_repo_list_collection_refs (OstreeRepo    *self,
       if (!glnx_opendirat (self->repo_dir_fd, "refs/heads", TRUE, &refs_heads_dfd, error))
         return FALSE;
 
-      if (!enumerate_refs_recurse (self, NULL, OSTREE_REPO_LIST_REFS_EXT_NONE,
+      if (!enumerate_refs_recurse (self, NULL, flags,
                                    main_collection_id, refs_heads_dfd, base_path,
                                    refs_heads_dfd, ".",
                                    ret_all_refs, cancellable, error))
@@ -1159,36 +1172,65 @@ ostree_repo_list_collection_refs (OstreeRepo    *self,
 
   g_string_truncate (base_path, 0);
 
-  gboolean refs_mirrors_exists = FALSE;
-  if (!ot_dfd_iter_init_allow_noent (self->repo_dir_fd, "refs/mirrors",
-                                     &dfd_iter, &refs_mirrors_exists, error))
-    return FALSE;
-
-  while (refs_mirrors_exists)
+  for (const char **iter = refs_dirs; iter && *iter; iter++)
     {
-      struct dirent *dent;
-      glnx_fd_close int collection_dfd = -1;
-
-      if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent, cancellable, error))
+      const char *refs_dir = *iter;
+      gboolean refs_dir_exists = FALSE;
+      if (!ot_dfd_iter_init_allow_noent (self->repo_dir_fd, refs_dir,
+                                         &dfd_iter, &refs_dir_exists, error))
         return FALSE;
-      if (!dent)
-        break;
 
-      if (dent->d_type != DT_DIR)
-        continue;
+      while (refs_dir_exists)
+        {
+          struct dirent *dent;
+          glnx_fd_close int subdir_fd = -1;
+          const gchar *current_collection_id;
+          g_autofree gchar *remote_collection_id = NULL;
 
-      if (match_collection_id != NULL && g_strcmp0 (match_collection_id, dent->d_name) != 0)
-        continue;
+          if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent, cancellable, error))
+            return FALSE;
+          if (!dent)
+            break;
 
-      if (!glnx_opendirat (dfd_iter.fd, dent->d_name, TRUE, &collection_dfd, error))
-        return FALSE;
+          if (dent->d_type != DT_DIR)
+            continue;
 
-      if (!enumerate_refs_recurse (self, NULL, OSTREE_REPO_LIST_REFS_EXT_NONE,
-                                   dent->d_name, collection_dfd, base_path,
-                                   collection_dfd, ".",
-                                   ret_all_refs,
-                                   cancellable, error))
-        return FALSE;
+          if (g_strcmp0 (refs_dir, "refs/mirrors") == 0)
+            {
+              if (match_collection_id != NULL && g_strcmp0 (match_collection_id, dent->d_name) != 0)
+                continue;
+              else
+                current_collection_id = dent->d_name;
+            }
+          else /* refs_dir = "refs/remotes" */
+            {
+              g_autoptr(GError) local_error = NULL;
+              if (!ostree_repo_get_remote_option (self, dent->d_name, "collection-id",
+                                                  NULL, &remote_collection_id, &local_error) ||
+                  !ostree_validate_collection_id (remote_collection_id, &local_error))
+                {
+                  g_debug ("Ignoring remote ‘%s’ due to no valid collection ID being configured for it: %s",
+                           dent->d_name, local_error->message);
+                  g_clear_error (&local_error);
+                  continue;
+                }
+
+              if (match_collection_id != NULL && g_strcmp0 (match_collection_id, current_collection_id) != 0)
+                continue;
+              else
+                  current_collection_id = remote_collection_id;
+            }
+
+          if (!glnx_opendirat (dfd_iter.fd, dent->d_name, TRUE, &subdir_fd, error))
+            return FALSE;
+
+          if (!enumerate_refs_recurse (self, NULL, flags,
+                                       current_collection_id, subdir_fd, base_path,
+                                       subdir_fd, ".",
+                                       ret_all_refs,
+                                       cancellable, error))
+            return FALSE;
+        }
     }
 
   ot_transfer_out_value (out_all_refs, &ret_all_refs);
diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index d6bc4a91..ba33a018 100644
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -4941,7 +4941,8 @@ ostree_repo_regenerate_summary (OstreeRepo     *self,
    * backwards compatibility). */
   {
     g_autoptr(GHashTable) collection_refs = NULL;
-    if (!ostree_repo_list_collection_refs (self, NULL, &collection_refs, cancellable, error))
+    if (!ostree_repo_list_collection_refs (self, NULL, &collection_refs,
+                                           OSTREE_REPO_LIST_REFS_EXT_NONE, cancellable, error))
       return FALSE;
 
     gsize collection_map_size = 0;
diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h
index 0d58729d..73da31e8 100644
--- a/src/libostree/ostree-repo.h
+++ b/src/libostree/ostree-repo.h
@@ -475,10 +475,12 @@ gboolean      ostree_repo_list_refs (OstreeRepo       *self,
  * OstreeRepoListRefsExtFlags:
  * @OSTREE_REPO_LIST_REFS_EXT_NONE: No flags.
  * @OSTREE_REPO_LIST_REFS_EXT_ALIASES: Only list aliases.  Since: 2017.10
+ * @OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES: Exclude remote refs.  Since: 2017.11
  */
 typedef enum {
   OSTREE_REPO_LIST_REFS_EXT_NONE = 0,
-  OSTREE_REPO_LIST_REFS_EXT_ALIASES = 1,
+  OSTREE_REPO_LIST_REFS_EXT_ALIASES = (1 << 0),
+  OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES = (1 << 1),
 } OstreeRepoListRefsExtFlags;
 
 _OSTREE_PUBLIC
@@ -1190,11 +1192,12 @@ gchar *ostree_repo_resolve_keyring_for_collection (OstreeRepo    *self,
                                                    GError       **error);
 
 _OSTREE_PUBLIC
-gboolean ostree_repo_list_collection_refs (OstreeRepo    *self,
-                                           const char    *match_collection_id,
-                                           GHashTable   **out_all_refs,
-                                           GCancellable  *cancellable,
-                                           GError       **error);
+gboolean ostree_repo_list_collection_refs (OstreeRepo                 *self,
+                                           const char                 *match_collection_id,
+                                           GHashTable                 **out_all_refs,
+                                           OstreeRepoListRefsExtFlags flags,
+                                           GCancellable               *cancellable,
+                                           GError                     **error);
 
 #endif /* OSTREE_ENABLE_EXPERIMENTAL_API */
 
diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c
index 795fe098..ed357eca 100644
--- a/src/ostree/ot-builtin-fsck.c
+++ b/src/ostree/ot-builtin-fsck.c
@@ -252,6 +252,7 @@ ostree_builtin_fsck (int argc, char **argv, GCancellable *cancellable, GError **
 
   g_autoptr(GHashTable) all_collection_refs = NULL;  /* (element-type OstreeCollectionRef utf8) */
   if (!ostree_repo_list_collection_refs (repo, NULL, &all_collection_refs,
+                                         OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES,
                                          cancellable, error))
     return FALSE;
 
diff --git a/src/ostree/ot-builtin-prune.c b/src/ostree/ot-builtin-prune.c
index 997d848d..fd00e5fd 100644
--- a/src/ostree/ot-builtin-prune.c
+++ b/src/ostree/ot-builtin-prune.c
@@ -82,7 +82,9 @@ delete_commit (OstreeRepo *repo, const char *commit_to_delete, GCancellable *can
 
 #ifdef OSTREE_ENABLE_EXPERIMENTAL_API
   /* And check refs which *are* in a collection. */
-  if (!ostree_repo_list_collection_refs (repo, NULL, &collection_refs, cancellable, error))
+  if (!ostree_repo_list_collection_refs (repo, NULL, &collection_refs,
+                                         OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES,
+                                         cancellable, error))
     goto out;
 
   g_hash_table_iter_init (&hashiter, collection_refs);
diff --git a/src/ostree/ot-builtin-refs.c b/src/ostree/ot-builtin-refs.c
index ead4ba48..3742f050 100644
--- a/src/ostree/ot-builtin-refs.c
+++ b/src/ostree/ot-builtin-refs.c
@@ -64,7 +64,8 @@ do_ref_with_collections (OstreeRepo    *repo,
 
   if (!ostree_repo_list_collection_refs (repo,
                                          (!opt_create) ? refspec_prefix : NULL,
-                                         &refs, cancellable, error))
+                                         &refs, OSTREE_REPO_LIST_REFS_EXT_NONE,
+                                         cancellable, error))
     goto out;
 
   if (!opt_delete && !opt_create)
diff --git a/tests/test-find-remotes.sh b/tests/test-find-remotes.sh
index 0cf0127f..0bbe54f0 100755
--- a/tests/test-find-remotes.sh
+++ b/tests/test-find-remotes.sh
@@ -56,8 +56,11 @@ ${CMD_PREFIX} ostree --repo=local refs > refs
 assert_file_has_content refs "^apps-remote:app1$"
 assert_file_has_content refs "^os-remote:os/amd64/master$"
 
-${CMD_PREFIX} ostree --repo=local refs --collections | wc -l > refscount
-assert_file_has_content refscount "^0$"
+${CMD_PREFIX} ostree --repo=local refs --collections > refs
+cat refs | wc -l > refscount
+assert_file_has_content refs "^(org.example.AppsCollection, app1)$"
+assert_file_has_content refs "^(org.example.OsCollection, os/amd64/master)$"
+assert_file_has_content refscount "^2$"
 
 # Create a local mirror repository where we pull the branches *in mirror mode* from the two remotes.
 # This should pull them into refs/mirrors, since the remotes advertise a collection ID.
diff --git a/tests/test-refs-collections.sh b/tests/test-refs-collections.sh
index 4d49247d..f36fd7b9 100755
--- a/tests/test-refs-collections.sh
+++ b/tests/test-refs-collections.sh
@@ -103,6 +103,28 @@ ${CMD_PREFIX} ostree --repo=repo refs --collections > refs
 assert_file_has_content refs "^(org.example.Collection, ctest)$"
 assert_file_has_content refs "^(org.example.NewCollection, ctest-mirror)$"
 
+# Remote refs should be listed if they have collection IDs
+
+cd ${test_tmpdir}
+mkdir collection-repo
+ostree_repo_init collection-repo --collection-id org.example.RemoteCollection
+mkdir -p adir
+${CMD_PREFIX} ostree --repo=collection-repo commit --branch=rcommit -m rcommit -s rcommit adir
+${CMD_PREFIX} ostree --repo=repo remote add --no-gpg-verify --collection-id org.example.RemoteCollection collection-repo-remote "file://${test_tmpdir}/collection-repo"
+${CMD_PREFIX} ostree --repo=repo pull collection-repo-remote rcommit
+${CMD_PREFIX} ostree --repo=repo refs --collections > refs
+assert_file_has_content refs "^(org.example.RemoteCollection, rcommit)$"
+
+cd ${test_tmpdir}
+mkdir no-collection-repo
+ostree_repo_init no-collection-repo
+mkdir -p adir2
+${CMD_PREFIX} ostree --repo=no-collection-repo commit --branch=rcommit2 -m rcommit2 -s rcommit2 adir2
+${CMD_PREFIX} ostree --repo=repo remote add --no-gpg-verify no-collection-repo-remote "file://${test_tmpdir}/no-collection-repo"
+${CMD_PREFIX} ostree --repo=repo pull no-collection-repo-remote rcommit2
+${CMD_PREFIX} ostree --repo=repo refs --collections > refs
+assert_not_file_has_content refs "rcommit2"
+
 echo "ok 1 refs collections"
 
 # Test that listing, creating and deleting refs works from an old repository
diff --git a/tests/test-summary-collections.sh b/tests/test-summary-collections.sh
index 989e63e8..d12100ba 100755
--- a/tests/test-summary-collections.sh
+++ b/tests/test-summary-collections.sh
@@ -55,4 +55,29 @@ ${CMD_PREFIX} ostree --repo=repo summary --update
 ${CMD_PREFIX} ostree --repo=repo summary --view > summary
 assert_file_has_content summary "(org.example.OtherCollection, test-1-mirror)$"
 
+# Test that remote refs are listed, but only if they have collection IDs
+cd ${test_tmpdir}
+mkdir collection-repo
+ostree_repo_init collection-repo --collection-id org.example.RemoteCollection
+mkdir -p adir
+${CMD_PREFIX} ostree --repo=collection-repo commit --branch=rcommit -m rcommit -s rcommit adir
+${CMD_PREFIX} ostree --repo=repo remote add --no-gpg-verify --collection-id org.example.RemoteCollection collection-repo-remote "file://${test_tmpdir}/collection-repo"
+${CMD_PREFIX} ostree --repo=repo pull collection-repo-remote rcommit
+${CMD_PREFIX} ostree --repo=repo summary --update
+
+${CMD_PREFIX} ostree --repo=repo summary --view > summary
+assert_file_has_content summary "(org.example.RemoteCollection, rcommit)$"
+
+cd ${test_tmpdir}
+mkdir no-collection-repo
+ostree_repo_init no-collection-repo
+mkdir -p adir2
+${CMD_PREFIX} ostree --repo=no-collection-repo commit --branch=rcommit2 -m rcommit2 -s rcommit2 adir2
+${CMD_PREFIX} ostree --repo=repo remote add --no-gpg-verify no-collection-repo-remote "file://${test_tmpdir}/no-collection-repo"
+${CMD_PREFIX} ostree --repo=repo pull no-collection-repo-remote rcommit2
+${CMD_PREFIX} ostree --repo=repo summary --update
+
+${CMD_PREFIX} ostree --repo=repo summary --view > summary
+assert_not_file_has_content summary "rcommit2"
+
 echo "ok summary collections"