From 23304b8c15835e75e257c7eb589808e40f3e39c9 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Wed, 20 Feb 2019 13:18:31 -0800 Subject: [PATCH] lib/repo-refs: Fix resolving collection-refs My last commit "lib/repo-refs: Resolve collection-refs in-memory and in parent repos" changed ostree_repo_resolve_collection_ref() to check the in-memory set of refs *after* failing to find the ref on disk but that's not what we want. We want to use the in-memory set of refs first, because those are the most up to date commits, and then fall back to the on-disk repo and finally fall back to checking any parent repo. This commit makes such a change to the order of operations, which is consistent with how ostree_repo_resolve_rev() works. Aside from this change being logical, it also fixes some unit test failures on an unmerged branch of flatpak: https://github.com/flatpak/flatpak/pull/2705 Also, tweak the comments here. Closes: #1825 Approved by: jlebon --- src/libostree/ostree-repo-refs.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/libostree/ostree-repo-refs.c b/src/libostree/ostree-repo-refs.c index ee11eb53..be2eb59a 100644 --- a/src/libostree/ostree-repo-refs.c +++ b/src/libostree/ostree-repo-refs.c @@ -525,23 +525,17 @@ ostree_repo_resolve_collection_ref (OstreeRepo *self, GCancellable *cancellable, GError **error) { + g_autofree char *ret_contents = NULL; + g_return_val_if_fail (OSTREE_IS_REPO (self), FALSE); g_return_val_if_fail (ref != NULL, FALSE); g_return_val_if_fail (ref->collection_id != NULL && ref->ref_name != NULL, FALSE); g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), FALSE); g_return_val_if_fail (error == NULL || *error == NULL, FALSE); - g_autoptr(GHashTable) refs = NULL; /* (element-type OstreeCollectionRef utf8) */ - if (!ostree_repo_list_collection_refs (self, ref->collection_id, &refs, - OSTREE_REPO_LIST_REFS_EXT_NONE, - cancellable, error)) - return FALSE; - - g_autofree char *ret_contents = g_strdup (g_hash_table_lookup (refs, ref)); - - /* Check for refs in the current transaction that haven't been written to - * disk, to match the behavior of ostree_repo_resolve_rev() */ - if (ret_contents == NULL && self->in_transaction) + /* Check for the ref in the current transaction in case it hasn't been + * written to disk, to match the behavior of ostree_repo_resolve_rev() */ + if (self->in_transaction) { g_mutex_lock (&self->txn_lock); if (self->txn.collection_refs) @@ -549,7 +543,19 @@ ostree_repo_resolve_collection_ref (OstreeRepo *self, g_mutex_unlock (&self->txn_lock); } - /* Check for refs in the parent repo */ + /* Check for the ref on disk in the repo */ + if (ret_contents == NULL) + { + g_autoptr(GHashTable) refs = NULL; /* (element-type OstreeCollectionRef utf8) */ + if (!ostree_repo_list_collection_refs (self, ref->collection_id, &refs, + OSTREE_REPO_LIST_REFS_EXT_NONE, + cancellable, error)) + return FALSE; + + ret_contents = g_strdup (g_hash_table_lookup (refs, ref)); + } + + /* Check for the ref in the parent repo */ if (ret_contents == NULL && self->parent_repo != NULL) { if (!ostree_repo_resolve_collection_ref (self->parent_repo,