WARNING: Diff rendering failed for property svnmerge-integrated with renderer SubversionMergePropertyDiffRenderer

[Ryan J Ollos]

Another issue found in the logs. Probably not too significant, but hopefully we can raise a TracError for invalid input.

2016-01-19 19:12:45,719 Trac[changeset] WARNING: Diff rendering failed for property svnmerge-integrated with renderer SubversionMergePropertyDiffRenderer:
Traceback (most recent call last):
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/versioncontrol/web_ui/changeset.py", line 811, in render_property_diff
    options)
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/tracopt/versioncontrol/svn/svn_prop.py", line 367, in render_property_diff
    old_sources = parse_sources(old_props)
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/tracopt/versioncontrol/svn/svn_prop.py", line 364, in parse_sources
    sources[spath] = (set(Ranges(inheritable)),
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/util/__init__.py", line 982, in __init__
    self.appendrange(r)
  File "/usr/local/virtualenv/1.0-stable/lib/python2.7/site-packages/trac/util/__init__.py", line 1000, in appendrange
    a, b = int(x), int(x)
ValueError: invalid literal for int() with base 10: '7014 /trunk:1-4559'

[Ryan J Ollos]

Similar to #11308.

[Jun Omae]

Nice find.

The svnmerge-integrated property is separated by spaces but newlines are used for svn:mergeinfo.

$ svn propget -r7084 svnmerge-integrated https://svn.edgewall.org/repos/trac/trunk
/branches/0.11-stable:1-6940,6942-6971,6973-6979,6981-7012,7014-7036,7077,7080,7082 /trunk:1-4559,5254-5420

According to ​https://svn.apache.org/repos/asf/subversion/trunk/contrib/client-side/svnmerge/svnmerge.py, svnmerge-* property separated by any whitespace.

def dict_from_revlist_prop(propvalue):
    """Given a property value as a string containing per-source revision
    lists, return a dictionary whose key is a source path identifier
    and whose value is the revisions for that source."""
    prop = {}

    # Multiple sources are separated by any whitespace.
    for L in propvalue.split():
        # We use rsplit to play safe and allow colons in pathids.
        pathid_str, revs = rsplit(L.strip(), ":", 1)

[Ryan J Ollos]

The problem is seen in r7084, which has space-delimited svn properties rather than newline delimited properties.

The property changes are:

Index: trunk
===================================================================
--- trunk       (revision 7083)
+++ trunk       (revision 7084)

Property changes on: trunk
___________________________________________________________________
Modified: svnmerge-integrated
## -1 +1 ##
-/branches/0.11-stable:1-6940,6942-6971,6973-6979,6981-7012,7014-7036,7077,7080 /trunk:1-4559,5254-5420
\ No newline at end of property
+/branches/0.11-stable:1-6940,6942-6971,6973-6979,6981-7012,7014-7036,7077,7080,7082 /trunk:1-4559,5254-5420
\ No newline at end of property

$ svn pg svnmerge-integrated -r 7084 https://svn.edgewall.org/repos/trac/trunk
/branches/0.11-stable:1-6940,6942-6971,6973-6979,6981-7012,7014-7036,7077,7080,7082 /trunk:1-4559,5254-5420

The property diff should be rendered like this:

The following patch seems to fix the issue:

tracopt/versioncontrol/svn/svn_prop.py

@@ -16 +16 @@
 # Author: Christopher Lenz <cmlenz@gmx.de>
 #         Christian Boos <cboos@edgewall.org>
 
+import re
 import posixpath
 
 from genshi.builder import tag
@@ -356 +357 @@
         repos = rm.get_repository(old_context.resource.parent.id)
         def parse_sources(props):
             sources = {}
-            for line in props[name].splitlines():
+            for line in re.split('\s+', props[name]):
                 path, revs = line.split(':', 1)
                 spath = _path_within_scope(repos.scope, path)
                 if spath is not None:

That's not a good solution though because paths can have whitespace, such as in this little test example that I ran:

$svn pg svn:mergeinfo trunk
/branches/rel 2.0:23

Also, from what I recall properties should be newline delimited, not whitespace delimited. It looks like svnmerge-integrated was probably created by the svnmerge script, which I assume is a tool that was useful prior to the addition of svn:mergeinfo in SVN 1.5.

If the svnmerge-* properties are deprecated, and they are effectively malformed by using whitespace rather than newline delimiters, one possible resolution would be to just turn off rendering of the properties:

hide_properties = svk:merge, svnmerge-integrated, svnmerge-blocked

[Ryan J Ollos]

Replying to Jun Omae:

According to ​https://svn.apache.org/repos/asf/subversion/trunk/contrib/client-side/svnmerge/svnmerge.py, svnmerge-* property separated by any whitespace.

def dict_from_revlist_prop(propvalue):
    """Given a property value as a string containing per-source revision
    lists, return a dictionary whose key is a source path identifier
    and whose value is the revisions for that source."""
    prop = {}

    # Multiple sources are separated by any whitespace.
    for L in propvalue.split():
        # We use rsplit to play safe and allow colons in pathids.
        pathid_str, revs = rsplit(L.strip(), ":", 1)

I have to assume then that svnmerge.py would not store a path with whitespace encoding the path. With that information, maybe the proper solution involves parsing svnmerge-* differently from svn:mergeinfo.

[Jun Omae]

Yeah. I think we could simply fix it like this:

tracopt/versioncontrol/svn/svn_prop.py

@@ -356 +356 @@
         repos = rm.get_repository(old_context.resource.parent.id)
         def parse_sources(props):
             sources = {}
-            for line in props[name].splitlines():
+            value = props[name]
+            lines = value.splitlines() if name == 'svn:mergeinfo' else \
+                    value.split()
+            for line in lines:
                 path, revs = line.split(':', 1)
                 spath = _path_within_scope(repos.scope, path)
                 if spath is not None:

[Ryan J Ollos]

Committed to 1.0-stable in [14857], merged to trunk in [14858].

[Ryan J Ollos]

Changes have been deployed to t.e.o, and fix can be seen in r7084.