[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: [PATCH] python script for issue #3961, fixing the bogus mergeinfo

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: Mon, 12 Sep 2011 18:25:22 +0530

I like this patch. We need it.
I found this bogus mergeinfo's were causing 404 file not found,
which were sometimes causing the merge to fail, making it run longer than necessary.

Few comments.

> opts, args = my_getopt(sys.argv[1:], "h?fRc", ["help", "fix", "recursive", "commit"])

If you do not want to support 'commit' you can remove it as well.

2. I ran it against a working copy of https://ctf.open.collab.net/svn/repos/svnedge/trunk/console
and got the following output

[kamesh_at_kamesh console]$ python /tmp/mergeinfo-sanitizer.py .
The mergeinfo has to be sanitized/reverse-merged for the following as:
/branches/CTF_MODE/SvnEdge: 515-766,

[kamesh_at_kamesh console]$ python /tmp/mergeinfo-sanitizer.py --fix .
[kamesh_at_kamesh console]$ svn di
Property changes on: .
Modified: svn:mergeinfo
   Reverse-merged /branches/CTF_MODE/SvnEdge:r512-515

Based on my analysis on the above repo, I could see /branches/CTF_MODE/SvnEdge:r512-515 is bogus
and hence need to be removed. Anywway your script does it correctly with --fix.

I would suggest changing the output of default invocation(i.e no --fix)

Bogus mergeinfo summary:
Target 1: .
/branches/CTF_MODE/SvnEdge: 512-515,
/merge/source2: X-Y,

Target 2: subtree_path
/merge/source3: A-B

Target 3: sub_sub_tree
/merge/source4: C-D

Run with --fix to fix this in your working copy and commit yourself.

3. Your script seemed to sanitize to the depth of 'immediates' by default. What is the rational behind that?

4. I ran your script against https://svn.apache.org/repos/asf/subversion/trunk

And got the following output
 The path /subversion/branches/diff-optimizations/subversion/libsvn_subr/adler32.c is not found
 The path /subversion/branches/diff-optimizations/subversion/include/private/svn_adler32.h is not found

The mergeinfo has to be sanitized/reverse-merged for the following as:
/subversion/branches/tree-conflicts/subversion/include/svn_string.h: 872524-873154,868290-872329,
/subversion/branches/tree-conflicts/subversion/libsvn_diff/diff.h: 872524-873154,868290-872329,
/subversion/branches/svn-patch-improvements: 918520-934609,
/subversion/branches/tree-conflicts/subversion/libsvn_subr/hash.c: 872524-873154,868290-872329,
/subversion/branches/diff-optimizations/subversion/include/svn_string.h: 1031270-1037352,
/subversion/branches/diff-optimizations-bytes/subversion/include/private/svn_adler32.h: 1054361-1067789,
/subversion/branches/svn-patch-improvements/subversion/include/svn_string.h: 918520-934609,
/subversion/branches/diff-optimizations/subversion/libsvn_subr/hash.c: 1031270-1037352,
/subversion/branches/tree-conflicts: 872524-873154,868290-872329,
/subversion/branches/diff-optimizations-bytes/subversion/libsvn_subr/adler32.c: 1054361-1067789,
/subversion/branches/svn-patch-improvements/subversion/libsvn_diff/diff.h: 918520-934609,
/subversion/branches/diff-optimizations: 1031270-1037352,
/subversion/branches/svn-patch-improvements/subversion/libsvn_subr/hash.c: 918520-934609,
/subversion/branches/integrate-cache-item-serialization/subversion/libsvn_fs_fs/temp_serializer.h: 1068738-1068739,
/subversion/branches/integrate-cache-item-serialization/subversion/libsvn_subr/svn_temp_serializer.c: 1068723-1068739,
/subversion/branches/integrate-cache-item-serialization/subversion/libsvn_fs_fs/temp_serializer.c: 1068738-1068739,
/subversion/trunk/subversion/libsvn_diff/diff.h: 1037352-1054248,
/subversion/trunk/subversion/libsvn_subr/adler32.c: 1054276-1054360,
/subversion/trunk/subversion/include/private/svn_adler32.h: 1054276-1054360,
/subversion/branches/integrate-cache-item-serialization/subversion/include/private/svn_temp_serializer.h: 1068723-1068739,
/subversion/trunk/subversion/libsvn_diff/util.c: 1037352-1054251,

path not found error need to be handled. error should be on STDERR.
5. In main() you can call check_local_modifications before get_original_mergeinfo().

6. In get_original_mergeinfo()

> if depth == 3:
Use the named constant than the arbitrary value.

> if depth == 3:
> for entry in mergeinfo_catalog:
> pathwise_mergeinfo = pathwise_mergeinfo + mergeinfo_catalog[entry] + "\n"

I think you do something wrong with the above concatenation of mergeinfos of different targets.

> else:
> pathwise_mergeinfo = mergeinfo_catalog[wcpath]

> return core.svn_mergeinfo_parse(pathwise_mergeinfo, temp_pool)

7. In get_new_location_segments():

> for revision_range in parsed_original_mergeinfo[path]:
> try:
> ra.get_location_segments(ra_session, "", revision_range.end,
> revision_range.end, revision_range.start + 1, receiver)
> except svn.core.SubversionException:
> try:
> ra.get_location_segments(ra_session, "", core.SVN_INVALID_REVNUM,
> revision_range.end, revision_range.start + 1, receiver)

Not sure why you run location segments against HEAD upon exception.

> except svn.core.SubversionException:
> print " The path %s is not found " % path

8. Function parse_args is not used.

9. Function receiver can have more meaningful name.

10. You need to organize your functions in such a way that it appears in some logical order.
For example I started my review from 'main()' which is in the middle of the file and I move above and below that function to review further.

With regards
Kamesh Jayachandran

-----Original Message-----
From: Prabhu Gnana Sundar [mailto:prabhugs_at_collab.net]
Sent: Mon 9/12/2011 2:25 PM
To: dev_at_subversion.apache.org
Subject: Re: [PATCH] python script for issue #3961, fixing the bogus mergeinfo
On Friday 19 August 2011 11:45 AM, Prabhu Gnana Sundar wrote:
> Correcting the issue number as #3961...

Minor fix in one of the commented descriptions... Attaching the updated
Please share your thoughts.

Thanks and regards
Received on 2011-09-12 14:57:53 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.