git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>,
	gitster@pobox.com, avarab@gmail.com
Subject: [PATCH v2] fetch,fetch-pack: clarify connectivity check error
Date: Fri, 10 Jun 2022 12:52:47 -0700	[thread overview]
Message-ID: <20220610195247.1177549-1-jonathantanmy@google.com> (raw)
In-Reply-To: <20220608210537.185094-1-jonathantanmy@google.com>

When the connectivity check after a fetch fails, an error message
"<remote> did not send all necessary objects" is printed. That error
message is printed regardless of the reason of failure: in particular,
that message may be printed if the connectivity check fails because a
local object is missing. (The connectivity check reads local objects too
because it compares the set of objects that the remote claims to send
against the set of objects that our refs directly or indirectly
reference.)

Since it is not necessarily true that the remote did not send all
necessary objects, update the error message to be something that more
correctly reflects the situation.

The updated error message is admittedly vague and one alternative
solution would be to update the revision walking code to be able to more
precisely specify if a bad object is supposed to be available locally or
supposed to have been sent by the remote. That would likely require, in
the revision walking code, to delay parsing any object until all its
children has been parsed, which seems to be a significant undertaking.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
As Junio said in [1], my original v1 code doesn't work when an object is
referenced both by a local object and a remote one. I tried looking into
making it work but it looks difficult.

So here is a patch that just changes the error message to a vaguer but
hopefully more correct one. I wonder what the community thinks of it -
I think it is more correct (and means that we do not need to say "it's
not the remote fault despite what the error message says") but at the
same time, many people are already used to this message (and a search
online reveals several web sites that talk about this).

[1] https://lore.kernel.org/git/xmqqh74upyz3.fsf@gitster.g/
---
 builtin/fetch.c | 2 +-
 fetch-pack.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index ac29c2b1ae..15737ca293 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1133,7 +1133,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 
 		rm = ref_map;
 		if (check_connected(iterate_ref_map, &rm, &opt)) {
-			rc = error(_("%s did not send all necessary objects\n"), url);
+			rc = error(_("connectivity check failed after fetching from %s\n"), url);
 			goto abort;
 		}
 	}
diff --git a/fetch-pack.c b/fetch-pack.c
index cb6647d657..91269008cc 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -2021,7 +2021,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
 		if (args->deepen)
 			opt.is_deepening_fetch = 1;
 		if (check_connected(iterate_ref_map, &iterator, &opt)) {
-			error(_("remote did not send all necessary objects"));
+			error(_("connectivity check failed after fetching from remote"));
 			free_refs(ref_cpy);
 			ref_cpy = NULL;
 			rollback_shallow_file(the_repository, &shallow_lock);
-- 
2.36.1.476.g0c4daa206d-goog


  parent reply	other threads:[~2022-06-10 19:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-08 21:05 [PATCH] connected: distinguish local/remote bad objects Jonathan Tan
2022-06-08 22:33 ` Junio C Hamano
2022-06-09 17:17   ` Jonathan Tan
2022-06-09 16:55 ` Junio C Hamano
2022-06-09 17:17   ` Jonathan Tan
2022-06-09 18:00   ` Ævar Arnfjörð Bjarmason
2022-06-10 19:52 ` Jonathan Tan [this message]
2022-06-10 20:25   ` [PATCH v2] fetch,fetch-pack: clarify connectivity check error Junio C Hamano
2022-06-17 20:03     ` Jonathan Tan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220610195247.1177549-1-jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).