git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Davide Berardi <berardi.dav@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] Segmentation Fault on non-commit --branch clone
Date: Sat, 02 Nov 2019 19:16:23 +0900	[thread overview]
Message-ID: <xmqqh83m1t3c.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20191101193558.GA1169@sigill.intra.peff.net> (Jeff King's message of "Fri, 1 Nov 2019 15:35:58 -0400")

Jeff King <peff@peff.net> writes:

> I don't know how often this would actually help users, though. It _is_ a
> pretty rare situation to ask for a non-commit. So maybe it's all
> over-engineering, and we should start with just die(). If somebody comes
> along later and wants to enhance it, it should be pretty
> straightforward.

I like that; after update_head() finishes, there are a few clean-up
things that the caller wants to do besides a checkout() call, but if
we make update_head() return a failure, perhaps the caller side
change would be as small as the attached patch.  That would go nicely
with the "make the result just barely usable" approach of leaving an
unborn master branch I suggested in a separate message, I would think.

 builtin/clone.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index c46ee29f0a..fa0558fa3e 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1246,7 +1246,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 			   branch_top.buf, reflog_msg.buf, transport,
 			   !is_local, filter_options.choice);
 
-	update_head(our_head_points_at, remote_head, reflog_msg.buf);
+	err = update_head(our_head_points_at, remote_head, reflog_msg.buf) < 0;
 
 	/*
 	 * We want to show progress for recursive submodule clones iff
@@ -1265,8 +1265,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	}
 
 	junk_mode = JUNK_LEAVE_REPO;
-	fetch_if_missing = 1;
-	err = checkout(submodule_progress);
+	if (!err) {
+		fetch_if_missing = 1;
+		err = checkout(submodule_progress);
+	}
 
 	strbuf_release(&reflog_msg);
 	strbuf_release(&branch_top);




  reply	other threads:[~2019-11-02 10:16 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-01  0:24 Davide Berardi
2019-11-01 19:08 ` Johannes Schindelin
2019-11-01 19:35   ` Jeff King
2019-11-02 10:16     ` Junio C Hamano [this message]
2019-11-03 18:16       ` Davide Berardi
2019-11-04  3:55         ` Junio C Hamano
2019-11-02  9:18   ` Junio C Hamano
2019-11-01 19:43 ` Jeff King
2019-11-02 10:07 ` Junio C Hamano
2019-11-03 18:07 ` [PATCH v2] clone: Don't segfault on -b specifing a non-commit Davide Berardi
2019-11-05  4:37   ` Jeff King
2019-11-06  1:36     ` Junio C Hamano
2019-11-06  4:05       ` Jeff King
2019-11-06  9:53         ` Junio C Hamano

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

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

  git send-email \
    --in-reply-to=xmqqh83m1t3c.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=berardi.dav@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --subject='Re: [PATCH] Segmentation Fault on non-commit --branch clone' \
    /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

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 NNTP newsgroup(s).