All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Dmitry Ivankov <divanorama@gmail.com>,
	git@vger.kernel.org, Jeff King <peff@peff.net>,
	Sverre Rabbelier <srabbelier@gmail.com>,
	Shawn Pearce <spearce@spearce.org>
Subject: Re: [PATCH v2 2/3] fast-import: allow "merge $null_sha1" command
Date: Wed, 27 Jun 2012 18:39:31 -0500	[thread overview]
Message-ID: <20120627233931.GA3014@burratino> (raw)
In-Reply-To: <7v395g75gg.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Dmitry Ivankov <divanorama@gmail.com> writes:

>> "from $null_sha1" and "merge $empty_branch" are already allowed so
>> allow "merge $null_sha1" command too.
>
> Would accepting such a "merge oops-do-not-do-anything" allow
> exporters' job to be simpler?

Good question.

I was uncomfortable with the patch and couldn't pin down why and I
think you've hit it.

I can imagine an importer that does

	cat <<EOF
	commit refs/heads/master
	from $parent
	merge $second_parent
[etc]
	EOF

and uses parent=0000000000000000000000000000000000000000 in the
degenerate case, but it is not hard to use

	cat <<EOF
	commit refs/heads/master
	$optional_from_line$optional_second_parent
[etc]
	EOF

so this is not a very strong justification.  Mostly it felt like a
step in the right direction because once you can do it for "from",
someone might try it with "merge" and it's simplest to explain the
syntax if we're consistent.

On the other side to be weighed against that is the danger that
someone might actually start using "merge" this way.  They would be
making their frontend break compatibility with old versions of git
fast-import for no good reason.

So on second thought, it does not seem like a good direction at all.
[Though the cleanup I mentioned might be nice in any case. ;-)]

I wonder if anyone using "from" with a branch name that resolves in
the internal branch table to $null_sha1 was actually intending that.
Would any importers break if we started to forbid it?  Would it make
sense to add that check in "next" for a release or two and see if
anyone complains?

Looking at the patch for 00e2b884 (Remove branch creation command from
fast-import, 2006-08-24), it looks like support for "from $null_sha1"
was intentional.  Maybe mailing list discussions from around then have
insight.

Thanks for some food for thought,
Jonathan

  reply	other threads:[~2012-06-27 23:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-27 17:40 [PATCH/RFC v2 0/3] fast-import: disallow empty branches as parents Dmitry Ivankov
2012-06-27 17:40 ` [PATCH v2 1/3] fast-import: do not write null_sha1 as a merge parent Dmitry Ivankov
2012-06-27 21:25   ` Jonathan Nieder
2012-07-24 19:30   ` Jonathan Nieder
2012-06-27 17:40 ` [PATCH v2 2/3] fast-import: allow "merge $null_sha1" command Dmitry Ivankov
2012-06-27 21:33   ` Jonathan Nieder
2012-06-27 22:30   ` Junio C Hamano
2012-06-27 23:39     ` Jonathan Nieder [this message]
2012-07-23  1:28       ` Jonathan Nieder
2012-06-27 17:40 ` [PATCH v2 3/3] fast-import: disallow "merge $itself" command Dmitry Ivankov
2012-06-27 21:22   ` Jonathan Nieder
2012-07-24 19:40   ` Jonathan Nieder

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=20120627233931.GA3014@burratino \
    --to=jrnieder@gmail.com \
    --cc=divanorama@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=spearce@spearce.org \
    --cc=srabbelier@gmail.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.