From 1823d5996412e4b1627047ff8a30ca70a9f680a6 Mon Sep 17 00:00:00 2001 From: "Stolee, Derrick" Date: Thu, 15 Feb 2018 14:30:03 +0000 Subject: [PATCH] fixup! fetch: avoid loose object downloads in GVFS This reverts the changes from that commit. The reasons are: 1. There is a bug when a ref points to a tag that points to a non-commit. It then does not set the merge type correctly. 2. There are a lot of special code checks intended only to reduce the number of tree downloads (we do not eliminate commit loads because the ref update machinery will download those commits). 3. We should do the _correct_ thing that we were already planning to do as part of the commit graph work: let's remove the requirement that lookup_commit() loads the root tree. It requires finding all places where the commit->tree value is used and use a method to verify it works. --- builtin/fetch.c | 84 +++++++++++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 44 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 55ab52da3e2a6e..1e22952c8f3cf9 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -626,14 +626,14 @@ static int update_local_ref(struct ref *ref, struct strbuf *display, int summary_width) { - struct commit *current = NULL, *updated = NULL; + struct commit *current = NULL, *updated; + enum object_type type; struct branch *current_branch = branch_get(NULL); const char *pretty_ref = prettify_refname(ref->name); - if (!core_gvfs || fetch_show_forced_updates) { - if (sha1_object_info(ref->new_oid.hash, NULL) < 0) - die(_("object %s not found"), oid_to_hex(&ref->new_oid)); - } + type = sha1_object_info(ref->new_oid.hash, NULL); + if (type < 0) + die(_("object %s not found"), oid_to_hex(&ref->new_oid)); if (!oidcmp(&ref->old_oid, &ref->new_oid)) { if (verbosity > 0) @@ -666,39 +666,37 @@ static int update_local_ref(struct ref *ref, return r; } - if (!core_gvfs || fetch_show_forced_updates) { - current = lookup_commit_reference_gently(&ref->old_oid, 1); - updated = lookup_commit_reference_gently(&ref->new_oid, 1); - if (!current || !updated) { - const char *msg; - const char *what; - int r; - /* - * Nicely describe the new ref we're fetching. - * Base this on the remote's ref name, as it's - * more likely to follow a standard layout. - */ - const char *name = remote_ref ? remote_ref->name : ""; - if (starts_with(name, "refs/tags/")) { - msg = "storing tag"; - what = _("[new tag]"); - } else if (starts_with(name, "refs/heads/")) { - msg = "storing head"; - what = _("[new branch]"); - } else { - msg = "storing ref"; - what = _("[new ref]"); - } - - if ((recurse_submodules != RECURSE_SUBMODULES_OFF) && - (recurse_submodules != RECURSE_SUBMODULES_ON)) - check_for_new_submodule_commits(&ref->new_oid); - r = s_update_ref(msg, ref, 0); - format_display(display, r ? '!' : '*', what, - r ? _("unable to update local ref") : NULL, - remote, pretty_ref, summary_width); - return r; + current = lookup_commit_reference_gently(&ref->old_oid, 1); + updated = lookup_commit_reference_gently(&ref->new_oid, 1); + if (!current || !updated) { + const char *msg; + const char *what; + int r; + /* + * Nicely describe the new ref we're fetching. + * Base this on the remote's ref name, as it's + * more likely to follow a standard layout. + */ + const char *name = remote_ref ? remote_ref->name : ""; + if (starts_with(name, "refs/tags/")) { + msg = "storing tag"; + what = _("[new tag]"); + } else if (starts_with(name, "refs/heads/")) { + msg = "storing head"; + what = _("[new branch]"); + } else { + msg = "storing ref"; + what = _("[new ref]"); } + + if ((recurse_submodules != RECURSE_SUBMODULES_OFF) && + (recurse_submodules != RECURSE_SUBMODULES_ON)) + check_for_new_submodule_commits(&ref->new_oid); + r = s_update_ref(msg, ref, 0); + format_display(display, r ? '!' : '*', what, + r ? _("unable to update local ref") : NULL, + remote, pretty_ref, summary_width); + return r; } if (!fetch_show_forced_updates || in_merge_bases(current, updated)) { @@ -713,7 +711,7 @@ static int update_local_ref(struct ref *ref, warning(_(" git rev-list --left-right --count ...@{1}")); } - strbuf_add_unique_abbrev(&quickref, ref->old_oid.hash, DEFAULT_ABBREV); + strbuf_add_unique_abbrev(&quickref, current->object.oid.hash, DEFAULT_ABBREV); strbuf_addstr(&quickref, ".."); strbuf_add_unique_abbrev(&quickref, ref->new_oid.hash, DEFAULT_ABBREV); if ((recurse_submodules != RECURSE_SUBMODULES_OFF) && @@ -811,12 +809,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, continue; } - if (!core_gvfs) { - commit = lookup_commit_reference_gently(&rm->old_oid, - 1); - if (!commit) - rm->fetch_head_status = FETCH_HEAD_NOT_FOR_MERGE; - } + commit = lookup_commit_reference_gently(&rm->old_oid, + 1); + if (!commit) + rm->fetch_head_status = FETCH_HEAD_NOT_FOR_MERGE; if (rm->fetch_head_status != want_status) continue;