Re: FreeBSD project and subversion.
On 06.02.2013 07:21, Branko Čibej wrote: Given all the above, I think we should find a way to warn users -- with a one-line note in the header of the diff output, for example? -- about the cases where the two-parameter diff could be ambiguous (which, I believe, is when both parametrs are WC paths or URLs); and further, when any of the paths are unversioned. One other option would be, as was noted elsewhere in this thread, to invent a new subcommand for tree comparisons, keeping only the historical differences feature in svn diff. That would solve the ambiguity, but it would (a) break backward compatibility, and (b) invent yet another subcommand, which we don't like. Yet another option would be to introduce a new option, so instead of using svn diff --old A --new B you'd get svn diff --compare A B to generate a diff between the two targets, and plain svn diff A B to get the historical diff of each target. Unfortunately this option has all the drawbacks of the new subcommand, and none of the benefits (the benefit being that with a subcommand, the help text could be shorter, since that's tied to the subcommand, not individual options). -- Brane -- Branko Čibej Director of Subversion | WANdisco | www.wandisco.com
Re: FreeBSD project and subversion.
On Wed, Feb 06, 2013 at 07:21:04AM +0100, Branko Čibej wrote: Given all the above, I think we should find a way to warn users -- with a one-line note in the header of the diff output, for example? -- about the cases where the two-parameter diff could be ambiguous (which, I believe, is when both parametrs are WC paths or URLs); and further, when any of the paths are unversioned. We've always had the following 2-params cases (where no -r or -c options are present): diff WC-PATH WC-PATH - show diff against BASE for each target diff URL1 URL2 - show diff between URLs We've now added URL WC-PATH and WC-PATH URL cases to the mix, in response to a user who expected these to behave like URL1 URL2, and was surprised to learn that they were erroring out instead: diff URL WC-PATH - show diff between URL and WC-PATH diff WC-PATH URL - show diff between WC-PATH and URL I think there is a case to be made for keeping these. Because the odd one out here is the WC-PATH WC-PATH case. As you stated this case had to be compatible to CVS. It has always behaved differently to the URL1 URL2 case. So users have already been confused with 2-param diff behaviour depending on the target type. Hence I don't think adding the new cases makes things any worse. I still think it's a usability enhancement because the user gets a reasonable diff instead of an error. But I can see that adding unversioned paths to the mix would increase the likelihood of user confusion. That would open the door for 'svn diff PATH1 PATH2' to produce either a diff against BASE for each file if both files are versioned, or a diff between PATH1 and PATH2 if either or both of PATH1 are unversioned. That's asking for trouble...
2-params diff (was: Re: FreeBSD project and subversion.)
On Wed, Feb 06, 2013 at 12:41:05PM +0100, Branko Čibej wrote: One other option would be, as was noted elsewhere in this thread, to invent a new subcommand for tree comparisons, keeping only the historical differences feature in svn diff. That would solve the ambiguity, but it would (a) break backward compatibility, and (b) invent yet another subcommand, which we don't like. I don't think we need to change how 'svn diff' works. A new subcommand could simply provide nicer syntax than 'svn diff' for some use cases. We can keep 'svn diff' working as it is. I don't think we should ever change how 'svn diff URL1 URL2' and 'svn diff WC-PATH1 WC-PATH2' work. But a new subcommand could behave differently in the 'WC-PATH1 WC-PATH2' case, and compare WC-PATH1 to WC-PATH2. It would essentially be an alias for 'diff --old --new'. It may need to overlap a lot with 'svn diff', for options such as --summarize perhaps, and all the little options that control little aspects of the diff (--ignore-properties, --show-copies-as-adds, etc). Would it be worth adding? Not sure. Yet another option would be to introduce a new option, so instead of using svn diff --old A --new B you'd get svn diff --compare A B to generate a diff between the two targets, and plain svn diff A B to get the historical diff of each target. I don't think adding a new option to 'svn diff' would help at all. The point is to unclutter the user interface by providing a more straightforward way of comparing two paths/URLs with each other. Another option (especially one that overlaps with functionality provided by existing --old and --new options) would make the UI even more complicated than it already is.
Re: FreeBSD project and subversion.
On Tue, Feb 5, 2013 at 1:39 AM, Alexey Neyman sti...@att.net wrote: On Monday, February 04, 2013 10:17:29 PM Stefan Sperling wrote: On Mon, Feb 04, 2013 at 11:54:21AM -0800, Alexey Neyman wrote: On Thursday, January 31, 2013 09:08:20 am Stefan Sperling wrote: BTW, I went over one of FreeBSD's svn wiki pages a while back, and added answers to open questions on this page: https://wiki.freebsd.org/SubversionMissing Speaking of which: Is there a reason why it is necessary to use the svn diff --old ^/somebranch --new . instead of the intuitive svn ^/somebranch .? Would svn diff ^/somebranch . be synopsis 1? svn diff -rN:M ^/somebranch@HEAD .@HEAD It cannot be synopsis 1, as its description states that TARGETs may be all working copy paths or all URLs. In this case, we have one of each. Moreover, synopsis 1 says that for URLs, -r or -c must be present - and my example has no -r/-c option. Sorry, that's not true. From 'svn help diff' of 1.7: diff (di): Display the differences between two revisions or paths. usage: 1. diff [-c M | -r N[:M]] [TARGET[@REV]...] 2. diff [-r N[:M]] --old=OLD-TGT[@OLDREV] [--new=NEW-TGT[@NEWREV]] \ [PATH...] 3. diff OLD-URL[@OLDREV] NEW-URL[@NEWREV] So for usage 1 the -c and -r are optional. Also, the condition may be all working copy paths or all URLs is pretty weak for deciding to use usage 1, because for usage 2 (--old/--new) you can just as well use two working copy paths or two URLs. I think it would be even more confusing if 'svn diff A B' would decide to use usage 1 if they are both URLs or both wc paths, and use usage 2 if only one of them is a URL and the other a wc path (even though usage 2 can just as well accomodate two URLs and two wc paths). So, the only possible use of this syntax would be synopsis 2: svn diff --old ^/somebranch@HEAD --new .@HEAD (i.e. show the difference between the two (path, revision) pairs somebranch@HEAD and .@HEAD) ? It has bugged me for a while until I discovered this --old/--new trick. I agree that --old --new syntax isn't intuitive at all. What adds oil to the fire is that the error message, which claims that Target lists to diff may not contain both working copy paths and URL. If instead it said that the supplied syntax is ambiguous and suggested to use --new/--old for disambiguation, this would have saved a lot of frustration and would eliminate the need of svn compare subcommand :) For example: svn: Ambiguous target list (contains both work copy paths and URLs). Use --old/--new syntax to disambiguate. Given what I said above, this can't be done in general (not if you give two URLs or two wc paths, because then both synopsis 1 and 2 are possible -- it would obviously break backwards compatibility if we would now start erroring out on these, while these used to work fine with automatically picking usage 1 before). Only if one of them is a URL and the other a WC path, because we give an error message then anyway. -- Johan
Re: FreeBSD project and subversion.
On Tue, Feb 05, 2013 at 12:59:43PM +0100, Johan Corveleyn wrote: Given what I said above, this can't be done in general (not if you give two URLs or two wc paths, because then both synopsis 1 and 2 are possible -- it would obviously break backwards compatibility if we would now start erroring out on these, while these used to work fine with automatically picking usage 1 before). Only if one of them is a URL and the other a WC path, because we give an error message then anyway. I think Alexey meant only the cases where we currently error out, i.e. where exactly two targets are given and one is a path and the other is a URL. I didn't realise this but there is indeed no ambiguity in allowing this syntax as a shorthand for a corresponding '--old X --new Y' diff invocation. Thanks Alexey, see http://svn.apache.org/r1442640 Making these invocations produce a diff instead of an error makes things less surprising for users.
Re: FreeBSD project and subversion.
On Mon, Feb 04, 2013 at 04:39:32PM -0800, Alexey Neyman wrote: What adds oil to the fire is that the error message, which claims that Target lists to diff may not contain both working copy paths and URL. If instead it said that the supplied syntax is ambiguous and suggested to use --new/--old for disambiguation, this would have saved a lot of frustration and would eliminate the need of svn compare subcommand :) For example: svn: Ambiguous target list (contains both work copy paths and URLs). Use --old/--new syntax to disambiguate. Agreed. The error message wasn't very helpful. I've improved it in http://svn.apache.org/r1442645 based on your suggestion. Thanks!
Re: FreeBSD project and subversion.
On Tuesday, February 05, 2013 05:08:40 PM Stefan Sperling wrote: On Tue, Feb 05, 2013 at 12:59:43PM +0100, Johan Corveleyn wrote: Given what I said above, this can't be done in general (not if you give two URLs or two wc paths, because then both synopsis 1 and 2 are possible -- it would obviously break backwards compatibility if we would now start erroring out on these, while these used to work fine with automatically picking usage 1 before). Only if one of them is a URL and the other a WC path, because we give an error message then anyway. I think Alexey meant only the cases where we currently error out, i.e. where exactly two targets are given and one is a path and the other is a URL. I didn't realise this but there is indeed no ambiguity in allowing this syntax as a shorthand for a corresponding '--old X --new Y' diff invocation. Thanks Alexey, see http://svn.apache.org/r1442640 Making these invocations produce a diff instead of an error makes things less surprising for users. Thanks! There is one more weird issue with svn diff, see the script below. The issue is that --old=A --new=B is not opposite of --old=B --new=A. I don't know if it is a bug or another ambuguity I am not aware of :) Here is the script: [[[ #!/bin/bash REPO=/tmp/foo WC=/tmp/foo.wc rm -rf $REPO $WC svnadmin create $REPO svn co -q file://$REPO $WC cd $WC echo r1 a svn add -q a svn ci -q -m R1 echo r2 a svn ci -q -m R2 svn up -q -r 1 echo new a echo Issue: --old=A --new=B is not opposite of --old=B --new=A echo Running: svn di --old=^/ --new=. svn di --old=^/ --new=. echo Running: svn di --old=. --new=^/ svn di --old=. --new=^/ ]]] And here is the output (svn 1.7.6): [[[ Issue: --old=A --new=B is not opposite of --old=B --new=A Running: svn di --old=^/ --new=. Index: a === --- a (.../file:///tmp/foo) (revision 2) +++ a (working copy) @@ -1 +1 @@ -r2 +new Running: svn di --old=. --new=^/ Index: a === --- a (working copy) +++ a (.../file:///tmp/foo) (revision 2) @@ -1 +1 @@ -r1 +r2 ]]] Regards, Alexey.
Re: FreeBSD project and subversion.
On Tuesday, February 05, 2013 12:59:43 PM Johan Corveleyn wrote: On Tue, Feb 5, 2013 at 1:39 AM, Alexey Neyman sti...@att.net wrote: On Monday, February 04, 2013 10:17:29 PM Stefan Sperling wrote: On Mon, Feb 04, 2013 at 11:54:21AM -0800, Alexey Neyman wrote: Is there a reason why it is necessary to use the svn diff --old ^/somebranch --new . instead of the intuitive svn ^/somebranch .? Would svn diff ^/somebranch . be synopsis 1? svn diff -rN:M ^/somebranch@HEAD .@HEAD It cannot be synopsis 1, as its description states that TARGETs may be all working copy paths or all URLs. In this case, we have one of each. Moreover, synopsis 1 says that for URLs, -r or -c must be present - and my example has no -r/-c option. Sorry, that's not true. From 'svn help diff' of 1.7: diff (di): Display the differences between two revisions or paths. usage: 1. diff [-c M | -r N[:M]] [TARGET[@REV]...] 2. diff [-r N[:M]] --old=OLD-TGT[@OLDREV] [--new=NEW-TGT[@NEWREV]] \ [PATH...] 3. diff OLD-URL[@OLDREV] NEW-URL[@NEWREV] So for usage 1 the -c and -r are optional. You didn't quote the second part of 'svn help diff' description of synopsis 1. Here it is: 1. Display the changes made to TARGETs as they are seen in REV between two revisions. TARGETs may be all working copy paths or all URLs. If TARGETs are working copy paths, N defaults to BASE and M to the working copy; if URLs, N must be specified and M defaults to HEAD. The '-c M' option is equivalent to '-r N:M' where N = M-1. Using '-c -M' does the reverse: '-r M:N' where N = M-1. If you read it, you'll see that (a) Mix of URLs and paths are not allowed for this synopsis (b) For URLs, either -r or -c must be provided Regards, Alexey. y.
Re: FreeBSD project and subversion.
On Tue, Feb 05, 2013 at 09:19:19AM -0800, Alexey Neyman wrote: There is one more weird issue with svn diff, see the script below. The issue is that --old=A --new=B is not opposite of --old=B --new=A. I don't know if it is a bug or another ambuguity I am not aware of :) Here is the script: [[[ #!/bin/bash REPO=/tmp/foo WC=/tmp/foo.wc rm -rf $REPO $WC svnadmin create $REPO svn co -q file://$REPO $WC cd $WC echo r1 a svn add -q a svn ci -q -m R1 echo r2 a svn ci -q -m R2 svn up -q -r 1 echo new a echo Issue: --old=A --new=B is not opposite of --old=B --new=A echo Running: svn di --old=^/ --new=. svn di --old=^/ --new=. echo Running: svn di --old=. --new=^/ svn di --old=. --new=^/ ]]] And here is the output (svn 1.7.6): [[[ Issue: --old=A --new=B is not opposite of --old=B --new=A Running: svn di --old=^/ --new=. Index: a === --- a (.../file:///tmp/foo) (revision 2) +++ a (working copy) @@ -1 +1 @@ -r2 +new Running: svn di --old=. --new=^/ Index: a === --- a (working copy) +++ a (.../file:///tmp/foo) (revision 2) @@ -1 +1 @@ -r1 +r2 ]]] Regards, Alexey. I can reproduce this with a trunk build. Can you please file an issue for this? I'm not going to get to this right away. Thanks!
Re: FreeBSD project and subversion.
On 02/05/2013 01:14 PM, Stefan Sperling wrote: On Tue, Feb 05, 2013 at 09:19:19AM -0800, Alexey Neyman wrote: There is one more weird issue with svn diff, see the script below. The issue is that --old=A --new=B is not opposite of --old=B --new=A. I don't know if it is a bug or another ambuguity I am not aware of :) Here is the script: [[[ #!/bin/bash REPO=/tmp/foo WC=/tmp/foo.wc rm -rf $REPO $WC svnadmin create $REPO svn co -q file://$REPO $WC cd $WC echo r1 a svn add -q a svn ci -q -m R1 echo r2 a svn ci -q -m R2 svn up -q -r 1 echo new a echo Issue: --old=A --new=B is not opposite of --old=B --new=A echo Running: svn di --old=^/ --new=. svn di --old=^/ --new=. echo Running: svn di --old=. --new=^/ svn di --old=. --new=^/ ]]] And here is the output (svn 1.7.6): [[[ Issue: --old=A --new=B is not opposite of --old=B --new=A Running: svn di --old=^/ --new=. Index: a === --- a (.../file:///tmp/foo) (revision 2) +++ a (working copy) @@ -1 +1 @@ -r2 +new Running: svn di --old=. --new=^/ Index: a === --- a (working copy) +++ a (.../file:///tmp/foo) (revision 2) @@ -1 +1 @@ -r1 +r2 ]]] Regards, Alexey. I can reproduce this with a trunk build. Can you please file an issue for this? I'm not going to get to this right away. Thanks! Here is the issue that I see: The --old=. get's the workspace version of ., but --new get's the non-changed version of /. So, I believe it is comparing r1 with the r2 contents of r2 and not comparing both workspace versions. RevContents 1r1 2r2 2* new 2* only get's referenced when it is in the --old position, 2 get's used when its in the --new position. Please correct me if I'm wrong about this, but that is what seems to be a logical reasoning behind that behavior. Lathan
Re: FreeBSD project and subversion.
On Tue, Feb 5, 2013 at 5:08 PM, Stefan Sperling s...@elego.de wrote: On Tue, Feb 05, 2013 at 12:59:43PM +0100, Johan Corveleyn wrote: Given what I said above, this can't be done in general (not if you give two URLs or two wc paths, because then both synopsis 1 and 2 are possible -- it would obviously break backwards compatibility if we would now start erroring out on these, while these used to work fine with automatically picking usage 1 before). Only if one of them is a URL and the other a WC path, because we give an error message then anyway. I think Alexey meant only the cases where we currently error out, i.e. where exactly two targets are given and one is a path and the other is a URL. I didn't realise this but there is indeed no ambiguity in allowing this syntax as a shorthand for a corresponding '--old X --new Y' diff invocation. Thanks Alexey, see http://svn.apache.org/r1442640 Making these invocations produce a diff instead of an error makes things less surprising for users. Hmm, okay, but I would have preferred some more discussion before you implemented this. It's not clear to me that this decreases the surprises. I can perfectly imagine just as many questions being asked on users@ with the question: Why does 'svn diff left.txt right.txt' give me an empty diff, while 'svn diff ^/left.txt right.txt' does the right thing? I think it would be better to consistently point users to the --old/--new syntax, so as to educate users more (so I'm okay with the change in the error message), not hide it even more from them. Speaking of the error message: On Tue, Feb 5, 2013 at 5:24 PM, Stefan Sperling s...@elego.de wrote: On Mon, Feb 04, 2013 at 04:39:32PM -0800, Alexey Neyman wrote: What adds oil to the fire is that the error message, which claims that Target lists to diff may not contain both working copy paths and URL. If instead it said that the supplied syntax is ambiguous and suggested to use --new/--old for disambiguation, this would have saved a lot of frustration and would eliminate the need of svn compare subcommand :) For example: svn: Ambiguous target list (contains both work copy paths and URLs). Use --old/--new syntax to disambiguate. Agreed. The error message wasn't very helpful. I've improved it in http://svn.apache.org/r1442645 based on your suggestion. Thanks! How is this useful after r1442640? Seems to me we should either improve the error message (which would be my preferred option) *or* make 'svn diff' handle these automatically as a shorthand syntax (hence no error message). But not both. Or am I missing something? -- Johan
Re: FreeBSD project and subversion.
On Tuesday, February 05, 2013 01:55:20 PM Lathan Bidwell wrote: On 02/05/2013 01:14 PM, Stefan Sperling wrote: On Tue, Feb 05, 2013 at 09:19:19AM -0800, Alexey Neyman wrote: There is one more weird issue with svn diff, see the script below. The issue is that --old=A --new=B is not opposite of --old=B --new=A. I don't know if it is a bug or another ambuguity I am not aware of :) Here is the script: [[[ #!/bin/bash REPO=/tmp/foo WC=/tmp/foo.wc rm -rf $REPO $WC svnadmin create $REPO svn co -q file://$REPO $WC cd $WC echo r1 a svn add -q a svn ci -q -m R1 echo r2 a svn ci -q -m R2 svn up -q -r 1 echo new a echo Issue: --old=A --new=B is not opposite of --old=B --new=A echo Running: svn di --old=^/ --new=. svn di --old=^/ --new=. echo Running: svn di --old=. --new=^/ svn di --old=. --new=^/ ]]] And here is the output (svn 1.7.6): [[[ Issue: --old=A --new=B is not opposite of --old=B --new=A Running: svn di --old=^/ --new=. Index: a === --- a (.../file:///tmp/foo) (revision 2) +++ a (working copy) @@ -1 +1 @@ -r2 +new Running: svn di --old=. --new=^/ Index: a === --- a (working copy) +++ a (.../file:///tmp/foo) (revision 2) @@ -1 +1 @@ -r1 +r2 ]]] Regards, Alexey. I can reproduce this with a trunk build. Can you please file an issue for this? I'm not going to get to this right away. Thanks! Here is the issue that I see: The --old=. get's the workspace version of ., but --new get's the non-changed version of /. So, I believe it is comparing r1 with the r2 contents of r2 and not comparing both workspace versions. RevContents 1r1 2r2 2* new 2* only get's referenced when it is in the --old position, 2 get's used when its in the --new position. Please correct me if I'm wrong about this, but that is what seems to be a logical reasoning behind that behavior. First, it is just the opposite: 2* gets referenced when it is in the --new, not --old position. Second, it is not the reasoning (why it behaves so) but rather a description of current behavior, which I think was rather obvious from the example script. The reason for this behavior is that the code in diff-cmd.c makes the following defaults for the old/new revisions: if (opt_state-start_revision.kind == svn_opt_revision_unspecified) opt_state-start_revision.kind = svn_path_is_url(old_target) ? svn_opt_revision_head : svn_opt_revision_base; if (opt_state-end_revision.kind == svn_opt_revision_unspecified) opt_state-end_revision.kind = svn_path_is_url(new_target) ? svn_opt_revision_head : svn_opt_revision_working; These defaults make some sense, given that new target is optional and defaults to old target if not specified. If new target is specified, and is different from from old target, I am not so sure if it makes sense. Note that there is a special case if both old/new targets are WC paths - in that case, both old and new targets default to svn_opt_revision_working. So, $ svn diff --old=foo --new=bar will compare modified version of 'foo' against modified version of 'bar' [*], but $ svn diff --old=foo --new=^/bar would compare base version of 'foo' against HEAD version of 'bar. Does it make sense? I would say, if both old and new are specified, old target's revision should default to 'working' as well. Problem is further aggravated since there is no commandline alias to request svn_opt_revision_working setting from the command line: only 'head', 'prev', 'base' and 'committed' are currently available. Hence, it is not possible to request the exact opposite of the patch using -rN:M syntax if one of the -- old/--new targets is a working copy path. Is there a reason why 'working' is not parsed by revision_from_word()? Also note that 'svn help diff' does not describe revision defaults for synopsis 2 (and the default is different from synopsis 1, which requires old revision to be specified for URLs). PS. Stephan apparently made 'working' revision as default in his check-in (r1442640) at least for the short-hand form, but Julian Foad, optimizing the logic in r1442659 and r1442676, switched it back to be the same as --old/-- new logic. PPS. If agreed to suggested change of default, the patch is attached. [[[ Make svn diff --old=.. --new=.. default to WORKING revision for old target if new target is explicitly specified. * subversion/svn/diff-cmd.c (svn_cl__diff) Change defaults as described and simplify the logic. ]]] Regards, Alexey.Index: subversion/svn/diff-cmd.c === --- subversion/svn/diff-cmd.c (revision 1442686) +++ subversion/svn/diff-cmd.c (working copy) @@ -238,10 +238,11 @@
Re: FreeBSD project and subversion.
On Tue, Feb 05, 2013 at 09:18:33PM +0100, Johan Corveleyn wrote: Hmm, okay, but I would have preferred some more discussion before you implemented this. It's not clear to me that this decreases the surprises. I can perfectly imagine just as many questions being asked on users@ with the question: Why does 'svn diff left.txt right.txt' give me an empty diff, while 'svn diff ^/left.txt right.txt' does the right thing? I think it would be better to consistently point users to the --old/--new syntax, so as to educate users more (so I'm okay with the change in the error message), not hide it even more from them. Well, the point that convinced me was that previously 'svn diff' raised an error in the case of 'svn diff ^/left.txt right.txt'. That's clearly intended to be a comparison between a URL and a path, which the diff code already supports. So I think the diff code should just show an appropriate diff. I don't see how people could be conflating a URL-WC or WC-URL diff with a WC-WC diff. BTW, I'm thinking about adding more shortcuts: svn diff UNVERSIONED-PATH WCPATH svn diff WCPATH UNVERSIONED-PATH svn diff UNVERSIONED-PATH1 UNVERSIONED-PATH2 All of which are supported by 'svn diff --old=X --new=Y' (in 1.8-to-be, not in 1.7), but not by plain 'svn diff'. The --old --new syntax is specific to Subversion. Many users who are accustomed to standard diff commands will not try using --old and --new (unless they've taken the time to read 'svn help diff', of course, which apparently is too much time to ask from some users ;) That said, if you'd rather just print the error message instead, that's fine with me, too. I don't care strongly enough about this to the point where I'd say that we _must_ make this change to 'svn diff'. It's been committed, but that doesn't mean that this has to be in 1.8.0 at all costs. Speaking of the error message: How is this useful after r1442640? Seems to me we should either improve the error message (which would be my preferred option) *or* make 'svn diff' handle these automatically as a shorthand syntax (hence no error message). But not both. Or am I missing something? The error message is triggered by an invocation like: svn diff -c42 foo ^/trunk bar Which is clearly invalid usage according to 'svn help diff'.
Re: FreeBSD project and subversion.
On Wed, Feb 6, 2013 at 12:07 AM, Stefan Sperling s...@apache.org wrote: On Tue, Feb 05, 2013 at 09:18:33PM +0100, Johan Corveleyn wrote: Hmm, okay, but I would have preferred some more discussion before you implemented this. It's not clear to me that this decreases the surprises. I can perfectly imagine just as many questions being asked on users@ with the question: Why does 'svn diff left.txt right.txt' give me an empty diff, while 'svn diff ^/left.txt right.txt' does the right thing? I think it would be better to consistently point users to the --old/--new syntax, so as to educate users more (so I'm okay with the change in the error message), not hide it even more from them. Well, the point that convinced me was that previously 'svn diff' raised an error in the case of 'svn diff ^/left.txt right.txt'. That's clearly intended to be a comparison between a URL and a path, which the diff code already supports. So I think the diff code should just show an appropriate diff. I don't see how people could be conflating a URL-WC or WC-URL diff with a WC-WC diff. BTW, I'm thinking about adding more shortcuts: svn diff UNVERSIONED-PATH WCPATH svn diff WCPATH UNVERSIONED-PATH svn diff UNVERSIONED-PATH1 UNVERSIONED-PATH2 All of which are supported by 'svn diff --old=X --new=Y' (in 1.8-to-be, not in 1.7), but not by plain 'svn diff'. The --old --new syntax is specific to Subversion. Many users who are accustomed to standard diff commands will not try using --old and --new (unless they've taken the time to read 'svn help diff', of course, which apparently is too much time to ask from some users ;) That said, if you'd rather just print the error message instead, that's fine with me, too. I don't care strongly enough about this to the point where I'd say that we _must_ make this change to 'svn diff'. It's been committed, but that doesn't mean that this has to be in 1.8.0 at all costs. Nah, it's fine, I think I'm starting to get convinced ;-). But it's still a pity that there is no relief (at least I don't see any) for confusion around the fact that 'svn diff WC-PATH1 WC-PATH2' doesn't diff WC-PATH1 and WC-PATH2 against each other. The shorthands with the unversioned paths are quite nice I think, but there is an additional risk of confusion: 'svn diff UNVERSIONED-PATH1 UNVERSIONED-PATH2' will mean something totally different than 'svn diff WC-PATH1 WC-PATH2' (even though they could both refer to the same local files, but the commands are separated by an invocation of 'svn add'). Speaking of the error message: How is this useful after r1442640? Seems to me we should either improve the error message (which would be my preferred option) *or* make 'svn diff' handle these automatically as a shorthand syntax (hence no error message). But not both. Or am I missing something? The error message is triggered by an invocation like: svn diff -c42 foo ^/trunk bar Which is clearly invalid usage according to 'svn help diff'. Yes, but in this case the user gives 3 targets, so he clearly couldn't have meant one of the shorthand syntaxes, right? -- Johan
Re: FreeBSD project and subversion.
On Wed, Feb 06, 2013 at 12:30:05AM +0100, Johan Corveleyn wrote: Nah, it's fine, I think I'm starting to get convinced ;-). But it's still a pity that there is no relief (at least I don't see any) for confusion around the fact that 'svn diff WC-PATH1 WC-PATH2' doesn't diff WC-PATH1 and WC-PATH2 against each other. I agree very much that it is rather confusing that svn diff WC-PATH1 WC-PATH1 does something else than: svn diff --old WC-PATH1 --new WC-PATH2 However, I think that the current help text makes the difference clear, if only the user cares to read it. As long as the documentation is clear enough, such subtleties are fine with me. I mean that I'll probably be able to remember them and explain them to people -- which is what I'm supposed to be doing at $DAYJOB -- hence that's why I'm fine with it :) Joke aside, I'm not sure if there is a really good way to resolve these ambiguities. I don't really like that fact that we've got these --old and --new options in the first place. It seems like a crutch for a lack of a better syntax that can express all use cases equally well without being rather weird and uncommon. But since the syntax is present in releases we have to keep supporting it... The shorthands with the unversioned paths are quite nice I think, but there is an additional risk of confusion: 'svn diff UNVERSIONED-PATH1 UNVERSIONED-PATH2' will mean something totally different than 'svn diff WC-PATH1 WC-PATH2' (even though they could both refer to the same local files, but the commands are separated by an invocation of 'svn add'). Yeah, it is going to make things even more confusing, of course! But only for those who type the diff command and expect a different result than they're getting :) The error message is triggered by an invocation like: svn diff -c42 foo ^/trunk bar Which is clearly invalid usage according to 'svn help diff'. Yes, but in this case the user gives 3 targets, so he clearly couldn't have meant one of the shorthand syntaxes, right? Only if the user did not understand the (somewhat obscure by nature) help text... The warning is really meant to get people who provide nonsense arguments to read 'svn help diff'.
Re: FreeBSD project and subversion.
On Thursday, January 31, 2013 09:08:20 am Stefan Sperling wrote: BTW, I went over one of FreeBSD's svn wiki pages a while back, and added answers to open questions on this page: https://wiki.freebsd.org/SubversionMissing Speaking of which: Is there a reason why it is necessary to use the svn diff --old ^/somebranch --new . instead of the intuitive svn ^/somebranch .? It has bugged me for a while until I discovered this --old/--new trick. Regards, Alexey.
Re: FreeBSD project and subversion.
On Mon, Feb 04, 2013 at 11:54:21AM -0800, Alexey Neyman wrote: On Thursday, January 31, 2013 09:08:20 am Stefan Sperling wrote: BTW, I went over one of FreeBSD's svn wiki pages a while back, and added answers to open questions on this page: https://wiki.freebsd.org/SubversionMissing Speaking of which: Is there a reason why it is necessary to use the svn diff --old ^/somebranch --new . instead of the intuitive svn ^/somebranch .? Would svn diff ^/somebranch . be synopsis 1? svn diff -rN:M ^/somebranch@HEAD .@HEAD (i.e. svn diff -rN:M ^/somebranch@HEAD; svn diff -rN:M .@HEAD; show the differences between rN to rM for ^/somebranch@HEAD and then show the differences between rN and rM for .@HEAD -- where N defaults to BASE if the target is a working copy path, and M defaults to HEAD) Note that you can also specify 3 or more targets: svn diff -rN:M ^/somebranch@HEAD .@HEAD ^/someotherbranch@42 ... Or would it be synopsis 2: svn diff --old ^/somebranch@HEAD --new .@HEAD (i.e. show the difference between the two (path, revision) pairs somebranch@HEAD and .@HEAD) ? It has bugged me for a while until I discovered this --old/--new trick. I agree that --old --new syntax isn't intuitive at all. But I believe it exists to resolve the above ambiguity. The only remedy that comes to my mind is to introduce a new subcommand: svn compare TARGET1 TARGET2 which would equal svn diff --old TARGET1 TARGET2 I'm not sure if the community would like to see yet another new subcommand though :)
Re: FreeBSD project and subversion.
On Monday, February 04, 2013 10:17:29 PM Stefan Sperling wrote: On Mon, Feb 04, 2013 at 11:54:21AM -0800, Alexey Neyman wrote: On Thursday, January 31, 2013 09:08:20 am Stefan Sperling wrote: BTW, I went over one of FreeBSD's svn wiki pages a while back, and added answers to open questions on this page: https://wiki.freebsd.org/SubversionMissing Speaking of which: Is there a reason why it is necessary to use the svn diff --old ^/somebranch --new . instead of the intuitive svn ^/somebranch .? Would svn diff ^/somebranch . be synopsis 1? svn diff -rN:M ^/somebranch@HEAD .@HEAD It cannot be synopsis 1, as its description states that TARGETs may be all working copy paths or all URLs. In this case, we have one of each. Moreover, synopsis 1 says that for URLs, -r or -c must be present - and my example has no -r/-c option. So, the only possible use of this syntax would be synopsis 2: svn diff --old ^/somebranch@HEAD --new .@HEAD (i.e. show the difference between the two (path, revision) pairs somebranch@HEAD and .@HEAD) ? It has bugged me for a while until I discovered this --old/--new trick. I agree that --old --new syntax isn't intuitive at all. What adds oil to the fire is that the error message, which claims that Target lists to diff may not contain both working copy paths and URL. If instead it said that the supplied syntax is ambiguous and suggested to use --new/--old for disambiguation, this would have saved a lot of frustration and would eliminate the need of svn compare subcommand :) For example: svn: Ambiguous target list (contains both work copy paths and URLs). Use --old/--new syntax to disambiguate. Regards, Alexey.
Re: FreeBSD project and subversion.
/man/man4/geom_map.4 D share/man/man4/geom_map.4 $ svn resolved share/man/man4/geom_map.4 $ svn status M . M share/man/man4/Makefile D share/man/man4/geom_map.4 Now we can merge r221501 again, this time in the forward direction of history, to bring in the file we actually want: $ svn merge -cr221501 ^/head --- Merging r221501 into '.': Gshare/man/man4/Makefile Ashare/man/man4/geom_map.4 --- Recording mergeinfo for merge of r221501 into '.': G . $ svn st R +share/man/man4/geom_map.4 $ So the changeset scheduled for commit now now is a replacement of the geom_map.4 file with its cousin which has the proper line of history: $ svn info share/man/man4/geom_map.4 | grep ^Copied\ From Copied From URL: svn://svn.freebsd.org/base/head/share/man/man4/geom_map.4 Copied From Rev: 221501 Since the largeSMP branch has already been merged back to ^/head, the mistake now needs to be fixed up on ^/head. This could again be done via appropriate reverse-merges and re-merges, but there's an easier way. We can reconnect the file to its proper line of history by deleting the file with the bad line of history and replacing it with the file with the good line of history. First, let's check for differences between the latest version of the file in head and the version from before the largeSMP branch was reintegrated: $ svn diff ^/head/share/man/man4/geom_map.4 ^/head/share/man/man4/geom_map.4@r222012 Index: geom_map.4 === --- geom_map.4 (revision 246254) +++ geom_map.4 (revision 222012) Property changes on: geom_map.4 ___ Deleted: svn:eol-style ## -1 +0,0 ## -native \ No newline at end of property Deleted: svn:mime-type ## -1 +0,0 ## -text/plain \ No newline at end of property The only information we need to manually preserve are these two properties. That's easy: $ svn rm share/man/man4/geom_map.4 D share/man/man4/geom_map.4 $ svn copy ^/head/share/man/man4/com_map.4@r222012 share/man/man4/geom_map.4 A share/man/man4/geom_map.4 1.7.x-power $ svn status RM +share/man/man4/geom_map.4 $ svn propset svn:eol-style native share/man/man4/geom_map.4 property 'svn:eol-style' set on 'share/man/man4/geom_map.4' $ svn propset svn:mime-type text/plain share/man/man4/geom_map.4 property 'svn:mime-type' set on 'share/man/man4/geom_map.4' $ svn status RM +share/man/man4/geom_map.4 $ svn diff Index: share/man/man4/geom_map.4 === --- share/man/man4/geom_map.4 (revision 246254) +++ share/man/man4/geom_map.4 (working copy) Property changes on: share/man/man4/geom_map.4 ___ Added: svn:mergeinfo Merged /projects/quota64/share/man/man4/geom_map.4:r184125-207707 Merged /vendor/resolver/dist/share/man/man4/geom_map.4:r1540-186085 Now the only property changes are svn:mergeinfo changes. We don't really care about these, but always commit them!!! Notice how log history has been repaired: $ svn log share/man/man4/geom_map.4 r222012 | uqs | 2011-05-17 11:51:02 +0200 (Tue, 17 May 2011) | 4 lines More thorough mdoc and language fixes. Submitted by: ru r222008 | uqs | 2011-05-17 10:12:59 +0200 (Tue, 17 May 2011) | 2 lines Typos, wording and mdoc fixes. r221501 | adrian | 2011-05-05 16:43:35 +0200 (Thu, 05 May 2011) | 4 lines Add a manpage for geom_map(4). Submitted by: r...@dlink.ua I would just generate a diff and manually apply that to a HEAD checkout and commit that. You could perhaps svn cp over new files from the nand branch to HEAD to preserve their history, but I worry that anything other than diff + patch for existing files risks hosing history. WOAH!! Please lets gain some new experience with 'svn merge' using version 1.7. We do 100's of merges a year at $WORK (with svn 1.6) on a code base 10x that of FreeBSD -- it works. I've never had problems with merging downstream into work branches. I've only seen upstream merges blow up. -- John Baldwin -- This mail is for the internal use of the FreeBSD project committers, and as such is private. This mail may not be published or forwarded outside the FreeBSD committers' group or disclosed to other unauthorised parties without the explicit permission of the author(s). Date: Fri, 1 Feb 2013 09:50:37 -0500 From: John Baldwin j...@freebsd.org To: Alfred Perlstein bri...@mu.org Cc: Peter Wemm pe...@wemm.org Subject: Re: Fwd: Re: FreeBSD project and subversion. X-Spam-Level: User-Agent: KMail/1.13.5 (FreeBSD/8.2
Re: FreeBSD project and subversion.
On 1/31/13 12:08 PM, Stefan Sperling wrote: On Thu, Jan 31, 2013 at 10:37:14AM -0500, Alfred Perlstein wrote: FreeBSD has moved to Subversion a few years ago and it's worked very, very well for us. Thanks! That's encouraging to hear. The one area where we are having issues is merging code from project branches back into trunk. The typical workflow is: 1) create project branch. 2) code code code. 3) sync from HEAD (this works great). 4) repeat steps 2+3 until feature is complete. 5) svn diff and apply to head then commit. Is there a way to do 5 automatically? I think the worry is mergeinfo from the project branch coming back into HEAD. Any tips would be appreciated. I've read the FreeBSD svn merging docs some time ago. I'm not sure if changes have been made since, but it was probably an older version of what currently lives at this URL: http://www.freebsd.org/doc/en_US.ISO8859-1/articles/committers-guide/subversion-primer.html Back then I was somewhat worried that apparently the person who wrote them didn't really understand much about how merges in Subversion work. There was irrational fear of mergeinfo propagation, to the point where the recommended practice is to remove mergeinfo before commit, which any Subversion user with a clue will know is very wrong (expect in very exceptional circumstances and only if you are equipped with sufficient expertise to deal with the consequences). What surprised me also was a complete lack of mention of the --reintegrate option, which I suspect must be causing quite a lot of grief among FreeBSD developers due to bogus conflicts during merges back into FreeBSD's head branch (i.e. the trunk). We'll need more details to help you in a constructive way, though. Can you provide more details about your steps 1 to 5, i.e. show the exact commands you're running in each step? The repository is public, after all, which will help greatly with identifying and reproducing specific problems. I'm happy to provide input for improving FreeBSD's docs to the point where FreeBSD makes the best possible use of Subversion 1.7's merge implementation, and can also provide some hints as to how future versions of Subversion will improve to make life easier in certain cases. BTW, I went over one of FreeBSD's svn wiki pages a while back, and added answers to open questions on this page: https://wiki.freebsd.org/SubversionMissing Thank you Stefan, So I have two answers here: 1) about mergeinfo It seems as if doing it all at the top can make merges over long haul internet very painful. 2) about reintegrate I've attached the two messages that show what makes FreeBSD gun shy about merges to HEAD. Maybe these issues are resolved, or maybe the developer did something incorrect, or maybe something else entirely different happened. See attached. Thank you, -Alfred ---BeginMessage--- -- John Baldwin ---BeginMessage--- On Friday, June 01, 2012 1:40:29 pm David O'Brien wrote: On Wed, May 16, 2012 at 11:00:48AM -0400, John Baldwin wrote: I more or less don't trust svn merge to DTRT here. This was done with the cpuset branch merge up to HEAD and it broke the log history of many files in HEAD. Specifically how did it break log history? http://svnweb.freebsd.org/base/head/share/man/man4/geom_map.4?view=log You have to walk up to the previous directory in svnweb and go back to change 222812 and then click on geom_map.4 to find its original log. sys/dev/iicbus/ad7417.c was also busted this way. I would just generate a diff and manually apply that to a HEAD checkout and commit that. You could perhaps svn cp over new files from the nand branch to HEAD to preserve their history, but I worry that anything other than diff + patch for existing files risks hosing history. WOAH!! Please lets gain some new experience with 'svn merge' using version 1.7. We do 100's of merges a year at $WORK (with svn 1.6) on a code base 10x that of FreeBSD -- it works. I've never had problems with merging downstream into work branches. I've only seen upstream merges blow up. -- John Baldwin -- This mail is for the internal use of the FreeBSD project committers, and as such is private. This mail may not be published or forwarded outside the FreeBSD committers' group or disclosed to other unauthorised parties without the explicit permission of the author(s). ---End Message--- ---End Message--- ---BeginMessage--- On Friday, February 01, 2013 7:53:57 am Alfred Perlstein wrote: John and Peter, both of you are +inf more knowledgeable about svn than I am. I see we still try to minimize svn mergeinfo from the FAQ (Selecting the Source and Target) http://www.freebsd.org/doc/en_US.ISO8859-1/articles/committers- guide/subversion-primer.html#AEN771 I know I've seen some emails explaining the reasoning behind this but I can't find them. What would the effect of bringing mergeinfo to the top be? Possible problems: 1) it would get very large 2) if we
FreeBSD project and subversion.
FreeBSD has moved to Subversion a few years ago and it's worked very, very well for us. The one area where we are having issues is merging code from project branches back into trunk. The typical workflow is: 1) create project branch. 2) code code code. 3) sync from HEAD (this works great). 4) repeat steps 2+3 until feature is complete. 5) svn diff and apply to head then commit. Is there a way to do 5 automatically? I think the worry is mergeinfo from the project branch coming back into HEAD. Any tips would be appreciated. thank you, -Alfred
Re: FreeBSD project and subversion.
On Thu, Jan 31, 2013 at 10:37:14AM -0500, Alfred Perlstein wrote: FreeBSD has moved to Subversion a few years ago and it's worked very, very well for us. Thanks! That's encouraging to hear. The one area where we are having issues is merging code from project branches back into trunk. The typical workflow is: 1) create project branch. 2) code code code. 3) sync from HEAD (this works great). 4) repeat steps 2+3 until feature is complete. 5) svn diff and apply to head then commit. Is there a way to do 5 automatically? I think the worry is mergeinfo from the project branch coming back into HEAD. Any tips would be appreciated. I've read the FreeBSD svn merging docs some time ago. I'm not sure if changes have been made since, but it was probably an older version of what currently lives at this URL: http://www.freebsd.org/doc/en_US.ISO8859-1/articles/committers-guide/subversion-primer.html Back then I was somewhat worried that apparently the person who wrote them didn't really understand much about how merges in Subversion work. There was irrational fear of mergeinfo propagation, to the point where the recommended practice is to remove mergeinfo before commit, which any Subversion user with a clue will know is very wrong (expect in very exceptional circumstances and only if you are equipped with sufficient expertise to deal with the consequences). What surprised me also was a complete lack of mention of the --reintegrate option, which I suspect must be causing quite a lot of grief among FreeBSD developers due to bogus conflicts during merges back into FreeBSD's head branch (i.e. the trunk). We'll need more details to help you in a constructive way, though. Can you provide more details about your steps 1 to 5, i.e. show the exact commands you're running in each step? The repository is public, after all, which will help greatly with identifying and reproducing specific problems. I'm happy to provide input for improving FreeBSD's docs to the point where FreeBSD makes the best possible use of Subversion 1.7's merge implementation, and can also provide some hints as to how future versions of Subversion will improve to make life easier in certain cases. BTW, I went over one of FreeBSD's svn wiki pages a while back, and added answers to open questions on this page: https://wiki.freebsd.org/SubversionMissing One more point, which I like to bring up whenever a FreeBSD person shows up on our lists: There is an outstanding patch in the FreeBSD ports tree which should be shepherded into Subversion's upstream sources. I'm happy to be the upstream point of contact for this but we also need someone from the FreeBSD project to drive this and digest our feedback. Because we want this feature to be generally usable and also avoid breaking functionality that FreeBSD is relying on in their custom patch, of course. So both sides need to be involved. See here for details: http://subversion.tigris.org/issues/show_bug.cgi?id=890 (and also the mailing list threads linked from the issue)
