Re: Issue 3674 in reviewboard: rbt post crashes for Subversion post-commit with revision range and empty file

2015-03-03 Thread reviewboard

Updates:
Status: Fixed
Owner: bar...@beanbaginc.com

Comment #2 on issue 3674 by bar...@beanbaginc.com: rbt post crashes for  
Subversion post-commit with revision range and empty file

https://code.google.com/p/reviewboard/issues/detail?id=3674

(No comment was entered for this change.)

--
You received this message because this project is configured to send all  
issue notifications to this address.

You may adjust your notification preferences at:
https://code.google.com/hosting/settings

--
You received this message because you are subscribed to the Google Groups 
reviewboard-issues group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to reviewboard-issues+unsubscr...@googlegroups.com.
To post to this group, send email to reviewboard-issues@googlegroups.com.
Visit this group at http://groups.google.com/group/reviewboard-issues.
For more options, visit https://groups.google.com/d/optout.


Re: Issue 3674 in reviewboard: rbt post crashes for Subversion post-commit with revision range and empty file

2015-01-09 Thread reviewboard

Updates:
Labels: EasyFix Component-RBTools

Comment #1 on issue 3674 by trowb...@gmail.com: rbt post crashes for  
Subversion post-commit with revision range and empty file

https://code.google.com/p/reviewboard/issues/detail?id=3674

(No comment was entered for this change.)

--
You received this message because this project is configured to send all  
issue notifications to this address.

You may adjust your notification preferences at:
https://code.google.com/hosting/settings

--
You received this message because you are subscribed to the Google Groups 
reviewboard-issues group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to reviewboard-issues+unsubscr...@googlegroups.com.
To post to this group, send email to reviewboard-issues@googlegroups.com.
Visit this group at http://groups.google.com/group/reviewboard-issues.
For more options, visit https://groups.google.com/d/optout.


Issue 3674 in reviewboard: rbt post crashes for Subversion post-commit with revision range and empty file

2014-11-17 Thread reviewboard

Status: New
Owner: 
Labels: Type-Defect Priority-Medium

New issue 3674 by griffin@gmail.com: rbt post crashes for Subversion  
post-commit with revision range and empty file

https://code.google.com/p/reviewboard/issues/detail?id=3674

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\wcrbt 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\wcsvn diff --diff-cmd=diff --notice-ancestry -r 28668:29029
Index: somefile.txt
===

C:\my\wcsvn 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.


--
You received this message because this