Welcome to the Splat alpha!

Anyone? This seems like a major show stopping bug to me. A lot of people use Subversion...

Here is a sample header extracted from a diff that has property changes and fails when being uploaded to ReviewBoard.  Hopefully this helps someone to track this down.  

I wonder if pysvn needs to be updated to use svn 1.7.x since ReviewBoard uses that to parse the diff file.  The devs for pysvn seem to be working on an update for svn 1.7.x.

Index: library/ACS/Filter/Date
===================================================================
--- library/ACS/Filter/Date	(revision 2210)
+++ library/ACS/Filter/Date	(revision 2214)

Property changes on: library/ACS/Filter/Date
___________________________________________________________________
Added: bugtraq:label
## -0,0 +1 ##
+Issue #
Added: bugtraq:message
## -0,0 +1 ##
+issue %BUGID%
Added: bugtraq:warnifnoissue
## -0,0 +1 ##
+true
Added: bugtraq:url
## -0,0 +1 ##
+http://xxxxxx.com/mantis/view.php?id=%BUGID%
Index: library/ACS/Pdf/Order.php
===================================================================
--- library/ACS/Pdf/Order.php	(revision 2210)
+++ library/ACS/Pdf/Order.php	(revision 2214)

I have recently upgraded to Subversion 1.7.x and faced the same issue.  Currently it forces me to manually strip out items for directories from the diff file, this is ridiculous.

We are also suffering from the same issue, this is brutal has there been any mention on when it may be fixed?

• Duplicates:

  +	2396 - RBTools : Post-Review unable to detect SVN info since upgraded to svn client 1.7.1

We have been using ReviewBoard for a long time with SVN and really like it. Unfortunately, we recently upgraded our server to SVN 1.7.2 and started hitting this issue also (Using the latest ReviewBoard 1.6.3).

This is a big blocker. Almost seems like it should be a critical RB defect to support SVN 1.7.

• Duplicates:

  +	2434 - post-review crashes with property changes on a directory in svn

FYI for those following this issue.  A new release of Pysvn was just released which supports svn 1.7.3.  Haven't been able to test it yet.

new pysvn seems to go with python 3.2
review board tools are still on python 2.7.
Conclusion : review board post-review cannot yet be used with svn 1.7 clients ?

Hmm... according to the release notes, that shouldn't be the case.

http://pysvn.tigris.org/ds/viewMessage.do?dsForumId=1333&dsMessageId=2930786
"The kits have been built against SVN 1.7.3 and SVN 1.6.17 release
for python 2.6, 2.7 and 3.2."

I have not been able to test yet.

Source of the problem is changed format of [svn diff] command, now it includes header for each changed directory while all RB-related code uses such headers as file header.

SVN 1.7.x format:
----8<--------8<--------8<--------8<--------8<--------8<--------8<----
Index: test-dir-props
===================================================================
--- test-dir-props      (revision 10223)
+++ test-dir-props      (working copy)

Property changes on: test-dir-props
___________________________________________________________________
Added: PROP-NAME
## -0,0 +1 ##
+PROP-VALUE
----8<--------8<--------8<--------8<--------8<--------8<--------8<----

Old SVN 1.6.x format:
----8<--------8<--------8<--------8<--------8<--------8<--------8<----
Property changes on: test-dir-props
___________________________________________________________________
Added: PROP-NAME
## -0,0 +1 ##
+PROP-VALUE
----8<--------8<--------8<--------8<--------8<--------8<--------8<----

I've attached simple script which you can use instead of [svn diff] command in working directory. Script calls [svn diff] by itself, cuts SVN 1.7.x extra directory headers and print result to stdout. You can grab/pipe it to file and then upload to ReviewBoard.

In case you use Subversion's pre-commit hook its easy to pass transaction's dirs-changed list to function strip_svn17x_extra_dirs_info() and then upload resulting diff to ReviewBoard.
ATTENTION: you can call this function in hook after expanding all diff paths to absolute ones.

• Files:

  +

  cast-svn17x-diff-to-svn16x.py #!/usr/local/bin/python # -*- coding: utf-8 -*- """ Inspired by migration to SVN Client 1.7.x Details: http://code.google.com/p/reviewboard/issues/detail?id=2359 """ import os import subprocess import sys import tempfile def out(cmds, split_lines=True): """ get command, return list of lines (with EOL) or string """ fd, pathname = tempfile.mkstemp() os.close(fd) fh = open(pathname, 'r+') p = subprocess.Popen(cmds, stdout=fh, env={'LANG':'en_US.UTF-8'}) ret = p.wait() if ret != 0: raise Exception("Process ret=%d" % (ret)) fh.seek(0) if split_lines: data = fh.readlines() else: data = fh.read() fh.close() os.unlink(pathname) return data DIFF_PREFIXES = ['=' * 67, '--- ', '+++ '] INDEX_PREFIX = 'Index: ' def strip_svn17x_extra_dirs_info(diff_content, dirs_changed=None): """ tries to remove extra SVN 1.7.x header for changed directories Its easy to pass a l

Can you post a version that supports revision ranges?

New version pass CLI arguments to [svn diff].

python cast-svn17x-diff-to-svn16x_ver2.py -r 10:15

of course, it wont work with options like --xml :)

• Files:

  +

  cast-svn17x-diff-to-svn16x_ver2.py #!/usr/local/bin/python # -*- coding: utf-8 -*- """ Inspired by migration to SVN Client 1.7.x Details: http://code.google.com/p/reviewboard/issues/detail?id=2359 """ import os import subprocess import sys import tempfile def out(cmds, split_lines=True): """ get command, return list of lines (with EOL) or string """ fd, pathname = tempfile.mkstemp() os.close(fd) fh = open(pathname, 'r+') p = subprocess.Popen(cmds, stdout=fh, env={'LANG':'en_US.UTF-8'}) ret = p.wait() if ret != 0: raise Exception("Process ret=%d" % (ret)) fh.seek(0) if split_lines: data = fh.readlines() else: data = fh.read() fh.close() os.unlink(pathname) return data DIFF_PREFIXES = ['=' * 67, '--- ', '+++ '] INDEX_PREFIX = 'Index: ' def strip_svn17x_extra_dirs_info(diff_content, dirs_changed=None): """ tries to remove extra SVN 1.7.x header for changed directories Its easy to pass a l

It worked after I removed the setting of the env parameter in the call to Popen.  With it in there my default environment variables were not set and an older version of SVN ran.  Does it matter if LANG is set?

> Does it matter if LANG is set?

It depends on files and properties -- if they have non-ascii characters then it can be exported in diff even in base64-encoded form.
So I suggest use utf-8 under unix/linux :).

• Duplicates:

  +	2561 - Subversion diff failed to parse

• Duplicates:

  +	2547 - post-review Error uploading diff when the diff cause is an svn property change on a directory

My development team uses 'feature branches' so almost every time we generate a diff for review it contains svn:mergeinfo property changes.  We have to manually strip these out of the diff and it makes the post-review tool unusable.  This is probably the biggest usability issue that we have. I recommend making this issue a high priority.

@Denis
How to use your patch file with post-review.exe?

I'm using RB as review tool for every commit, so I have SVN post-commit-hook that generates review using:
post-review --repository-url=$repoURL --revision-range=$prev:$rev --summary=\"$summary\" --server=$reviewBoardURL --username=$reviewBoardUser --password=$reviewBoardPwd --submit-as=$auth -p --description-file=$descfile

for each commit.

Thanks

Hi,

my patch is for altering diff file in manual way and then uploading to ReviewBoard.

I never used post-review.exe, but if there is some option for specifying diff utility available, it is possible to create simple batch script which will:
1. get diff to temporary file via 'svn diff' command
2. alter it by my patch
3. return to post-review.exe

Regards,
Denis

SVN property should be treated as code modification. When submmiter change the value of svn:externals, it really make a big change to the code base and must be reviewd.

I hope we can have a patch on ReviewBoard server or on RBTools, althouth it may introduce some big change for the implementation of ReviewBoard.

• Duplicates:

  +	2729 - Can't create new review request

Just wanted to add that this issue caused me embarrassment on the very day I was publicly demonstrating the value of ReviewBoard to our company software manager.  Not good.

A fix will be out tonight.

• Status:

  +	PendingReview

• Tags:

  -	Priority-Medium
  +	Priority-Critical
  +	Milestone-Release1.6.x
  +	Component-DiffParser

• Assigned to:

  +	chipx86

• Duplicates:

  +	2660 - binary data is processed improperly in svn, post-review

[Sorry for the double-post: the first one had the wrong email address on it.]

I tried pulling down the patch, and it looks like there's still a bit more work needed before the bug can be completely closed.  Of course, I could have pulled it in wrong (I'm using 1.5, until we get some things in place to do an upgrade.)

One of the diffs I tried feeding it updated the svn:mergeinfo property, since I had merged in the trunk changes onto this branch:
Index: .
===================================================================
--- .   (.../base/mainline/foo)       (revision 249342)
+++ .   (.../uid/mbraun/293895) (revision 249342)

Property changes on: .
___________________________________________________________________
Modified: svn:mergeinfo
   Merged /components/foo/base/mainline/foo:r248401-248793

When I tried uploading it, RB refused complaining:
   URL 'https://XXX/components/foo/base/mainline/foo' refers to a directory 
   Can't get text contents of a directory

This is with SVN 1.7?

I'll play around with this more before we release it.

I just want to express how excited I am that this is finally being addressed. After almost a year of manually editing patch files this will be a very welcome fix. :)

If there's an way I can help test a fix, please let me know.

Yeah, I dropped the ball, but you'll hopefully never have to deal with this again :)

I'll have a new patch up before long if people want to pull it down and give it a try. Stay tuned to http://reviews.reviewboard.org/r/3347/

There's a new diff up if you guys can give it a shot. I'd like to release tonight if possible.

Just FYI, it looks like this was released as part of 1.6.12. :)

Yep.

• Status:

  -	PendingReview
  +	Fixed

Hello guys, was this bug really fixed? We've installed ReviewBoard 1.7.11 (and we're using SVN client 1.8, while SVN server 1.6), and the problem still exists.

We patch diff using third-party module for diff parsing:
http://bazaar.launchpad.net/~bzr-pqm/bzr/bzr.dev/view/head:/bzrlib/patches.py

We have moved to RB 1.6.11 but still upload patched diff with property changes stripped off.

I see -> It means it's not fixed (as for me). Probably I'll start some discussion in mailing list - since I'm not sure what's the policy about commenting close issues here.

We are using latest RB 1.7 with SVN 1.7 without any issues.  This issue was fixed.

I seem to recall reading in the release notes of SVN 1.8, there may have been changes made again to the output of svn diff.

If I were you, I would open a new issue against SVN 1.8 with the appropriate information, perhaps a sample header, to help with any necessary code changes in order to work with patches produced by SVN 1.8.

Thank you! I've already started a discussion in the google group https://groups.google.com/forum/#!topic/reviewboard-dev/--bazsEla5I
probably as a result of this discussion there might be an issue created