post-review error with svn and gnu diffutils (new review with an added binary file)

[tpinet]

Good day!

I am encountering an error when using post-review with a changeset
range in a very particular scenario. I am using Subversion as my repo.
There were a group of new files (one was a binary image) svn added to
the repository in one of the changesets. When svn attempts its diff
all of the text files that had no previous revision (rev0 in the logs)
were fine except for the binary files that did now have a previous
revision. There is an error thrown:

'Index: Reports/toolbar/upd.gif\n',
'===================================================================
\n', 'Files Reports/toolbar/upd.gif\t(revision 0) and Reports/toolbar/
upd.gif\t(revision 17489) differ\n', "svn: 'diff' returned 2\n", 'svn:
Error reading spooled REPORT request response\n']

I checked SVN docs and the svn diff command only accepts 0 and 1. When
checking the diffutils commands (http://www.gnu.org/software/hello/
manual/diff.html#Invoking%20diff) it looks as though an exit status of
0 means no differences were found, 1 means some differences were
found, and 2 means trouble. When looking at http://svn.haxx.se/users/archive-2006-03/0377.shtml
it looks as though diffutils returns 2 when one of the files does not
exist, yet the svn diff will return 1 if one of the files does not
exist.

Is there a way to tell post-review to NOT include the --diff-cmd param
so that SVN uses its native diff instead of the diffutils in my PATH?
Hopefully that will pull back the correct exit code. Otherwise I am
completely blocked if someone adds a binary file.

Thanks,
Tim

[Timothy Pinet]

Alternatively, the diffutils docs (http://www.gnu.org/software/hello/
manual/diff.html#Binary) state:

"Differing binary files are considered to cause trouble because the
resulting diff output does not capture all the differences. This
trouble causes diff to exit with status 2. However, this trouble
cannot occur with the --a or --text option, or with the -q or --brief
option, as these options both cause diff to treat binary files like
text files."

Can post-review call svn diff with the -a option? Is there a way that
I can tell it to do that in config?

I believe the reason is that the svn:mime-type property is not set
correctly on the binary file and svn diff is trying to text diff it.

Thanks,
Tim

[Christian Hammond]

There's no way to pass additional arguments right now. Personally I don't like that we even use --diff-cmd, but it works around some other problems we've hit in the past (I'd have to check the commit logs to remember exactly what they were).

It may be worth modifying your copy for now to remove that parameter, if it's blocking you.

Christian

--
Christian Hammond - chi...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com

Thanks,
Tim

--
Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~----------~----~----~----~------~----~------~--~---
To unsubscribe from this group, send email to reviewboard...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en

[Timothy Pinet]

In which file would I look to remove the param? I am using a Windows environment.

 

Thanks,

Tim

[Timothy Pinet]

In which file does reviewboard invoke the "svn diff --diff-cmd=diff"
command? I have been looking but can not find it. I then thought that
you probably call the pysvn lib for this. I checked the pysvn
documentation (http://pysvn.tigris.org/docs/
pysvn_prog_ref.html#pysvn_client_diff) and there is a flag
"ignore_content_type" for the "pysvn.Client.diff" command. I assume
that reviewboard calls this method.

Is there a way I can manually include this flag?

Are there any other suggestions on how I can tell subversion, gnu
diff, or pysvn to ignore the content type so binary files do not block
reviews?

Thanks for all of your help!

I really want to get this operating to show my organization how
awesome this tool is. In my opinion it beats out SmartBear
CodeCollaborator and Atlassian Crucible suites.

Thanks,
Tim

[Chris Clark]

Timothy Pinet wrote:
> In which file does reviewboard invoke the "svn diff --diff-cmd=diff"
> command? I have been looking but can not find it. I then thought that
> you probably call the pysvn lib for this. I checked the pysvn
> documentation (http://pysvn.tigris.org/docs/
> pysvn_prog_ref.html#pysvn_client_diff) and there is a flag
> "ignore_content_type" for the "pysvn.Client.diff" command. I assume
> that reviewboard calls this method.
>

It actually isn't in ReviewBoard, it is in postreview.py :-)

Jut fork postreview locally and you can then customize it for your
environment. This is very common, I've done it for my team as we needed
a bunch of customizations which really aren't appropriate for anyone else.

Chris

[Timothy Pinet]

I wanted to check post-review however since I am on Windows my
Python26\Scripts\post-review.exe is a compiled executable and not a
python file. Would it be enough to grab the post-review.py from the
repo and delete the exe?

Thanks,
Tim

[Chris Clark]

Timothy Pinet wrote:
> I wanted to check post-review however since I am on Windows my
> Python26\Scripts\post-review.exe is a compiled executable and not a
> python file. Would it be enough to grab the post-review.py from the
> repo and delete the exe?
>

Not with the current version. There are a few dependencies (not many),
e.g. a version module. So you should take the whole thing. You can of
course make an exe from it very easily. See end of mail for the custom
setup script I use.

Chris

"""
Issue:

NOTE using Python 2.4, this results in an exe about 4Mb in size.
NOTE using Python 2.6, this results in an exe about 5.5Mb in size.

c:\python24\python ingres_setup.py py2exe
setup.py py2exe

Quick-N-Dirty create win32 binaries and zip file script.
Zero error checking.

TODO inject 'py2exe' into sys.argv?
"""
import os
import glob
import shutil

from distutils.core import setup

import py2exe

# Clean temp Python/Jython files
delete_list=glob.glob('simplejson/*.pyc')+glob.glob('simplejson/*$py.class')
for x in delete_list:
os.remove(x)

try:
shutil.rmtree('dist')
except WindowsError, info:
# assume directory does not exist
pass

# disable optimization- we _may_ need docs strings, specifically "copyright"
setup(
options = {"py2exe": {
#"includes": ["decimal"],
"optimize": 1, ## 1 and NOT 2 because I use
the __doc__ string as the usage string. 2 optimises out the doc strings
'bundle_files': 1,
## options to reduce size of final exe
#~ 'ascii': True, # Exclude encodings
'excludes':[
'_ssl', # Exclude _ssl
'pyreadline', #'difflib',
'doctest', #'locale',
#'optparse',
'pickle', #'calendar',# Exclude
standard library
#'re',
],
}},
zipfile = None, ## try and make a single exe, if do not want this
loose this and the 'bundle_files' option
console=['postreview.py']
)

zipfilename='distribute_me.zip'
zipfilelist=['ingres_readme.txt', 'postreview.py', ] +
glob.glob('simplejson/*')+ glob.glob('dist/*')

import zipfile
z = zipfile.ZipFile(zipfilename, 'w')
for x in zipfilelist:
z.write(x)
z.close()

print 'Created:', zipfilename

[Timothy Pinet]

Thanks for the suggestion. I downloaded rbtools 0.2 stable from
http://github.com/reviewboard/rbtools, made my changes to
postreview.py, and reinstalled the package using "python setup.py
install" at the rbtool root.

In postreview.py I changed the "svn diff --diff-cmd=diff" to "svn diff
--diff-cmd=C:\\diffForceText.bat" and recompiled. I create the wrapper
script "diffForceText.bat" to simply include:

================================
@echo off
diff %* --text
================================

Which forwards all calls into GNU diff but forces the --text param.
This overcomes my binary file being text diffed and returning exit
code 2 to SVN diff.

I now have a fully working ReviewBoard + Windows + TFS repo solution
for my org and it is blowing minds and melting faces. I will post how
I got TFS to work as it was not very straight forward.

Tim