Status: New
Owner:
Labels: Type-Defect Priority-Medium
New issue 3497 by jco...@gmail.com: rbt tools not posting deleted files for
perforce
http://code.google.com/p/reviewboard/issues/detail?id=3497
*** READ THIS BEFORE POSTING!
***
*** You must complete this form in its entirety, or your bug report will be
*** rejected.
***
*** If you have a security issue to report, please send it confidentially
to
*** secur...@reviewboard.org. Posting security-related issues to this bug
*** tracker causes us to have to do an emergency release.
***
*** For customer support, please post to reviewbo...@googlegroups.com
***
*** If you have a patch, please submit it to
http://reviews.reviewboard.org/
***
*** This bug tracker is public. Please check that any logs or other
information
*** that you include has been stripped of confidential information.
What version are you running?
1.7.22
What's the URL of the page containing the problem?
Private system.
What steps will reproduce the problem?
1. Submit a perforce change containing one or more deleted files
2. Run "rbt post " (where the change list no is the one
submitted in step 1)
3. On the generate review you will see that the deleted files are NOT
present.
What is the expected output? What do you see instead?
The deleted files should be present in the diff report.
What operating system are you using? What browser?
Linux on the server, Windows 7 on the client with Firefox
Please provide any additional information below.
I think the issue is this:-
The most recent version of the file in the submitted change list output
from perforce is the delete.
I think the issue begins in _compute_range_changes in perforce.py
What I believe happens is the code in perforce.py runs "p4
filelog "//...@," to get the list of changes from the
change list I supplied.
It iterates through the list of changes, parsing out the depot files,
action and locations.
_accumulate_range_change is called for each file. If "old_action" is edit,
then it decrements initialRev. This is therefore not done for a delete, so
the value of initialRev remains as the last (deleted) version of the file:-
def _accumulate_range_change(self, file_entry, change):
"""Compute the effects of a given change on a given file"""
old_action = file_entry['action']
current_action = change['action']
if old_action == 'none':
# This is the first entry for this file.
new_action = current_action
file_entry['depotFile'] = file_entry['initialDepotFile']
# If the first action was an edit, then the initial revision
# (that we'll use to generate the diff) is n-1
if current_action == 'edit':
file_entry['initialRev'] -= 1
It then iterates through the built list of files and if action is delete,
does this:-
elif action == 'delete':
try:
old_file, new_file = self._extract_delete_files(
initial_depot_file, initial_rev)
except ValueError:
logging.warning('Skipping file %s: %s', depot_file, e)
continue
diff_lines += self._do_diff(
old_file, new_file, initial_depot_file, initial_rev,
depot_file, 'D', ignore_unmodified=True)
In this case the value passed to extract_delete_files is the deleted
revision of the file (I.E. the version output from p4 filelog).
def _extract_delete_files(self, depot_file, revision):
"""Extract the 'old' and 'new' files for a delete operation.
Returns a tuple of (old filename, new filename). This can raise a
ValueError if extraction fails.
"""
# Get the old version out of perforce
old_filename = make_tempfile()
self._write_file('%s#%s' % (depot_file, revision), old_filename)
# Make an empty tempfile for the new file
new_filename = make_tempfile()
return old_filename, new_filename
The _write_file method then causes it to execute this command:-
p4 print -o /tmp/tmp-9Bncr -q //depot//DeletedFile#2 (where #2 is the
deleted revision of the file)
In this case if the temp file did not already exist, perforce does not
create it. But it was already created by make_tempfile (and was empty). So
the result is we still have an empty file. The new file is also created
with make_tempfile and hence is empty.
This means that when the diff is run, it ends up comparing two empty files.
It then of course says there is no change and so the file that is deleted
ends up NOT getting included in the diff that rbt uploads. I think this is
an error and in the old_filename code above the depot revision needs to be
decreased by 1. This will cause "p4 print" to output the old version of the
file (prior to the delete). Then when the diff runs, it compares the old
(pre delete) ver
