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

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);

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,
+gboolean      ostree_repo_list_collection_refs (OstreeRepo                  *self,
-                                                const char    *match_collection_id,
+                                                const char                  *match_collection_id,
-                                                GHashTable   **out_all_refs,
+                                                GHashTable                 **out_all_refs,
-                                                GCancellable  *cancellable,
+                                                OstreeRepoListRefsExtFlags   flags,
-                                                GError       **error);
+                                                GCancellable                *cancellable,
+                                                GError                     **error);
 
 void          ostree_repo_transaction_set_collection_ref (OstreeRepo                *self,
                                                           const OstreeCollectionRef *ref,

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)

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 (!(flags & OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES))
-
-      if (!glnx_dirfd_iterator_init_at (self->repo_dir_fd, "refs/remotes", TRUE, &dfd_iter, error))
-        return FALSE;
-
-      while (TRUE)
         {
-          struct dirent *dent;
+          g_string_truncate (base_path, 0);
-          glnx_fd_close int remote_dfd = -1;
 
-          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;
-
-          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,
+          while (TRUE)
-                                       remote_dfd, ".",
+            {
-                                       ret_all_refs,
+              struct dirent *dent;
-                                       cancellable, error))
+              glnx_fd_close int remote_dfd = -1;
-            return FALSE;
+
+              if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent, cancellable, error))
+                return FALSE;
+              if (!dent)
+                break;
+
+              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
+ * List all local, mirrored, and remote refs, mapping them to the commit
- * currently point to in @out_all_refs. If @match_collection_id is specified,
+ * checksums they currently point to in @out_all_refs. If @match_collection_id
- * the results will be limited to those with an equal 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,
+ostree_repo_list_collection_refs (OstreeRepo                 *self,
-                                  const char    *match_collection_id,
+                                  const char                 *match_collection_id,
-                                  GHashTable   **out_all_refs,
+                                  GHashTable                 **out_all_refs,
-                                  GCancellable  *cancellable,
+                                  OstreeRepoListRefsExtFlags flags,
-                                  GError       **error)
+                                  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;
+  for (const char **iter = refs_dirs; iter && *iter; iter++)
-  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)
     {
-      struct dirent *dent;
+      const char *refs_dir = *iter;
-      glnx_fd_close int collection_dfd = -1;
+      gboolean refs_dir_exists = FALSE;
-
+      if (!ot_dfd_iter_init_allow_noent (self->repo_dir_fd, refs_dir,
-      if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent, cancellable, error))
+                                         &dfd_iter, &refs_dir_exists, error))
-        return FALSE;
-      if (!dent)
-        break;
-
-      if (dent->d_type != DT_DIR)
-        continue;
-
-      if (match_collection_id != NULL && g_strcmp0 (match_collection_id, dent->d_name) != 0)
-        continue;
-
-      if (!glnx_opendirat (dfd_iter.fd, dent->d_name, TRUE, &collection_dfd, error))
         return FALSE;
 
-      if (!enumerate_refs_recurse (self, NULL, OSTREE_REPO_LIST_REFS_EXT_NONE,
+      while (refs_dir_exists)
-                                   dent->d_name, collection_dfd, base_path,
+        {
-                                   collection_dfd, ".",
+          struct dirent *dent;
-                                   ret_all_refs,
+          glnx_fd_close int subdir_fd = -1;
-                                   cancellable, error))
+          const gchar *current_collection_id;
-        return FALSE;
+          g_autofree gchar *remote_collection_id = NULL;
+
+          if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent, cancellable, error))
+            return FALSE;
+          if (!dent)
+            break;
+
+          if (dent->d_type != DT_DIR)
+            continue;
+
+          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);

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;

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,
+gboolean ostree_repo_list_collection_refs (OstreeRepo                 *self,
-                                           const char    *match_collection_id,
+                                           const char                 *match_collection_id,
-                                           GHashTable   **out_all_refs,
+                                           GHashTable                 **out_all_refs,
-                                           GCancellable  *cancellable,
+                                           OstreeRepoListRefsExtFlags flags,
-                                           GError       **error);
+                                           GCancellable               *cancellable,
+                                           GError                     **error);
 
 #endif /* OSTREE_ENABLE_EXPERIMENTAL_API */
 

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;
 

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);

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)

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
+${CMD_PREFIX} ostree --repo=local refs --collections > refs
-assert_file_has_content refscount "^0$"
+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.

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

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"