lib/repo-pull: Fix counting of latest commits when finding repos

The intended behaviour of ostree_repo_find_remotes() is to return
results which have the latest version of at least one of the requested
refs. Results which have some of the requested refs, but don’t have the
latest version of any of them, should be ignored. The logic to do this
was broken in the case that a result contained a positive number of the
requested refs, but none of them were the latest version. (It previously
worked when the result contained none of the requested refs.)

Fix the counting to work correctly in both cases.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1058
Approved by: cgwalters

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index fd4be4995e8811a8081a0fb28f3ab1d69045a3a7..ed616a81a4d442c2db27222efde78016bf5053be 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -5077,7 +5077,7 @@ find_remotes_cb (GObject      *obj,
     {
       OstreeRepoFinderResult *result = g_ptr_array_index (results, i);
       g_autoptr(GHashTable) validated_ref_to_checksum = NULL;  /* (element-type utf8 utf8) */
-      gsize j;
+      gsize j, n_latest_refs;
 
       /* Previous error processing this result? */
       if (result == NULL)
@@ -5089,6 +5089,7 @@ find_remotes_cb (GObject      *obj,
                                                          ostree_collection_ref_equal,
                                                          (GDestroyNotify) ostree_collection_ref_free,
                                                          g_free);
+      n_latest_refs = 0;
 
       for (j = 0; refs[j] != NULL; j++)
         {
@@ -5096,11 +5097,13 @@ find_remotes_cb (GObject      *obj,
 
           if (pointer_table_get (refs_and_remotes_table, j, i) != latest_commit_for_ref)
             latest_commit_for_ref = NULL;
+          if (latest_commit_for_ref != NULL)
+            n_latest_refs++;
 
           g_hash_table_insert (validated_ref_to_checksum, ostree_collection_ref_dup (refs[j]), g_strdup (latest_commit_for_ref));
         }
 
-      if (g_hash_table_size (validated_ref_to_checksum) == 0)
+      if (n_latest_refs == 0)
         {
           g_debug ("%s: Omitting remote ‘%s’ from results as none of its refs are new enough.",
                    G_STRFUNC, result->remote->name);

diff --git a/tests/test-find-remotes.sh b/tests/test-find-remotes.sh
index 0b887664439764a1a40caec66fc2d3558bcc00c7..0cf0127fa9cb9b373a446d23321f66479e2e83d0 100755 (executable)
--- a/tests/test-find-remotes.sh
+++ b/tests/test-find-remotes.sh
@@ -71,11 +71,16 @@ ${CMD_PREFIX} ostree --repo=local-mirror pull --mirror os-remote os/amd64/master
 
 ${CMD_PREFIX} ostree --repo=local-mirror refs | wc -l > refscount
 assert_file_has_content refscount "^0$"
+ls -1 local-mirror/refs/remotes | wc -l > remotescount
+assert_file_has_content remotescount "^0$"
 
 ${CMD_PREFIX} ostree --repo=local-mirror refs --collections > refs
 assert_file_has_content refs "^(org.example.AppsCollection, app1)$"
 assert_file_has_content refs "^(org.example.OsCollection, os/amd64/master)$"
 
+assert_file_has_content local-mirror/refs/mirrors/org.example.AppsCollection/app1 "^$(cat app1-checksum)$"
+assert_file_has_content local-mirror/refs/mirrors/org.example.OsCollection/os/amd64/master "^$(cat os-checksum)$"
+
 for repo in local local-mirror; do
     # Try finding an update for an existing branch.
     ${CMD_PREFIX} ostree --repo=$repo find-remotes org.example.AppsCollection app1 > find
@@ -169,14 +174,16 @@ for repo in local-mirror; do
     assert_file_has_content pull "^Pulled 1/1 refs successfully.$"
     assert_not_file_has_content pull "Failed to pull some refs from the remotes"
     assert_ref $repo os/amd64/master $(cat os-checksum-2)
+
+    # We need to manually update the refs afterwards, since the original pull
+    # into the local-mirror was a --mirror pull — so it wrote refs/mirrors/blah.
+    # This pull was not, so it wrote refs/remotes/blah.
+    ${CMD_PREFIX} ostree --repo=$repo refs --collections --create org.example.OsCollection:os/amd64/master os-remote:os/amd64/master
 done
 
 # Add the local mirror to the local repository as a remote, so that the local repo
 # has two configured remotes for the os-collection. Ensure its summary is up to date first.
-#${CMD_PREFIX} ostree --repo=local-mirror summary --update
-# FIXME: This `cp` can be changed to `ostree summary --update` once PR #946 lands.
-# Prior to that, we need to preserve the signatures.
-cp os-collection/summary{,.sig} local-mirror/
+${CMD_PREFIX} ostree --repo=local-mirror summary --update
 ${CMD_PREFIX} ostree --repo=local remote add os-remote-local-mirror file://$(pwd)/local-mirror --collection-id org.example.OsCollection --gpg-import=${test_tmpdir}/gpghome/key2.asc
 
 for repo in local; do
@@ -212,9 +219,6 @@ for repo in local; do
     assert_file_has_content find "^Result [0-9]\+: file://$(pwd)/os-collection$"
     assert_file_has_content find "^ - Keyring: os-remote.trustedkeys.gpg$"
     assert_file_has_content find "^    - (org.example.OsCollection, os/amd64/master) = $(cat os-checksum-3)$"
-    assert_file_has_content find "^Result [0-9]\+: file://$(pwd)/local-mirror$"
-    assert_file_has_content find "^ - Keyring: os-remote-local-mirror.trustedkeys.gpg$"
-    assert_file_has_content find "^    - (org.example.OsCollection, os/amd64/master) = $(cat os-checksum-3)$"
     assert_file_has_content find "^1/1 refs were found.$"
     assert_not_file_has_content find "^No results.$"