All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: "Jonathan Nieder" <jrnieder@gmail.com>,
	"Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>,
	"Randal L. Schwartz" <merlyn@stonehenge.com>,
	"Ralf Nyren" <ralf.nyren@ericsson.com>,
	git@vger.kernel.org
Subject: Re: [PATCH 0/2] merging renames of empty files
Date: Thu, 22 Mar 2012 16:37:05 -0700	[thread overview]
Message-ID: <7vpqc4us0u.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20120322224651.GA14874@sigill.intra.peff.net> (Jeff King's message of "Thu, 22 Mar 2012 18:46:51 -0400")

Jeff King <peff@peff.net> writes:

> Here's a 2-patch series to replace the old 3/3 (they go on top of the
> first two cleanups from the previous iteration).

Hrm. As this is probably useful for older maintenance track, I would have
preferred not to see the first one that touches sequencer.c that did not
exist in the 1.7.7.x maintenance track, as the change is purely "cosmetic"
and does not have anything to do with fixing the over-agressive merge.

>   [1/2]: teach diffcore-rename to optionally ignore empty content
>   [2/2]: merge-recursive: don't detect renames of empty files
>
> Thinking on this more, it is actually a more generic problem than just
> empty files. It is really a problem of having generic placeholder files
> with the same content. So a fully general solution would be something
> like a gitattribute for "don't do renames on this". However, in
> practice, these placeholder files are empty (since any non-empty file is
> likely to actually have different content). So I think just dropping the
> empty files as rename candidates is a pretty good heuristic, and it's
> nice and simple.

I thought that our recommendation for keeping an otherwise empty
directories around was to have .gitignore file with two entries in it,
namely:

	.*
        *

So these files will be everywhere and without being empty, no?

But I tend to prefer the simplicity of limiting this to empty files
anyway.

> After responding to Jonathan, I'm on the fence about whether diff should
> follow the same heuristic. I left the diff behavior unchanged, but a 3/2
> that turns it off by default would be a trivial one-liner.

I am also torn, but somewhat in favor of avoiding random permutations of
empty files.

I can think of only one situation somebody _could_ argue that detecting
empty-to-empty rename is halfway sensible: when there is only one empty
file that was removed, and one new empty file that was added.  Even in
such a case, the user could have done "rm this; >that", and depending on
the nature of these two files, "mv this that" may not have made _any_
sense even if they are both empty, and that is why I said "halfway"
sensible.

  parent reply	other threads:[~2012-03-22 23:37 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-21 10:28 Strange effect merging empty file Ralf Nyren
2012-03-21 10:54 ` Zbigniew Jędrzejewski-Szmek
2012-03-21 17:14   ` Junio C Hamano
2012-03-22 12:17   ` Randal L. Schwartz
2012-03-22 12:39     ` Ralf Nyren
2012-03-22 12:47     ` Zbigniew Jędrzejewski-Szmek
2012-03-22 14:01       ` Jeff King
2012-03-22 17:03         ` Junio C Hamano
2012-03-22 17:59           ` Jeff King
2012-03-22 18:25             ` Jeff King
2012-03-22 18:52               ` Jeff King
2012-03-22 18:53                 ` [PATCH 1/3] drop casts from users EMPTY_TREE_SHA1_BIN Jeff King
2012-03-22 18:53                 ` [PATCH 2/3] make is_empty_blob_sha1 available everywhere Jeff King
2012-03-22 18:53                 ` [PATCH 3/3] merge-recursive: don't detect renames from empty files Jeff King
2012-03-22 19:18                   ` Jonathan Nieder
2012-03-22 21:53                     ` Jeff King
2012-03-22 18:52               ` Strange effect merging empty file Junio C Hamano
2012-03-22 19:03                 ` Jeff King
2012-03-22 19:12                   ` Junio C Hamano
2012-03-22 22:46                     ` [PATCH 0/2] merging renames of empty files Jeff King
2012-03-22 22:52                       ` [PATCH 1/2] teach diffcore-rename to optionally ignore empty content Jeff King
2012-03-22 22:52                       ` [PATCH 2/2] merge-recursive: don't detect renames of empty files Jeff King
2012-03-22 23:37                       ` Junio C Hamano [this message]
2012-03-23  0:23                         ` [PATCH 0/2] merging " Jeff King
2012-03-23  4:56                           ` 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=7vpqc4us0u.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=merlyn@stonehenge.com \
    --cc=peff@peff.net \
    --cc=ralf.nyren@ericsson.com \
    --cc=zbyszek@in.waw.pl \
    /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.