Re: svn merge --reintegrate like diff

[Stefan Sperling]

On Sun, Oct 02, 2016 at 12:16:27PM -0700, Alexey Neyman wrote:
> On 10/01/2016 10:59 PM, Stefan Sperling wrote:
> > On Sat, Oct 01, 2016 at 10:19:34PM -0700, Alexey Neyman wrote:
> > > On 09/28/2016 09:49 AM, Stefan Sperling wrote:
> > > > Hi Alexey,
> > > > 
> > > > Could you compile an SVN client from trunk and try some merges with it,
> > > > and let me know how the merging of moves with the new conflict resolver
> > > > (which is still work-in-progress) is working out for you?
> > > > My goal is to make scripts like yours unnecessary.
> > > > 
> > > > The current implementation does not yet detect moves which happened
> > > > inside copies, but I hope to get that fixed before release.
> > > > 
> > > > Thanks,
> > > > Stefan
> > > I gave it a try (r1763039) and it is not different from what I see with
> > > 1.9.x: the files that were renamed on the branch are still copied from the
> > > branch, not renamed on the trunk.
> > > I.e.,
> > > 
> > > svn cp $SVNREPO/trunk $SVNREPO/branch/x
> > > svn co $SVNREPO/branch/x
> > > cd x
> > > svn mv foo.c bar.c
> > > vi bar.c
> > > svn ci
> > > cd ..
> > > rm -rf x
> > > svn co $SVNREPO/trunk
> > > cd trunk
> > > svn merge ^/branch/x
> > > svn info bar.c
> > > 
> > > The last command shows bar.c as being copied, without any changes, from
> > > ^/branch/x/bar.c - rather than being copied from ^/trunk/bar.c and 
> > > modified.
> > > And, since there are no changes in the diff, ReviewBoard shows nothing in
> > > the diff for bar.c.
> > You'll have to produce some kind of tree conflict involving the renamed 
> > file.
> > The run 'svn resolve' (or use the conflict prompt 'svn merge' opens for 
> > you).
> > 
> > To be clear, 1.10.x will *not* change 'svn merge'.
> > It changes post-merge behaviour during tree conflict resolution only.
> > 
> > I'm afraid this won't fix your problem with ReviewBoard.
> Well, tree conflicts during merge is a separate pain point with SVN, but

OK. I realize we're not really talking about the same set of problems then :)

> here I was referring specifically to the reviewability of the merges - so
> 1.10 will not make this script obsolete, unfortunately. Perhaps, the change
> in merge behavior can be made conditional on some command line option, e.g.
> 'svn merge --replay-merged-renames'?

There are no such plans. Changing how 'svn merge' works is... not impossible,
but rather difficult, especially when it comes to moves and renames.
Julian Foad has done a lot of research into this:
http://wiki.apache.org/subversion/MoveDev/MoveDev

> Also, I was just made aware that the script I posted has a shortcoming if
> the branch being merged has been previously merged into trunk, and then had
> seen more development on the branch (i.e., it is a branch for ongoing
> development, not "merge-and-delete" branch). Thing is, it is not very
> straightforward with Subversion to discover *where this file has been copied
> to*. For example, "I have a file foo.c in revision 111; under which name(s)
> is this file appearing in HEAD?" I think adding an ability to obtain this
> kind of information has been discussed in the past - is it on the table for
> 1.10?

No, that's not on the table for 1.10 AFAIK.

Back in 2011 I did some work on this problem together with Daniel Shahaf.
But it never went anywhere because we found that it didn't help the goal
we were pursuing at the time, which was better tree conflict resolution.
See the fs-successor-ids branch:
http://svn.apache.org/repos/asf/subversion/branches/fs-successor-ids/BRANCH-README
Of course, there may be other valid use cases, in which case this branch
could serve as a starting point. The design is done, and most of the
implementation is done, too. Both would need to be updated to account for
changes made since the (biggest relevant change being FSFS format 7).

I'm sorry to disappoint you. It seems your best path forward is to keep
scripting. Unless of course you'd like to try fixing these problems in
core SVN together with us, and drive development of the features you need.

Re: svn merge --reintegrate like diff

[Alexey Neyman]

On 10/01/2016 10:59 PM, Stefan Sperling wrote:

On Sat, Oct 01, 2016 at 10:19:34PM -0700, Alexey Neyman wrote:

On 09/28/2016 09:49 AM, Stefan Sperling wrote:

Hi Alexey,

Could you compile an SVN client from trunk and try some merges with it,
and let me know how the merging of moves with the new conflict resolver
(which is still work-in-progress) is working out for you?
My goal is to make scripts like yours unnecessary.

The current implementation does not yet detect moves which happened
inside copies, but I hope to get that fixed before release.

Thanks,
Stefan

I gave it a try (r1763039) and it is not different from what I see with
1.9.x: the files that were renamed on the branch are still copied from the
branch, not renamed on the trunk.
I.e.,

svn cp $SVNREPO/trunk $SVNREPO/branch/x
svn co $SVNREPO/branch/x
cd x
svn mv foo.c bar.c
vi bar.c
svn ci
cd ..
rm -rf x
svn co $SVNREPO/trunk
cd trunk
svn merge ^/branch/x
svn info bar.c

The last command shows bar.c as being copied, without any changes, from
^/branch/x/bar.c - rather than being copied from ^/trunk/bar.c and modified.
And, since there are no changes in the diff, ReviewBoard shows nothing in
the diff for bar.c.

You'll have to produce some kind of tree conflict involving the renamed file.
The run 'svn resolve' (or use the conflict prompt 'svn merge' opens for you).

To be clear, 1.10.x will *not* change 'svn merge'.
It changes post-merge behaviour during tree conflict resolution only.

I'm afraid this won't fix your problem with ReviewBoard.
Well, tree conflicts during merge is a separate pain point with SVN, but 
here I was referring specifically to the reviewability of the merges - 
so 1.10 will not make this script obsolete, unfortunately. Perhaps, the 
change in merge behavior can be made conditional on some command line 
option, e.g. 'svn merge --replay-merged-renames'?


Also, I was just made aware that the script I posted has a shortcoming 
if the branch being merged has been previously merged into trunk, and 
then had seen more development on the branch (i.e., it is a branch for 
ongoing development, not "merge-and-delete" branch). Thing is, it is not 
very straightforward with Subversion to discover *where this file has 
been copied to*. For example, "I have a file foo.c in revision 111; 
under which name(s) is this file appearing in HEAD?" I think adding an 
ability to obtain this kind of information has been discussed in the 
past - is it on the table for 1.10?


Regards,
Alexey.

Re: svn merge --reintegrate like diff

[Stefan Sperling]

On Sat, Oct 01, 2016 at 10:19:34PM -0700, Alexey Neyman wrote:
> On 09/28/2016 09:49 AM, Stefan Sperling wrote:
> > Hi Alexey,
> > 
> > Could you compile an SVN client from trunk and try some merges with it,
> > and let me know how the merging of moves with the new conflict resolver
> > (which is still work-in-progress) is working out for you?
> > My goal is to make scripts like yours unnecessary.
> > 
> > The current implementation does not yet detect moves which happened
> > inside copies, but I hope to get that fixed before release.
> > 
> > Thanks,
> > Stefan
> I gave it a try (r1763039) and it is not different from what I see with
> 1.9.x: the files that were renamed on the branch are still copied from the
> branch, not renamed on the trunk.
> I.e.,
> 
> svn cp $SVNREPO/trunk $SVNREPO/branch/x
> svn co $SVNREPO/branch/x
> cd x
> svn mv foo.c bar.c
> vi bar.c
> svn ci
> cd ..
> rm -rf x
> svn co $SVNREPO/trunk
> cd trunk
> svn merge ^/branch/x
> svn info bar.c
> 
> The last command shows bar.c as being copied, without any changes, from
> ^/branch/x/bar.c - rather than being copied from ^/trunk/bar.c and modified.
> And, since there are no changes in the diff, ReviewBoard shows nothing in
> the diff for bar.c.
> 
> Regards,
> Alexey.
> > 
> 

You'll have to produce some kind of tree conflict involving the renamed file.
The run 'svn resolve' (or use the conflict prompt 'svn merge' opens for you).

To be clear, 1.10.x will *not* change 'svn merge'.
It changes post-merge behaviour during tree conflict resolution only.

I'm afraid this won't fix your problem with ReviewBoard.

Re: svn merge --reintegrate like diff

[Stefan Sperling]

On Tue, Sep 27, 2016 at 10:12:28AM -0700, Alexey Neyman wrote:
> On 09/27/2016 01:46 AM, Daniel Shahaf wrote:
> > Johan Corveleyn wrote on Mon, Sep 26, 2016 at 13:04:04 +0200:
> > > Maybe I'm missing something, but I don't understand why 'svn diff
> > > --old=TRUNK --new=BRANCH' would show you things that you previously
> > > merged from TRUNK to BRANCH. It should show exactly the content-wise
> > > difference between TRUNK and BRANCH, so if some content was merged
> > > from TRUNK to BRANCH, both should be identical on that point, and it
> > > shouldn't show up in 'diff'.
> > That command would also show changes made on trunk that have not yet been
> > merged to the branch.  (E.g., if you ran it in on subversion's trunk and
> > 1.9.x branch, it would show -SVN_VER_MINOR 10\n +SVN_VER_MINOR 9\n.)
> > 
> > The OP asked for the changes merge would do, which is approximately
> > --old=TRUNK@REV --new=BRANCH
> > where REV is the youngest revision of trunk merged to the branch.
> > ("Approximately" because this is inaccurate when cherry-picks or subtree
> > merges hapepned.)
> There's one more issue with these approaches. ReviewBoard can be smart about
> displaying the moved/copied files. However:
> 
> - If one does 'svn merge --reintegrate', Subversion will copy new files from
> the branch unchanged, and 'svn diff' will not show them (and hence, RB won't
> either) - or, with --show-copies-as-adds, it will show them as being added
> anew. For the review purposes, it would be better if instead of copying the
> file from the branch unchanged, Subversion would perform the same move on
> the trunk and apply the textual changes.
> - If you do 'svn diff --old=... --new=...', I believe the copy-from
> information is lost from the diff completely - and ReviewBoard will show all
> the moved files as adds/deletes, not showing diffs either.
> 
> For now, I am using the attached script to perform this task. The workflow
> is:
> 1. Perform a merge to trunk.
> 2. Run the script (which attempts to find the original location in trunk for
> every copied file and replay the move on trunk).
> 3. 'rbt post'.
> 
> The script is not perfect; it only operates at file level - so if there are
> renamed directories, the files inside them end up in 'R +' status (replaced
> with history) and ReviewBoard shows a spurious deletion for this file, in
> addition to a move/copy with changes.
> 
> Regards,
> Alexey.
> 

Hi Alexey,

Could you compile an SVN client from trunk and try some merges with it,
and let me know how the merging of moves with the new conflict resolver
(which is still work-in-progress) is working out for you?
My goal is to make scripts like yours unnecessary.

The current implementation does not yet detect moves which happened
inside copies, but I hope to get that fixed before release.

Thanks,
Stefan

Re: svn merge --reintegrate like diff

[Alexey Neyman]

On 09/27/2016 01:46 AM, Daniel Shahaf wrote:

Johan Corveleyn wrote on Mon, Sep 26, 2016 at 13:04:04 +0200:

Maybe I'm missing something, but I don't understand why 'svn diff
--old=TRUNK --new=BRANCH' would show you things that you previously
merged from TRUNK to BRANCH. It should show exactly the content-wise
difference between TRUNK and BRANCH, so if some content was merged
from TRUNK to BRANCH, both should be identical on that point, and it
shouldn't show up in 'diff'.

That command would also show changes made on trunk that have not yet been
merged to the branch.  (E.g., if you ran it in on subversion's trunk and
1.9.x branch, it would show -SVN_VER_MINOR 10\n +SVN_VER_MINOR 9\n.)

The OP asked for the changes merge would do, which is approximately
--old=TRUNK@REV --new=BRANCH
where REV is the youngest revision of trunk merged to the branch.
("Approximately" because this is inaccurate when cherry-picks or subtree
merges hapepned.)
There's one more issue with these approaches. ReviewBoard can be smart 
about displaying the moved/copied files. However:


- If one does 'svn merge --reintegrate', Subversion will copy new files 
from the branch unchanged, and 'svn diff' will not show them (and hence, 
RB won't either) - or, with --show-copies-as-adds, it will show them as 
being added anew. For the review purposes, it would be better if instead 
of copying the file from the branch unchanged, Subversion would perform 
the same move on the trunk and apply the textual changes.
- If you do 'svn diff --old=... --new=...', I believe the copy-from 
information is lost from the diff completely - and ReviewBoard will show 
all the moved files as adds/deletes, not showing diffs either.


For now, I am using the attached script to perform this task. The 
workflow is:

1. Perform a merge to trunk.
2. Run the script (which attempts to find the original location in trunk 
for every copied file and replay the move on trunk).

3. 'rbt post'.

The script is not perfect; it only operates at file level - so if there 
are renamed directories, the files inside them end up in 'R +' status 
(replaced with history) and ReviewBoard shows a spurious deletion for 
this file, in addition to a move/copy with changes.


Regards,
Alexey.

#!/usr/bin/python3
# vim: set et sw=4 :

import os
import re
import subprocess
import sys

allowed_paths = [ "/vendor/" ]
debug = False

def get_svninfo_value(svnlog, lookfor):
for l in svnlog.splitlines():
if l.startswith(lookfor):
return l[len(lookfor):]

def get_common_part(f1, f2):
l1 = f1.split('/')
l2 = f2.split('/')
for i in range(0, min(len(l1), len(l2))):
if l1[i] != l2[i]:
break;
else:
i = min(len(l1), len(l2)) + 1
return '/'.join(l1[0:i])

def get_original_path(f):
copied_path = f
rest = ""
wcroot = None
while True:
if copied_path == wcroot:
return f
svnlog = subprocess.check_output(["svn", "info", copied_path], 
universal_newlines=True)
if wcroot is None:
wcroot = get_svninfo_value(svnlog, "Working Copy Root Path: ") # 
Path to WC
rel_path = get_svninfo_value(svnlog, "Relative URL: ^") # Relative URL
root_url = get_svninfo_value(svnlog, "Repository Root: ") # Repository 
root URL
copy_url = get_svninfo_value(svnlog, "Copied From URL: ") # Copy-from 
URL
if (rel_path is None or root_url is None):
raise ValueError
if (copy_url is not None):
if (not(copy_url.startswith(root_url + '/'))):
print("Invalid copy URL")
raise ValueError
break
last_slash = copied_path.rindex("/")
rest = copied_path[last_slash:] + rest
copied_path = copied_path[:last_slash]
rel_path += rest
copy_url += rest
svnlog = subprocess.check_output(["svn", "info", wcroot], 
universal_newlines=True)
rel_root_path = get_svninfo_value(svnlog, "Relative URL: ^") # Relative URL 
for root of WC
if rel_root_path is None:
print("No root path found")
raise ValueError
lookfor = copy_url[len(root_url):]
if debug:
print('wcroot %s' % wcroot)
print("root rel path {%s}" % rel_root_path)
print("look for {%s}" % lookfor)
try:
svnlog = subprocess.check_output(["svn", "log", "-qv", f], 
universal_newlines=True)
except subprocess.CalledProcessError:
return f # Ok, even though inside a copied path, this path does not 
seem to be copied
logrevs = 
svnlog.split("\n")[1:-1]
while True:
if lookfor.startswith(rel_root_path):
orig = wcroot + lookfor[len(rel_root_path):]
if debug:
print("found local copy source for `%s': `%s' (lookfor `%s')" % 
(f, orig, lookfor))
return orig
elif get_common_part(lookfor, rel_root_path):
if debug:
 

Re: Re: svn merge --reintegrate like diff

[Daniel Shahaf]

Johan Corveleyn wrote on Mon, Sep 26, 2016 at 13:04:04 +0200:
> Maybe I'm missing something, but I don't understand why 'svn diff
> --old=TRUNK --new=BRANCH' would show you things that you previously
> merged from TRUNK to BRANCH. It should show exactly the content-wise
> difference between TRUNK and BRANCH, so if some content was merged
> from TRUNK to BRANCH, both should be identical on that point, and it
> shouldn't show up in 'diff'.

That command would also show changes made on trunk that have not yet been
merged to the branch.  (E.g., if you ran it in on subversion's trunk and
1.9.x branch, it would show -SVN_VER_MINOR 10\n +SVN_VER_MINOR 9\n.)

The OP asked for the changes merge would do, which is approximately
   --old=TRUNK@REV --new=BRANCH
where REV is the youngest revision of trunk merged to the branch.
("Approximately" because this is inaccurate when cherry-picks or subtree
merges hapepned.)

Cheers,

Daniel

Re: Re: svn merge --reintegrate like diff

[Johan Corveleyn]

On Mon, Sep 26, 2016 at 10:30 AM, Veit Guna <veit.g...@gmx.de> wrote:
>> Gesendet: Montag, 26. September 2016 um 09:59 Uhr
>> Von: "Johan Corveleyn" <jcor...@gmail.com>
>> An: "Daniel Shahaf" <d...@daniel.shahaf.name>
>> Cc: "Veit Guna" <veit.g...@gmx.de>, "users@subversion.apache.org" 
>> <users@subversion.apache.org>
>> Betreff: Re: svn merge --reintegrate like diff
>>
>> On Sat, Sep 24, 2016 at 12:44 PM, Daniel Shahaf <d...@daniel.shahaf.name> 
>> wrote:
>> > Veit Guna wrote on Sat, Sep 24, 2016 at 12:19:16 +0200:
>> >> So basically what I need is a diff that shows me the same changes that
>> >> would be made to trunk if the branch
>> >> would be merged to it (ignoring trunk changes merged to branch).
>> >>
>> >> Is this somehow possible?
>> >
>> > Checkout trunk@HEAD, run 'merge --reintegrate' (without committing the
>> > result), and run 'diff'?
>> >
>> > That'd give the most accurate answer possible, even if the branch has
>> > received cherry-picking or subtree merges.
>>
>> I would think that 'svn diff --old=$trunk --new=$branch' should also
>> work pretty well.
>>
>> Perhaps experiment with some of the options like --no-diff-added,
>> --no-diff-deleted, --ignore-properties, --show-copies-as-adds, --git,
>> --patch-compatible, ...
>>
>> --
>> Johan
>>
> Hi Johan.
>
> That's what I've used in the past. But that showed trunk merges as well. 
> Haven't tried the mentioned options though.
> Currently testing the suggested merge-to-trunk-and-diff approach. Looks 
> promising...

Hi Veit,

[ Please remember to include the mailinglist when replying. ]

Maybe I'm missing something, but I don't understand why 'svn diff
--old=TRUNK --new=BRANCH' would show you things that you previously
merged from TRUNK to BRANCH. It should show exactly the content-wise
difference between TRUNK and BRANCH, so if some content was merged
from TRUNK to BRANCH, both should be identical on that point, and it
shouldn't show up in 'diff'. Except if you're talking about ignoring
certain content in a merge e.g. by using --record-only merges, or by
modifying the merge result during conflict resolution or something
like that ...

-- 
Johan

Re: svn merge --reintegrate like diff

[Johan Corveleyn]

On Sat, Sep 24, 2016 at 12:44 PM, Daniel Shahaf  wrote:
> Veit Guna wrote on Sat, Sep 24, 2016 at 12:19:16 +0200:
>> So basically what I need is a diff that shows me the same changes that
>> would be made to trunk if the branch
>> would be merged to it (ignoring trunk changes merged to branch).
>>
>> Is this somehow possible?
>
> Checkout trunk@HEAD, run 'merge --reintegrate' (without committing the
> result), and run 'diff'?
>
> That'd give the most accurate answer possible, even if the branch has
> received cherry-picking or subtree merges.

I would think that 'svn diff --old=$trunk --new=$branch' should also
work pretty well.

Perhaps experiment with some of the options like --no-diff-added,
--no-diff-deleted, --ignore-properties, --show-copies-as-adds, --git,
--patch-compatible, ...

-- 
Johan

svn merge --reintegrate like diff

[Veit Guna]

Hi.

Currently at work we're using git for SCM. Although coming from SVN and
having my doubts in the first place, I like it very much now.
Especially the Github style flow with PRs and reviewing makes sense.

Now in another project we're still using SVN. Here I would love to have
the same PR/review flow like in Github.
For this I would like to try the following approach (simplified):

- Use feature branches in SVN
- For every commit there, a post commit hook creates a diff from
branchpoint to branch HEAD
- This diff is posted as draft to ReviewBoard
- So one can see the changes of the whole branch within ReviewBoard
- When the branch is ready to merge, the draft review request is made
public and review starts
- When all comments have been applied to the branch, the branch is
merged and the review is closed.

Now, I'm already there. Diffs are created on-the-fly, reviews can be
done etc.
The only point that annoys me is the fact, when merging latest changes
from trunk to the feature branch
my svn diff shows these changes as well in the branch. Of course, this
isn't really helpful since these changes
got already been reviewed when they were merged to trunk in the first place.

So what I wanted to have is, that my diff should only contain the
changes made within the branch, ignoring
the changes that come from merging from trunk to the branch.

As subversion 1.8 added support for transparently handling such
scenarios without using --reintegrate any
longer, I hope that this logic might have been applied to diff as well.

So basically what I need is a diff that shows me the same changes that
would be made to trunk if the branch
would be merged to it (ignoring trunk changes merged to branch).

Is this somehow possible?

Thanks
Veit