git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 2/3] fast-import: allow "merge $null_sha1" command

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Jun 27, 2012, 23:39 UTC
Message-ID
<20120627233931.GA3014@burratino>
In-Reply-To
<7v395g75gg.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Dmitry Ivankov <divanorama@gmail.com> writes:
Show 5 quoted lines
>> "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

Previous: Junio C HamanoNext: Jonathan Nieder
Message 8 of 12 in “fast-import: disallow empty branches as parents”
  1. 0/3 fast-import: disallow empty branches as parentsDmitry Ivankov, Jun 27, 2012
  2. 1/3 fast-import: do not write null_sha1 as a merge parentDmitry Ivankov, Jun 27, 2012
  3. Jonathan NiederJun 27, 2012
  4. Jonathan NiederJul 24, 2012
  5. 2/3 fast-import: allow "merge $null_sha1" commandDmitry Ivankov, Jun 27, 2012
  6. Jonathan NiederJun 27, 2012
  7. Junio C HamanoJun 27, 2012
  8. Jonathan NiederJun 27, 2012
  9. Jonathan NiederJul 23, 2012
  10. 3/3 fast-import: disallow "merge $itself" commandDmitry Ivankov, Jun 27, 2012
  11. Jonathan NiederJun 27, 2012
  12. Jonathan NiederJul 24, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.