From be4832242da27887399eb0c0bb7be1378d858b14 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 7 Aug 2017 13:37:57 +0100 Subject: [PATCH] lib/repo-pull: Fix counting of latest commits when finding repos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Closes: #1058 Approved by: cgwalters --- src/libostree/ostree-repo-pull.c | 7 +++++-- tests/test-find-remotes.sh | 18 +++++++++++------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index fd4be499..ed616a81 100644 --- 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 0b887664..0cf0127f 100755 --- 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.$"