[RFC/PATCH] t9159-*.sh: Don't use the svn '@<rev>' syntax

[RFC/PATCH] t9159-*.sh: Don't use the svn '@<rev>' syntax

[Ramsay Jones]

Avoiding the use of the '@<rev>' syntax, in favour of an '-r <rev>'
parameter, allows older versions of svn to execute the test.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Hi Junio,

This test is failing for me on Linux and MinGW (I don't have svn
installed on cygwin), again (i suspect) due to the older versions
of svn I have. [v1.4.3 on Linux and v1.4.6 on MinGW]

This patch fixes the test for me. However, I noticed that there are
two other uses of the syntax in t9104-git-svn-follow-parent.sh which
look like this:

        (svn_cmd cp -m "resurrecting trunk as junk" \
               "$svnrepo"/trunk@2 "$svnrepo"/junk ||
         svn cp -m "resurrecting trunk as junk" \
               -r2 "$svnrepo"/trunk "$svnrepo"/junk) &&

which, unless I'm confused (possible), has been coded specifically
to cater to just this problem!

However, I think the above is a little too "belt & braces" for my
liking. What do you think?

Hmm, I've just found commit ffab62681 (git-svn: handle changed svn
command-line syntax, 21-09-2007), which seems to imply that the
above is necessary due to a change in behaviour. I'm afraid I don't
know svn much at all; the only reason it is installed on Linux is
to improve test coverage of git! :-P

So, maybe this patch should do the same as above (which is the reason
for the RFC); dunno ...

Hopefully, someone who knows svn can pick this up and make a proper patch.

ATB,
Ramsay Jones

 t/t9159-git-svn-no-parent-mergeinfo.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t9159-git-svn-no-parent-mergeinfo.sh b/t/t9159-git-svn-no-parent-mergeinfo.sh
index 85120b7..4cf26e9 100755
--- a/t/t9159-git-svn-no-parent-mergeinfo.sh
+++ b/t/t9159-git-svn-no-parent-mergeinfo.sh
@@ -11,7 +11,7 @@ test_expect_success 'test handling of root commits in merge ranges' '
 		cd tmp &&
 		echo "r2" > trunk/file.txt &&
 		svn_cmd commit -m "Modify file.txt on trunk" &&
-		svn_cmd cp trunk@1 branches/a &&
+		svn_cmd cp -r1 trunk branches/a &&
 		svn_cmd commit -m "Create branch a from trunk r1" &&
 		svn_cmd propset svn:mergeinfo /trunk:1-2 branches/a &&
 		svn_cmd commit -m "Fake merge of trunk r2 into branch a" &&
@@ -21,7 +21,7 @@ test_expect_success 'test handling of root commits in merge ranges' '
 		svn_cmd commit -m "Create branch b from thin air" &&
 		echo "r6" > branches/b/file2.txt &&
 		svn_cmd commit -m "Modify file2.txt on branch b" &&
-		svn_cmd cp branches/b@5 branches/c &&
+		svn_cmd cp -r5 branches/b branches/c &&
 		svn_cmd commit -m "Create branch c from branch b r5" &&
 		svn_cmd propset svn:mergeinfo /branches/b:5-6 branches/c &&
 		svn_cmd commit -m "Fake merge of branch b r6 into branch c"
-- 
1.7.6

Re: [RFC/PATCH] t9159-*.sh: Don't use the svn '@<rev>' syntax

[Junio C Hamano]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> Avoiding the use of the '@<rev>' syntax, in favour of an '-r <rev>'
> parameter, allows older versions of svn to execute the test.
>
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
>
> Hi Junio,
>
> This test is failing for me on Linux and MinGW (I don't have svn
> installed on cygwin), again (i suspect) due to the older versions
> of svn I have. [v1.4.3 on Linux and v1.4.6 on MinGW]
>
> This patch fixes the test for me. However, I noticed that there are
> two other uses of the syntax in t9104-git-svn-follow-parent.sh which
> look like this:
>
>         (svn_cmd cp -m "resurrecting trunk as junk" \
>                "$svnrepo"/trunk@2 "$svnrepo"/junk ||
>          svn cp -m "resurrecting trunk as junk" \
>                -r2 "$svnrepo"/trunk "$svnrepo"/junk) &&
>
> which, unless I'm confused (possible), has been coded specifically
> to cater to just this problem!
> However, I think the above is a little too "belt & braces" for my
> liking. What do you think?

Hmm, but I am actually having trouble understanding what ffab626 (git-svn:
handle changed svn command-line syntax, 2007-09-21) wanted to do.  It
says:

    git-svn: handle changed svn command-line syntax
    
    Previously, if you passed a revision and a path to svn cp, it meant to look
    back at that revision and select that path.  New behaviour is to get the
    path then go back to the revision (like other commands that accept @REV
    or -rREV do).  The more consistent syntax is not supported by the old
    tools, so we have to try both in turn.
    
    Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>
    Acked-by: Eric Wong <normalperson@yhbt.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

The explanation is not making sense at all. "Go back at the revision and
pick that path" vs "Pick the path and go back to the revision" would
change behaviour if the path was renamed since the revision, but it is not
like that the issue ffab626 wanted to fix was make sure which semantics is
used by using the right syntax that uses that semantics, so why did it
bring it up in the first place?  The first three lines and half might be
stating the fact, but it seems utterly irrelevant to what the commit does.

I am guessing that the "more consistent syntax" in the above is trying to
say that the author wants to see "path@REV" used as much as possible
instead of using "-rREV". But why use path@REV if newer subversion does
not drop support for the path@REV syntax?

Or is it "path@REV picks path and then goes to the REV, vs -rREV path goes
to REV and picks path"? If that is the case, then "we try both in turn"
needs a lot more explanation. If a newer subversion with path@REV support
that follows the "file identity" of path can be used, it copies from what
is now called "trunk" as it used to be at rev2, while an older subversion
without it cannot do so and copies from "trunk" that was at rev2. These
two may give vastly different results if "trunk" was renamed in between,
no? And if there was no such rename of "trunk" that affects what would
happen, then there is _no_ point in writing "path@REV || -rREV path" in
the first place---we know we can always use "-rREV path" without worrying
about what version of subversion is being used.

I am confused. Sam/Eric, care to help clarify the situation?

I also recall that svn:mergeinfo support is relatively new; is it even
supported in a old version that did not understand trunk@1 syntax?

>  t/t9159-git-svn-no-parent-mergeinfo.sh |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t9159-git-svn-no-parent-mergeinfo.sh b/t/t9159-git-svn-no-parent-mergeinfo.sh
> index 85120b7..4cf26e9 100755
> --- a/t/t9159-git-svn-no-parent-mergeinfo.sh
> +++ b/t/t9159-git-svn-no-parent-mergeinfo.sh
> @@ -11,7 +11,7 @@ test_expect_success 'test handling of root commits in merge ranges' '
>  		cd tmp &&
>  		echo "r2" > trunk/file.txt &&
>  		svn_cmd commit -m "Modify file.txt on trunk" &&
> -		svn_cmd cp trunk@1 branches/a &&
> +		svn_cmd cp -r1 trunk branches/a &&
>  		svn_cmd commit -m "Create branch a from trunk r1" &&
>  		svn_cmd propset svn:mergeinfo /trunk:1-2 branches/a &&
>  		svn_cmd commit -m "Fake merge of trunk r2 into branch a" &&
> @@ -21,7 +21,7 @@ test_expect_success 'test handling of root commits in merge ranges' '
>  		svn_cmd commit -m "Create branch b from thin air" &&
>  		echo "r6" > branches/b/file2.txt &&
>  		svn_cmd commit -m "Modify file2.txt on branch b" &&
> -		svn_cmd cp branches/b@5 branches/c &&
> +		svn_cmd cp -r5 branches/b branches/c &&
>  		svn_cmd commit -m "Create branch c from branch b r5" &&
>  		svn_cmd propset svn:mergeinfo /branches/b:5-6 branches/c &&
>  		svn_cmd commit -m "Fake merge of branch b r6 into branch c"

Re: [RFC/PATCH] t9159-*.sh: Don't use the svn '@<rev>' syntax

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 19.07.2011 21:58:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> 
>> Avoiding the use of the '@<rev>' syntax, in favour of an '-r <rev>'
>> parameter, allows older versions of svn to execute the test.
>>
>> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
>> ---
>>
>> Hi Junio,
>>
>> This test is failing for me on Linux and MinGW (I don't have svn
>> installed on cygwin), again (i suspect) due to the older versions
>> of svn I have. [v1.4.3 on Linux and v1.4.6 on MinGW]
>>
>> This patch fixes the test for me. However, I noticed that there are
>> two other uses of the syntax in t9104-git-svn-follow-parent.sh which
>> look like this:
>>
>>         (svn_cmd cp -m "resurrecting trunk as junk" \
>>                "$svnrepo"/trunk@2 "$svnrepo"/junk ||
>>          svn cp -m "resurrecting trunk as junk" \
>>                -r2 "$svnrepo"/trunk "$svnrepo"/junk) &&
>>
>> which, unless I'm confused (possible), has been coded specifically
>> to cater to just this problem!
>> However, I think the above is a little too "belt & braces" for my
>> liking. What do you think?
> 
> Hmm, but I am actually having trouble understanding what ffab626 (git-svn:
> handle changed svn command-line syntax, 2007-09-21) wanted to do.  It
> says:
> 
>     git-svn: handle changed svn command-line syntax
>     
>     Previously, if you passed a revision and a path to svn cp, it meant to look
>     back at that revision and select that path.  New behaviour is to get the
>     path then go back to the revision (like other commands that accept @REV
>     or -rREV do).  The more consistent syntax is not supported by the old
>     tools, so we have to try both in turn.
>     
>     Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>
>     Acked-by: Eric Wong <normalperson@yhbt.net>
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
> 
> The explanation is not making sense at all. "Go back at the revision and
> pick that path" vs "Pick the path and go back to the revision" would
> change behaviour if the path was renamed since the revision, but it is not
> like that the issue ffab626 wanted to fix was make sure which semantics is
> used by using the right syntax that uses that semantics, so why did it
> bring it up in the first place?  The first three lines and half might be
> stating the fact, but it seems utterly irrelevant to what the commit does.
> 
> I am guessing that the "more consistent syntax" in the above is trying to
> say that the author wants to see "path@REV" used as much as possible
> instead of using "-rREV". But why use path@REV if newer subversion does
> not drop support for the path@REV syntax?
> 
> Or is it "path@REV picks path and then goes to the REV, vs -rREV path goes
> to REV and picks path"? If that is the case, then "we try both in turn"
> needs a lot more explanation. If a newer subversion with path@REV support
> that follows the "file identity" of path can be used, it copies from what
> is now called "trunk" as it used to be at rev2, while an older subversion
> without it cannot do so and copies from "trunk" that was at rev2. These
> two may give vastly different results if "trunk" was renamed in between,
> no? And if there was no such rename of "trunk" that affects what would
> happen, then there is _no_ point in writing "path@REV || -rREV path" in
> the first place---we know we can always use "-rREV path" without worrying
> about what version of subversion is being used.
> 
> I am confused. Sam/Eric, care to help clarify the situation?

path@REV are so-called peg revisions, introduced in svn 1.1, and denote
"I mean the file named path in REV" (as opposed to "the file named path
now and maybe differently back then"). It (now) defaults to BASE (for
worktree) resp. HEAD (for URLs). A bit like our rename detection.

-r REV specifies the operative revision. After resolving the
name/location using the pegrev, the version at the resolved path at the
oprative version is operated on.

svn 1.5.0 (June 2008) introduced peg revisions to "svn copy", so I
assume our people were following svn trunk and adjusting in 2007 already
(to r22964). There were some fixes to "svn copy" with peg later on.

I do not understand the above commit message at all; and I did not find
anything about how "svn copy -r REV" acted in svn 1.4. I would assume
"operative revision", and the above commit message seems to imply that
peg defaulted to REV here (not HEAD) and that that changed in 1.5.0, but
that is a wild guess (svnbook 1.4 does not so anything).

> I also recall that svn:mergeinfo support is relatively new; is it even
> supported in a old version that did not understand trunk@1 syntax?

While that is true, the said test sets the svn:mergeinfo property and
commits it (which you can do with any svn that knows properties), and
then tests whether our own "git svn init -s && git svn fetch" can grok
that, which only needs a property-reading svn (-binding). So, this does
not need subversion's merge support at all.

Cheers,
Michael

Re: [RFC/PATCH] t9159-*.sh: Don't use the svn '@<rev>' syntax

[Sam Vilain]

On 20/07/11 10:07, Michael J Gruber wrote:
> path@REV are so-called peg revisions, introduced in svn 1.1, and denote
> "I mean the file named path in REV" (as opposed to "the file named path
> now and maybe differently back then"). It (now) defaults to BASE (for
> worktree) resp. HEAD (for URLs). A bit like our rename detection.
>
> -r REV specifies the operative revision. After resolving the
> name/location using the pegrev, the version at the resolved path at the
> oprative version is operated on.
>
> svn 1.5.0 (June 2008) introduced peg revisions to "svn copy", so I
> assume our people were following svn trunk and adjusting in 2007 already
> (to r22964). There were some fixes to "svn copy" with peg later on.
>
> I do not understand the above commit message at all; and I did not find
> anything about how "svn copy -r REV" acted in svn 1.4. I would assume
> "operative revision", and the above commit message seems to imply that
> peg defaulted to REV here (not HEAD) and that that changed in 1.5.0, but
> that is a wild guess (svnbook 1.4 does not so anything).

What happened is that I noticed that the code stopped working after svn
1.5 was released.  Previously I wrote it to detect the merge properties
as left by SVK and the experimental/contrib python script for merging. 
I was testing at times using trunk SVN versions.  You could probably
figure it out by running ffab6268^ with svn 1.4.x vs svn 1.5.x if you
cared.  My comment tries to explain what you describe above, but without
the correct terms.  I could see via experimentation what the difference
was between "-r N" and '/path@N', and that the behaviour changed in svn
1.5.  Apologies for not explaining this thoroughly enough in the
submitted description!

HTH,
Sam

Re: [RFC/PATCH] t9159-*.sh: Don't use the svn '@<rev>' syntax

[Ramsay Jones]

Sam Vilain wrote:
> On 20/07/11 10:07, Michael J Gruber wrote:
>> path@REV are so-called peg revisions, introduced in svn 1.1, and denote
>> "I mean the file named path in REV" (as opposed to "the file named path
>> now and maybe differently back then"). It (now) defaults to BASE (for
>> worktree) resp. HEAD (for URLs). A bit like our rename detection.
>>
>> -r REV specifies the operative revision. After resolving the
>> name/location using the pegrev, the version at the resolved path at the
>> oprative version is operated on.
>>
>> svn 1.5.0 (June 2008) introduced peg revisions to "svn copy", so I
>> assume our people were following svn trunk and adjusting in 2007 already
>> (to r22964). There were some fixes to "svn copy" with peg later on.
>>
>> I do not understand the above commit message at all; and I did not find
>> anything about how "svn copy -r REV" acted in svn 1.4. I would assume
>> "operative revision", and the above commit message seems to imply that
>> peg defaulted to REV here (not HEAD) and that that changed in 1.5.0, but
>> that is a wild guess (svnbook 1.4 does not so anything).
> 
> What happened is that I noticed that the code stopped working after svn
> 1.5 was released.  Previously I wrote it to detect the merge properties
> as left by SVK and the experimental/contrib python script for merging. 
> I was testing at times using trunk SVN versions.  You could probably
> figure it out by running ffab6268^ with svn 1.4.x vs svn 1.5.x if you
> cared.  My comment tries to explain what you describe above, but without
> the correct terms.  I could see via experimentation what the difference
> was between "-r N" and '/path@N', and that the behaviour changed in svn
> 1.5.  Apologies for not explaining this thoroughly enough in the
> submitted description!

Hmm, I was hoping that someone would say something like:

   "This test does not depend on the difference between the peg revision
and the operative revision, because the history represented in the test
repo is so simple that there *is* no difference, so Acked By: ... "

But, since that didn't happen, maybe the patch given below would be more
acceptable? (I personally prefer the original patch ...)

Given that I didn't quite follow Sam's explanation, I still don't know
if t9104-git-svn-follow-parent.sh needs to be changed (again, this test
*passes* for me), so ... :-P

ATB,
Ramsay Jones

-- >8 --
Subject: [PATCH] t9159-*.sh: Add an svn version check


Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
 t/t9159-git-svn-no-parent-mergeinfo.sh |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/t/t9159-git-svn-no-parent-mergeinfo.sh b/t/t9159-git-svn-no-parent-mergeinfo.sh
index 85120b7..69e4815 100755
--- a/t/t9159-git-svn-no-parent-mergeinfo.sh
+++ b/t/t9159-git-svn-no-parent-mergeinfo.sh
@@ -2,6 +2,14 @@
 test_description='git svn handling of root commits in merge ranges'
 . ./lib-git-svn.sh
 
+svn_ver="$(svn --version --quiet)"
+case $svn_ver in
+0.* | 1.[0-4].*)
+	skip_all="skipping git-svn test - SVN too old ($svn_ver)"
+	test_done
+	;;
+esac
+
 test_expect_success 'test handling of root commits in merge ranges' '
 	mkdir -p init/trunk init/branches init/tags &&
 	echo "r1" > init/trunk/file.txt &&
-- 
1.7.6

Re: [RFC/PATCH] t9159-*.sh: Don't use the svn '@<rev>' syntax

[Eric Wong]

Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:
> Hmm, I was hoping that someone would say something like:
> 
>    "This test does not depend on the difference between the peg revision
> and the operative revision, because the history represented in the test
> repo is so simple that there *is* no difference, so Acked By: ... "

Unfortunately, I remain perpetually confused w.r.t peg revisions vs "-r"
and how it's handled differently between different SVN versions.

> But, since that didn't happen, maybe the patch given below would be more
> acceptable? (I personally prefer the original patch ...)

I'm much more comfortable with your below patch as it won't break
on newer (and increasingly more widely-used) versions.

> Given that I didn't quite follow Sam's explanation, I still don't know
> if t9104-git-svn-follow-parent.sh needs to be changed (again, this test
> *passes* for me), so ... :-P

> Subject: [PATCH] t9159-*.sh: Add an svn version check

How about something along the lines of:

  Subject: [PATCH] t9159-*.sh: skip for mergeinfo test for svn <= 1.4

  t9159 relies on the command-line syntax of svn >= 1.5.
  Given the declining install base of older svn versions,
  it is not worth our time to support older svn syntax.

  Acked-by: Eric Wong <normalperson@yhbt.net>
  Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>

?

> diff --git a/t/t9159-git-svn-no-parent-mergeinfo.sh b/t/t9159-git-svn-no-parent-mergeinfo.sh
> index 85120b7..69e4815 100755
> --- a/t/t9159-git-svn-no-parent-mergeinfo.sh
> +++ b/t/t9159-git-svn-no-parent-mergeinfo.sh
> @@ -2,6 +2,14 @@
>  test_description='git svn handling of root commits in merge ranges'
>  . ./lib-git-svn.sh
>  
> +svn_ver="$(svn --version --quiet)"
> +case $svn_ver in
> +0.* | 1.[0-4].*)
> +	skip_all="skipping git-svn test - SVN too old ($svn_ver)"
> +	test_done
> +	;;
> +esac
> +
>  test_expect_success 'test handling of root commits in merge ranges' '
>  	mkdir -p init/trunk init/branches init/tags &&
>  	echo "r1" > init/trunk/file.txt &&
> -- 

Re: [RFC/PATCH] t9159-*.sh: Don't use the svn '@<rev>' syntax

[John Szakmeister]

On Tue, Sep 13, 2011 at 3:57 AM, Eric Wong <normalperson@yhbt.net> wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:
>> Hmm, I was hoping that someone would say something like:
>>
>>    "This test does not depend on the difference between the peg revision
>> and the operative revision, because the history represented in the test
>> repo is so simple that there *is* no difference, so Acked By: ... "
>
> Unfortunately, I remain perpetually confused w.r.t peg revisions vs "-r"
> and how it's handled differently between different SVN versions.

I haven't followed this conversation, but I can explain what peg
revisions are.  It used to be the only way to talk about a version of
a file or path was to use '-r REVNUM'.  IIRC, what would happen is
that the url you provided would refer to the now current name of the
file, and Subversion would walk back in history to look up the
specified version of the file.  However, that becomes a problem when
the file no longer exists, or has been renamed.  It's compounded if
there is a new file with the same name in HEAD, because it would look
up the wrong history.  So peg revisions were introduced to say "start
looking here @PEGREV and search forward or backwards for the version
in REVNUM."  So the difference is that old clients always looked at
HEAD to start the search, whereas new clients will use PEGREV if
specified, and HEAD otherwise.

There is a pretty thorough discussion of it in the SVN book:
    <http://svnbook.red-bean.com/en/1.6/svn.advanced.pegrevs.html>

Hope that helps!

-John

Re: [RFC/PATCH] t9159-*.sh: Don't use the svn '@<rev>' syntax

[Junio C Hamano]

Thanks; will queue.

Re: [RFC/PATCH] t9159-*.sh: Don't use the svn '@<rev>' syntax

[Michael J Gruber]

And here's just a ping post with Sam's *correct* address so that he's
not shut out but aware of the thread...

Re: [RFC/PATCH] t9159-*.sh: Don't use the svn '@<rev>' syntax

[Michael J Gruber]

Michael J Gruber venit, vidit, dixit 20.07.2011 11:10:
> And here's just a ping post with Sam's *correct* address so that he's

Or so I thought. Last try. Promised.

> not shut out but aware of the thread...