Re: [PATCH v7 1/3] Introduce bulk-move detection in diffcore.

From: Yann Dirson <ydirson@free.fr>
Date: Oct 28, 2010, 21:42 UTC
Message-ID: <20101028214240.GF3347@home.lan>
In-Reply-To: <7vr5fa9ij3.fsf@alter.siamese.dyndns.org>

On Thu, Oct 28, 2010 at 01:20:00PM -0700, Junio C Hamano wrote:

Show 7 quoted lines

> Yann Dirson <ydirson@free.fr> writes:
> 
> > OTOH, the quoting rules for diff output are quite minimalist, I don't
> > know whether adding "*" as a character that requires quotes around the
> > filename would be acceptable.
> 
> I suspect that would be an unacceptable entry to a slippery slope.

OK.  I have thought of only 2 remaining options:

* keep using 0{40} sha1 as differentiator
  => will prevent us to ever use this sha1 for something useful
* introducing a new change-type letter
  => another slippery slope, with at least bulk removals and bulk
  copies in sight, with already too few letters to keep them meaningful
  to human reader
* introducing a "bulk" flag
  => needs extension of the raw format
  Would could want to extend the <letter><score> group, for things like
  "RB100" (bulk rename), "CB090" (imperfect bulk copy), "DB096" (imperfect
  bulk removal, meaning there were some files left in source directory)

Show 28 quoted lines

> >> IOW, is the goal of this series
> >> to use the "A/* -> B/" to label the change as bulk directory rename, if
> >> the preimage has A/{1,2,3} and the postimage has their moved contents in
> >> B/{one,two,three}?
> >
> > Yes.  But --hide-bulk-move-details would not hide them, as they would
> > not be strictly included in the bulk move.  Desite their name change,
> > they are however a confirmation that the contents of A/ was move to B/.
> >
> >> I am wondering about the utility of such an extra information.  If there
> >> were no "a/file0 -> b/file3" entry in the example, I would imagine that we
> >> could use this "a/* -> b/" information to move "a/file5" to "b/file5" when
> >> rebasing this patch to apply to a different preimage that had files other
> >> than file{1,2} in directory "a", and I would further imagine that might be
> >> a wonderful thing.
> >
> > I imagined that as well, and that situation would not be a problem:
> > since "a/file0 -> b/file3" would be there in the rebased patch,
> > "apply" would be able to spot the possible conflict.
> >
> > OTOH, I had the vision of "merge does automatic moves" when starting
> > this project, but got convinced on-list that there are always cases
> > where the "automatic move" on merge would be wrong, and that we should
> > report a conflict instead.
> >
> > That would mostly shift the problem to...
> 
> That would mostly make this patch not worth worrying about, wouldn't it?

(assuming "that" refers to not doing automatic moves on merge)

No, at least the "avoid hiding useful info among less-relevant stuff" provided by the human-targetted --hide-bulk-move-details would stay regardless of what we decide here.

> What's the point of spending extra cycles to say "many things have moved
> in the same direction" without turning it into a usable information?

If we issue a conflict, we have already gone farther than where we are now. Deciding to have this particular type of conflict auto-resolved in the way useful to a wider audience could be done (having "adds" follow the bulkmove), with obviously the option of not auto-resolving, and possibly, the option of auto-resolving "adds" as they are (ie. keep current behaviour).

But my feeling is "automatically solving the conflict" would be be only marginally different from applying a patch generated with (what in current series is) --hide-bulk-move-details where you cannot know which individual files were moved in the patch author's tree, and has no info to decide if his own added files are at the right place or need to follow the move (which I understood to be your point in [1]).

Or did I miss your point ?

[1] http://marc.info/?l=git&m=122610146506413

-- 
Yann