git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Davide Berardi <berardi.dav@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	git@vger.kernel.org
Subject: Re: [PATCH] Segmentation Fault on non-commit --branch clone
Date: Sun, 3 Nov 2019 19:16:57 +0100	[thread overview]
Message-ID: <20191103181657.GA89185@carpenter.lan> (raw)
In-Reply-To: <xmqqh83m1t3c.fsf@gitster-ct.c.googlers.com>

On Sat, Nov 02, 2019 at 07:16:23PM +0900, Junio C Hamano wrote:
>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.
>
Thank you all for your precious comments, I've tried to implement
your suggestions and I've sent the patch here.

The problem with the proposed approach was that the code was
incompatible with some tests (specifically the tests which specifies an
empty .git directory would fail and fallback to the unborn master
branch).

The lookup commit have two error-paths:

1. the commit cannot be found;
2. the commit is found and cannot be casted to a commit (whoops!).

so, I've returned the second condition using an auxiliary variable and
declaring a new lookup_commit function keeping compatibility with the
old one.

I'm sorry for my errors but I'm far for an expert of git internals,
thank you (all) for your time and kindness.

ciao,
D.

  reply	other threads:[~2019-11-03 18:17 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-01  0:24 [PATCH] Segmentation Fault on non-commit --branch clone Davide Berardi
2019-11-01 19:08 ` Johannes Schindelin
2019-11-01 19:35   ` Jeff King
2019-11-02 10:16     ` Junio C Hamano
2019-11-03 18:16       ` Davide Berardi [this message]
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

  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=20191103181657.GA89185@carpenter.lan \
    --to=berardi.dav@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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).