git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Mike Hommey <mh@glandium.org>
To: git@vger.kernel.org
Cc: gitster@pobox.com, Mike Hommey <mh@glandium.org>
Subject: [PATCH] Don't ignore transport_disconnect error codes in fetch and clone
Date: Tue, 28 Sep 2021 09:17:26 +0900	[thread overview]
Message-ID: <20210928001726.2592734-1-mh@glandium.org> (raw)

When a remote-helper fails in a way that is not directly visible in the
remote-helper protocol, the helper failure is ignored by git during
fetch or clone.

For example, a helper cannot directly report an error during an `import`
command (short of sending `feature done` to the fast-import file
descriptor and not sending a `done` later on).

Or if the helper crashes at the wrong moment, git doesn't notice and
thinks everything went well.

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 builtin/clone.c | 5 +++--
 builtin/fetch.c | 6 +++---
 2 files changed, 6 insertions(+), 5 deletions(-)

What I'm not sure about is whether a message should be explicitly
printed by git itself in those cases.

diff --git a/builtin/clone.c b/builtin/clone.c
index 66fe66679c..f26fa027c5 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1398,7 +1398,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	submodule_progress = transport->progress;
 
 	transport_unlock_pack(transport);
-	transport_disconnect(transport);
+	err = transport_disconnect(transport);
 
 	if (option_dissociate) {
 		close_object_store(the_repository->objects);
@@ -1406,7 +1406,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	}
 
 	junk_mode = JUNK_LEAVE_REPO;
-	err = checkout(submodule_progress);
+	if (!err)
+		err = checkout(submodule_progress);
 
 	free(remote_name);
 	strbuf_release(&reflog_msg);
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 25740c13df..66bccf6f50 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1886,7 +1886,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv,
 {
 	struct refspec rs = REFSPEC_INIT_FETCH;
 	int i;
-	int exit_code;
+	int exit_code, disconnect_code;
 	int maybe_prune_tags;
 	int remote_via_config = remote_is_configured(remote, 0);
 
@@ -1952,9 +1952,9 @@ static int fetch_one(struct remote *remote, int argc, const char **argv,
 	exit_code = do_fetch(gtransport, &rs);
 	sigchain_pop(SIGPIPE);
 	refspec_clear(&rs);
-	transport_disconnect(gtransport);
+	disconnect_code = transport_disconnect(gtransport);
 	gtransport = NULL;
-	return exit_code;
+	return exit_code || disconnect_code;
 }
 
 int cmd_fetch(int argc, const char **argv, const char *prefix)
-- 
2.33.0


             reply	other threads:[~2021-09-28  0:45 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-28  0:17 Mike Hommey [this message]
2021-09-28  2:56 ` [PATCH] Don't ignore transport_disconnect error codes in fetch and clone Ævar Arnfjörð Bjarmason
2021-10-05  4:54   ` Mike Hommey
2021-10-05 12:21     ` Ævar Arnfjörð Bjarmason

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=20210928001726.2592734-1-mh@glandium.org \
    --to=mh@glandium.org \
    --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).