What version are you running?
RBTools 0.6.3
What's the URL of the page containing the problem?
n/a
What steps will reproduce the problem?
1. Try rbt diff/post with a revision range which includes an "empty" file.
What is the expected output? What do you see instead?
Expected: Successful diff/post. Observed: Crash.
What operating system are you using? What browser?
Windows 7
Please provide any additional information below.
rbt diff/post crashes when attempting to open a post-commit review request with a revision range, where the resultant diff contains at least one "empty" file.
Below is an example with stack trace:
C:\my\wc>rbt diff -d 28668 29029
>>> RBTools 0.6.3
>>> Python 2.7.8 (default, Jun 30 2014, 16:03:49) [MSC v.1500 32 bit (Intel)]
>>> Running on Windows-7-6.1.7601-SP1
>>> Home = C:\Users\gmyers\AppData\Roaming
>>> Current directory = C:\my\wc
>>> Checking for a Subversion repository...
>>> Running: svn info --non-interactive
>>> Running: diff --version
>>> repository info: Path: https://site/repos/foo, Base path: /some/path, Supports changesets: False
>>> Making HTTP GET request to https://reviews.mysite.com/api/
>>> Running: svn log -r 28668 -l 1 --xml
>>> Running: svn log -r 29029 -l 1 --xml
>>> Running: svn info --non-interactive
>>> Running: diff --version
>>> repository info: Path: https://site/repos/foo, Base path: /some/path, Supports changesets: False
>>> Running: svn status --ignore-externals
>>> Running: svn diff --diff-cmd=diff --notice-ancestry -r 28668:29029
>>> Running: svn diff --diff-cmd=diff --notice-ancestry -r 28668:29029 --no-diff-deleted
Traceback (most recent call last):
File "C:\Python27_32bit\Scripts\rbt-script.py", line 9, in <module>
load_entry_point('RBTools==0.6.3', 'console_scripts', 'rbt')()
File "C:\Python27_32bit\lib\site-packages\rbtools-0.6.3-py2.7.egg\rbtools\commands\main.py", line 134, in main
command.run_from_argv([RB_MAIN, command_name] + args)
File "C:\Python27_32bit\lib\site-packages\rbtools-0.6.3-py2.7.egg\rbtools\commands\__init__.py", line 416, in run_from_argv
exit_code = self.main(*args) or 0
File "C:\Python27_32bit\lib\site-packages\rbtools-0.6.3-py2.7.egg\rbtools\commands\diff.py", line 56, in main
extra_args=extra_args)
File "C:\Python27_32bit\lib\site-packages\rbtools-0.6.3-py2.7.egg\rbtools\clients\svn.py", line 293, in diff
empty_files_revisions)
File "C:\Python27_32bit\lib\site-packages\rbtools-0.6.3-py2.7.egg\rbtools\clients\svn.py", line 485, in _handle_empty_files
result.append('--- %s\t%s\n' % (filename, base))
UnboundLocalError: local variable 'base' referenced before assignment
Also for reference here are some outputs from svn diff:
C:\my\wc>svn diff --diff-cmd=diff --notice-ancestry -r 28668:29029
Index: somefile.txt
===================================================================
C:\my\wc>svn diff --diff-cmd=diff -r 28668:29029
C:\my\wc>
The issue is in _handle_empty_files() in svn.py. Here when iterating over a diff, "empty" files are identified and determined to either be added or deleted. In the branches for either of these cases, there is logic to set the local base and tip variables, but only if "not revisions['base'] and not revisions['tip']" is true, where the revisions dict is passed into the function. However, this only covers one of the four possible logical combinations of revisions['base']/revisions['tip'] and in the other three cases the local base and tip variables are not set. This leads to the above crash when these local variables are referenced. Furthermore, _handle_empty_files() appears to only be called from diff() and for the case where a revision range is specified, the base and tip dictionary elements of revisions appear to always be set.
While this describes the crash at hand there does appear to possibly be a logic fault with _handle_empty_files() in general. The commentary in this function and in the commit message where this code originated seems to only consider truly empty (e.g. 0-byte) files. The file in the provided example was not actually empty, but rather within the specified revision range it was modified and then the modifications were reverted, and the file was actually unchanged. Notice that "svn diff" without the --notice-ancestry switch reveals this fact as it produces no output, whereas the inclusion of this switch produces a diff which makes the file appear to be empty. This may be relatively moot as it appears that the code in _handle_empty_files() (and corresponding code in rbt patch) won't really do anything terrible, but it will result in the file being shown in the review request as an added file, when in fact it should not be included at all.
