You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2003/12/10 21:08:36 UTC

Issue #1093.

I've got a patch in progress for solving Issue #1093.  I want to
inform folks of the changes I'm making so nothing comes as a surprise,
and to gives you a chance to comment.

As a reminder, the problem I'm out to solve is really straightforward.
Because Subversion has 'copy' and 'move' functionality, and in fact
encourages the use of those concepts to pull off the whole "multiple
lines of development" model, the paths at which a given resource
exists over time can be many.  

Today's 'svn diff -r X:Y PATH' is really dumb -- it literally looks
for PATH in revision X and PATH in revision Y, and diffs them.  That's
not what people want.  For one think, PATH may not exist in one or the
other of those revisions due to some rename that occurred between the
two revisions.  Secondly, PATH might exist in both revisions, but not
as the same resource, which renders diff output possibly quite
insane, but definitely wrong.

Of course, a user *could* get the diff they wanted today using the
'svn diff URL@X URL@Y' syntax, but that means they have to do alot of
work (typically involving reading the output of 'svn log -v') to
figure out the two URLs they need to pass to the operation.  Ideally,
though (as I think we've all agreed by now), 'svn diff' would just
follow a resource's history back through revision time, finding the
appropriate locations in the repository to diff between.

A while ago I proposed a syntax and set of rules to give users control
better control over this expanded functionaly.  Greg Hudson and others
made comments and corrections, some of which I agree with, and some I
don't -- but most of which I just don't remember.  :-)

So allow me to just list the steps I plan to take to solve this
problem, or to at least position us to solve it later.

   Part 1: A new interface, svn_client_repos_locations(), which accepts
           a path (or URL), a "peg" revision, and two revisions in
           which we want to know the locations of that path/URL.  This
           will use RA->get_logs() under the hood -- just like 'svn
           blame' does -- to trace path changes in history.

           ## Finished, but current restricted to PEG >= X and PEG >= Y ##

   Part 2: Teach the command-line client to use this function in the
           'svn diff -rX:Y' cases.  

           ## Almost Finished ##

   Part 3: Teach 'svn merge' to do the right thing, too.  

           ## Not Started ##
   
At the very least, I'd like to get Change 1 in before 1.0.  Changes 2
and 3 would be great, too, but shouldn't (fingers crossed) need API
changes, so I don't care so much about those.

Folks may recall some debate about what do in the 'svn diff [-r X:Y]
PATH-OR-URL[@Z]' situation when Z is < either X or Y.  Since we can't
trace history forward in time (and even if we could, there could be
multiple choices of lineage to follow), I wanted to punt outright.
And I got fussed at for that.  Well, now I realize that we can at
least assume that PATH-OR-URL hasn't been renamed between Z and the
greater of X and Y.  And we can reliable determine if our guess was a
safe one by verifying that the path of the resource in Z is the same
as the resource's path in the greater of the X and Y.

So let's talk about syntax changes.  On Oct. 29, 2003, Greg Hudson
proposed the following:

   1. diff [-r M[:N]] [TARGET...]
   
      Each target may be a path or URL, but it is invalid to specify a
      URL with no -r option. Perform a pegged diff for each
      target. For paths, the default starting rev is BASE, the default
      ending rev is WC, and the peg rev is always WC. For URLs, there
      is no default starting rev (-r option must be present), the
      default ending rev is HEAD, and the peg rev is always HEAD.
   
   2. diff --old=OLD-TGT[@OLDREV] --new=NEW-TGT[@NEWREV] [PATH...]
   
      Compare OLD-TGT and NEW-TGT, restricted to the paths PATH... if
      given.  Can be used to answers questions like "what changes were
      made to libsvn_ra_dav between the 0.32 branch and the trunk?"
      (Limitation: if libsvn_ra_dav was renamed between the 0.32
      branch and the trunk, the user will have to go find out how; we
      can't answer complex questions like "what was the name of
      /trunk/subversion/libsvn_ra_dav under /branches/0.32?".)
   
   3. diff OLD-URL[@OLDREV] NEW-URL[@NEWREV]
   
      Syntactic sugar for 'diff --old=OLD-URL@OLDREV --new=NEW-URL@NEWREV'.  
      Easily distinguishable from the first usage because you can't
      use URLs without a -r option. Note "svn diff URL@R1
      URL@R2" is rather different from "svn diff -rR1:R2 URL",
      because the latter performs a pegged diff of URL@HEAD between
      R1 and R2, while the former looks up URL's pathname
      independently in R1 and R2 and diffs the results.

None of these forms mix the -rX:Y and the @-syntax because Greg deems
that stuff esoteric.  For now, I'm willing to lay down and concede
that.  In my mind, though, the best benefit of that fact is that the
command-line parsing code doesn't have a change a lick in order to
handle these use cases (though, I think there is one de-unification
I'd like to see, not directly related to this topic).

Wow.  (Breathe).

Thoughts?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Issue #1093: "svn diff" enhancement to follow moved/renamed items

Posted by Julian Foad <ju...@btopenworld.com>.
C. Michael Pilato wrote:
> Julian Foad <ju...@btopenworld.com> writes:
> 
>>We should clarify "restricted to the paths PATH..." with regard to
>>that limitation.  The current help says "PATHs, if given, are
>>relative to OLD-TGT and NEW-TGT and restrict the output to
>>differences for those paths."  This is still not quite clear; I
>>assume it means the union of PATHs that exist under OLD-TGT and
>>PATHs that exist under NEW-TGT, with those that are only present on
>>one side causing an "added" or "deleted" diff.  Or does it mean that
>>it is an error if any PATH does not exist on both sides?
> 
> It basically means that a diff will be done between each pair of
> OLD-TGT/PATH vs. NEW-TGT/PATH.  The behavior of each of those diffs
> will be exactly the same as if you'd done 'svn diff OLD-TGT/PATH
> NEW-TGT/PATH'.

i.e. erroring out of the whole command if a PATH does not exist under both OLD-TGT and NEW-TGT.

  svn: 'file:///.../PATH' was not found in the repository at revision 53

I have no problem with that; I just wanted to clarify it.

- Julian


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Issue #1093: "svn diff" enhancement to follow moved/renamed items

Posted by "C. Michael Pilato" <cm...@collab.net>.
Julian Foad <ju...@btopenworld.com> writes:

> That sounds sound.  Syntax 1 below doesn't allow Z to be specified,
> so, until syntax 1 is extended, this is limited to Z = WC.  I would
> guess that this case would most commonly arise in the form "diff
> -rX:HEAD path", when the WC is not quite up to date.

Yep, that was my thinking too.

> We should clarify "restricted to the paths PATH..." with regard to
> that limitation.  The current help says "PATHs, if given, are
> relative to OLD-TGT and NEW-TGT and restrict the output to
> differences for those paths."  This is still not quite clear; I
> assume it means the union of PATHs that exist under OLD-TGT and
> PATHs that exist under NEW-TGT, with those that are only present on
> one side causing an "added" or "deleted" diff.  Or does it mean that
> it is an error if any PATH does not exist on both sides?

It basically means that a diff will be done between each pair of
OLD-TGT/PATH vs. NEW-TGT/PATH.  The behavior of each of those diffs
will be exactly the same as if you'd done 'svn diff OLD-TGT/PATH
NEW-TGT/PATH'.

> > None of these forms mix the -rX:Y and the @-syntax because Greg deems
> > that stuff esoteric.  For now, I'm willing to lay down and concede
> > that.  In my mind, though, the best benefit of that fact is that the
> > command-line parsing code doesn't have a change a lick in order to
> > handle these use cases (though, I think there is one de-unification
> > I'd like to see, not directly related to this topic).
> 
> This seems like a nice clean syntax, and it leaves room to add
> support for "@PEG" on the TARGETs of syntax 1 so that deleted items
> can be referenced easily.

That was my thinking, too.  If we later need to expose the whole API
-- or if other GUI clients want to do so -- it's available.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Issue #1093: "svn diff" enhancement to follow moved/renamed items

Posted by Julian Foad <ju...@btopenworld.com>.
[Added a description to the Subject line so we can see what these messages are about without having to read several paragraphs or fire up the issue tracker. :-) ]


This all sounds good.  Some comments on the details follow.


C. Michael Pilato wrote:
> 
> Folks may recall some debate about what do in the 'svn diff [-r X:Y]
> PATH-OR-URL[@Z]' situation when Z is < either X or Y.  Since we can't
> trace history forward in time (and even if we could, there could be
> multiple choices of lineage to follow), I wanted to punt outright.
> And I got fussed at for that.  Well, now I realize that we can at
> least assume that PATH-OR-URL hasn't been renamed between Z and the
> greater of X and Y.  And we can reliable determine if our guess was a
> safe one by verifying that the path of the resource in Z is the same
> as the resource's path in the greater of the X and Y.

That sounds sound.  Syntax 1 below doesn't allow Z to be specified, so, until syntax 1 is extended, this is limited to Z = WC.  I would guess that this case would most commonly arise in the form "diff -rX:HEAD path", when the WC is not quite up to date.


> So let's talk about syntax changes.  On Oct. 29, 2003, Greg Hudson
> proposed the following:
> 
>    1. diff [-r M[:N]] [TARGET...]
>    
>       Each target may be a path or URL, but it is invalid to specify a
>       URL with no -r option. Perform a pegged diff for each
>       target. For paths, the default starting rev is BASE, the default
>       ending rev is WC, and the peg rev is always WC. For URLs, there
>       is no default starting rev (-r option must be present), the
>       default ending rev is HEAD, and the peg rev is always HEAD.

Add: "The default target is the path '.'."

I think that "peg is WC" is exactly what we need, but just want to point out that it may be slightly trickier to implement than "peg is BASE".  Shouldn't be a problem, though.


>    2. diff --old=OLD-TGT[@OLDREV] --new=NEW-TGT[@NEWREV] [PATH...]

The words "old" and "new" here match one common use case but are misleading in other cases.  But that's a separate issue, and is already in the existing implementation.

Requiring both "--old" and "--new" to be specified in this case is stricter than the current implementation, but I think that is a good thing.  It is always better to start strictly and have the option of relaxing the syntax later than vice versa.

Add: "Each of OLD-TGT and NEW-TGT may be a path or a URL."

Presumably we add: "Each of OLDREV and NEWREV defaults to HEAD for a URL or to WC for a local path."  (The existing implementation defaults to BASE for local paths, but we probably came to the conclusion that WC is better.)

>       Compare OLD-TGT and NEW-TGT, restricted to the paths PATH... if
>       given.  Can be used to answers questions like "what changes were
>       made to libsvn_ra_dav between the 0.32 branch and the trunk?"
>       (Limitation: if libsvn_ra_dav was renamed between the 0.32
>       branch and the trunk, the user will have to go find out how; we
>       can't answer complex questions like "what was the name of
>       /trunk/subversion/libsvn_ra_dav under /branches/0.32?".)

We should clarify "restricted to the paths PATH..." with regard to that limitation.  The current help says "PATHs, if given, are relative to OLD-TGT and NEW-TGT and restrict the output to differences for those paths."  This is still not quite clear; I assume it means the union of PATHs that exist under OLD-TGT and PATHs that exist under NEW-TGT, with those that are only present on one side causing an "added" or "deleted" diff.  Or does it mean that it is an error if any PATH does not exist on both sides?

>    3. diff OLD-URL[@OLDREV] NEW-URL[@NEWREV]
>    
>       Syntactic sugar for 'diff --old=OLD-URL@OLDREV --new=NEW-URL@NEWREV'.  
>       Easily distinguishable from the first usage because you can't
>       use URLs without a -r option. Note "svn diff URL@R1
>       URL@R2" is rather different from "svn diff -rR1:R2 URL",
>       because the latter performs a pegged diff of URL@HEAD between
>       R1 and R2, while the former looks up URL's pathname
>       independently in R1 and R2 and diffs the results.
> 
> None of these forms mix the -rX:Y and the @-syntax because Greg deems
> that stuff esoteric.  For now, I'm willing to lay down and concede
> that.  In my mind, though, the best benefit of that fact is that the
> command-line parsing code doesn't have a change a lick in order to
> handle these use cases (though, I think there is one de-unification
> I'd like to see, not directly related to this topic).

This seems like a nice clean syntax, and it leaves room to add support for "@PEG" on the TARGETs of syntax 1 so that deleted items can be referenced easily.

- Julian


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Issue #1093.

Posted by "C. Michael Pilato" <cm...@collab.net>.
"C. Michael Pilato" <cm...@collab.net> writes:

> I've got a patch in progress for solving Issue #1093.  I want to
> inform folks of the changes I'm making so nothing comes as a surprise,
> and to gives you a chance to comment.

I've decided that this fix is coming too late in the game for
Subversion 1.0.  I've posted my patch-in-progress to the issue, and am
ceasing work on the issue immediately.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Issue #1093.

Posted by Ben Collins-Sussman <su...@collab.net>.
On Wed, 2003-12-10 at 15:08, C. Michael Pilato wrote:

> 'svn diff' would just
> follow a resource's history back through revision time, finding the
> appropriate locations in the repository to diff between.

Oh man, this is so great.  Imagine having a working copy of a branch,
and running 'svn diff -rX foo.c', and *not* getting an error just
because the branch was created after rX!

People have been wanting this for so long.




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org