For perforce, we really don't have support for the second style of
diff--everything should be the first (with the file revision included,
rather than a timestamp). How did you end up with a diff file that has both
styles in it??
-David
On Wed, Mar 23, 2016 at 2:08 AM Subodh Konhor <skon...@gmail.com> wrote:
> Hi David,
>
> You are right. parse_diff_header is parsing the header incorrectly. I can
> state from a perforce diff point of view that it is incorrect.
>
> Reviewboard server code needs to be fixed
>
> Scenario:
> reviewboard/diffviewer/parser.py +169
>
> def parse_diff_header
> if linenum + 1 < len(self.lines) and \
> ((self.lines[linenum].startswith(b'--- ') and
> self.lines[linenum + 1].startswith(b'+++ ')) or
> (self.lines[linenum].startswith(b'*** ') and
> self.lines[linenum + 1].startswith(b'--- ') and
> not self.lines[linenum].endswith(b" ****"))):
>
> # This is a unified or context diff header. Parse the
> # file and extra info.
> try:
> info['origFile'], info['origInfo'] = \
> self.parse_filename_header(self.lines[linenum][4:],
> linenum)
>
> Both the condition is getting satisfied here for the same file
> One of the file has content as
>
>
>
>
>
>
>
> *--- //prod/re_test/*/f1 //prod/re_test/*/f1#0+++ //prod/re_test/*/f1
> 2016-03-17 16:02:19*** make_com.c.orig Fri Jul 4 04:18:35 1987---
> make_com.c Wed May 27 08:47:42 1992****************
> Therefore both conditions are matching. The problem is the diff parser
> treats make_commands.c.orig also as a file and generates dict value for
> info['origFile] = make_commands.c.orig
> info['origInfo'] = make_commands.c.orig
>
> This further breaks the system when *function parse_diff_revision* in
> *reviewboard/scmtools/perforce.py* tries to get revision of file through
> code
>
> *filename, revision = revision_str.rsplit('#', 1)*
>
> revision_str value is "make_commands.c.orig" which doesn't have a "#" in
> it and the code breaks.
>
> So probably the parse_diff_header should also be implemented by the
> specific repository.tool so that it's handled properly.
>
> Thanks,
> Subodh
>
>
>
>
> On Wednesday, March 23, 2016 at 12:35:36 AM UTC+5:30, David Trowbridge
> wrote:
>
>> I'm not super aware of your situation, but following a "process" just
>> because even though it's ridiculous in extreme cases like this is absurd.
>>
>> There's no good reason why --diff-filename doesn't work with Perforce
>> other than that it just hasn't been used. Passing in a diff file with
>> perforce is very rare because it would first require creating the diff with
>> "rbt diff" (since p4 diff creates broken diff files), and if you're doing
>> that, you might as well use "rbt post".
>>
>> Perhaps you can use "rbt post -X ..." or "rbt post -I ..." to pare down
>> the size of the diff rather than hand-editing it?
>>
>> On Tue, Mar 22, 2016 at 11:48 AM Subodh Konhor <sko...@gmail.com> wrote:
>>
>>> I also do acknowledge that it is humanly impossible to review but we
>>> have a process of having review done for certain products before checkin is
>>> made. Therefore we post review. So yes we do post huge diffs and cannot do
>>> away with it.
>>>
>>> Command I ran to post the changenum is
>>> rbt post -d <changenum>
>>>
>>> Error in reviewboard log is
>>>
>>> 2016-03-21 09:47:58,313 - DEBUG - - Logging to
>>> /var/www/rb/reviewboard/logs/reviewboard.log with a minimum level of DEBUG
>>> 2016-03-21 09:52:07,315 - DEBUG - - DiffParser.parse: Beginning parse
>>> of diff, size = 359762870
>>> 2016-03-21 09:52:59,314 - INFO - - Reloading logging settings
>>> 2016-03-21 09:52:59,314 - DEBUG - - Logging to
>>> /var/www/rb/reviewboard/logs/reviewboard.log with a minimum level of DEBUG
>>> 2016-03-21 09:58:00,333 - INFO - - Reloading logging settings
>>> 2016-03-21 09:58:00,333 - DEBUG - - Logging to
>>> /var/www/rb/reviewboard/logs/reviewboard.log with a minimum level of DEBUG
>>> 2016-03-21 10:00:21,018 - DEBUG - - DiffParser.parse: Finished parsing
>>> diff.
>>>
>>> 2016-03-21 10:00:55,337 - ERROR - - Unexpected error when validating
>>> diff.
>>> Traceback (most recent call last):
>>> File
>>> "/usr/local/lib/python2.7/site-packages/reviewboard/webapi/resources/validate_diff.py",
>>> line 156, in create
>>> save=False)
>>> File
>>> "/usr/local/lib/python2.7/site-packages/reviewboard/diffviewer/managers.py",
>>> line 417, in create_from_upload
>>> save=save)
>>> File
>>> "/usr/local/lib/python2.7/site-packages/reviewboard/diffviewer/managers.py",
>>> line 441, in create_from_data
>>> check_existence=(not parent_diff_file_contents)))
>>> File
>>> "/usr/local/lib/python2.7/site-packages/reviewboard/diffviewer/managers.py",
>>> line 562, in _process_files
>>> copied=f.copied)
>>> File
>>> "/usr/local/lib/python2.7/site-packages/reviewboard/scmtools/perforce.py",
>>> line 379, in parse_diff_revision
>>> filename, revision = revision_str.rsplit('#', 1)
>>> ValueError: need more than 1 value to unpack
>>>
>>>
>>> Why doesn't rbt post --diff-filename work?
>>>
>>> Thanks,
>>> Subodh
>>>
>>>
>>> On Tuesday, March 22, 2016 at 9:47:06 PM UTC+5:30, David Trowbridge
>>> wrote:
>>>
>>>> I'm not sure what's going on in the first couple tracebacks--somehow
>>>> the diff headers aren't right. It would help if you included what command
>>>> you ran to get that output. For the last traceback, it looks like there's
>>>> some issues with perforce and 'rbt post --diff-filename'
>>>>
>>>> That said, a change with 1000 or more files is going to be totally
>>>> impossible for humans to review. What's your goal in putting it on Review
>>>> Board?
>>>>
>>>> -David
>>>>
>>>> On Tue, Mar 22, 2016 at 4:28 AM Subodh Konhor <sko...@gmail.com> wrote:
>>>>
>>> Please donot go by the diff size, I am doing some testing as we do post
>>>>> huge integration's to reviewboard.
>>>>>
>>>>> Since we are migrating from 1.0.9 to 2.5.x I am doing some round of
>>>>> stress testing. I have generated merge diff which is all of type "branch"
>>>>> category.
>>>>> The diff size is whooping 359762870
>>>>>
>>>>> rbt error I get is
>>>>> >>> Making HTTP POST request to
>>>>> http://reviewboard-dev.datadomain.com/review/api/validation/diffs/
>>>>> >>> Got API Error 224 (HTTP code 400): The specified diff file could
>>>>> not be parsed.
>>>>> >>> Error data: {u'stat': u'fail', u'reason': u'need more than 1 value
>>>>> to unpack', u'err': {u'msg': u'The specified diff file could not be
>>>>> parsed.', u'code': 224}}
>>>>> Traceback (most recent call last):
>>>>> File "/usr/local/bin/rbt", line 9, in <module>
>>>>> load_entry_point('RBTools==0.7.5', 'console_scripts', 'rbt')()
>>>>> File
>>>>> "/usr/local/lib/python2.7/dist-packages/RBTools-0.7.5-py2.7.egg/rbtools/commands/main.py",
>>>>> line 133, in main
>>>>> command.run_from_argv([RB_MAIN, command_name] + args)
>>>>> File
>>>>> "/usr/local/lib/python2.7/dist-packages/RBTools-0.7.5-py2.7.egg/rbtools/commands/__init__.py",
>>>>> line 622, in run_from_argv
>>>>> exit_code = self.main(*args) or 0
>>>>> File
>>>>> "/usr/local/lib/python2.7/dist-packages/RBTools-0.7.5-py2.7.egg/rbtools/commands/post.py",
>>>>> line 857, in main
>>>>> (msg_prefix, e))
>>>>> rbtools.commands.CommandError: Error validating diff
>>>>>
>>>>> The specified diff file could not be parsed. (HTTP 400, API Error 224)
>>>>>
>>>>>
>>>>>
>>>>> on the reviewboard log I get the error as
>>>>> DiffParser.parse: Beginning parse of diff, size = 359762870
>>>>> 2016-03-21 10:00:55,337 - ERROR - - Unexpected error when validating
>>>>> diff.
>>>>> Traceback (most recent call last):
>>>>> File
>>>>> "/usr/local/lib/python2.7/site-packages/reviewboard/webapi/resources/validate_diff.py",
>>>>> line 156, in create
>>>>> save=False)
>>>>> File
>>>>> "/usr/local/lib/python2.7/site-packages/reviewboard/diffviewer/managers.py",
>>>>> line 417, in create_from_upload
>>>>> save=save)
>>>>> File
>>>>> "/usr/local/lib/python2.7/site-packages/reviewboard/diffviewer/managers.py",
>>>>> line 441, in create_from_data
>>>>> check_existence=(not parent_diff_file_contents)))
>>>>> File
>>>>> "/usr/local/lib/python2.7/site-packages/reviewboard/diffviewer/managers.py",
>>>>> line 562, in _process_files
>>>>> copied=f.copied)
>>>>> File
>>>>> "/usr/local/lib/python2.7/site-packages/reviewboard/scmtools/perforce.py",
>>>>> line 379, in parse_diff_revision
>>>>> filename, revision = revision_str.rsplit('#', 1)
>>>>> ValueError: need more than 1 value to unpack
>>>>>
>>>>> For sure the error is not due to unpacking behavior as I broke my
>>>>> changelist to a 1000 file changelist and posted which went through
>>>>> smoothly. All the files in this change list are of type branch.
>>>>>
>>>>> So what can be the problem here?
>>>>> 1) Is there a size limitation of rbt post diff that reviewboard is not
>>>>> able to handle as I can see in reviewboard log the diff is transmitted.
>>>>> 2) Should I look at apache file upload limits
>>>>> 3) Should I look into mysql limits.
>>>>>
>>>>> Since generating diff on my system takes around 6hrs, I thought of
>>>>> generating a diff file using the below command so that I can save time on
>>>>> generating diff in each iteration
>>>>> rbt diff 1234 > rb1234.diff
>>>>>
>>>>> Now if I try to post it as below
>>>>> rbt post --diff-filename=rb1234.diff
>>>>>
>>>>> I get below error
>>>>> >>> RBTools 0.7.5
>>>>> >>> Python 2.7.3 (default, Feb 27 2014, 19:58:35)
>>>>> [GCC 4.6.3]
>>>>> >>> Running on Linux-3.5.0-39-generic-x86_64-with-Ubuntu-12.04-precise
>>>>> >>> Home = /auto/home/konhos
>>>>> >>> Current directory = /home/konhos/p4ws/test_re_reviewboard
>>>>> >>> Checking for a Perforce repository...
>>>>> >>> Running: p4 info
>>>>> >>> Running: diff --version
>>>>> >>> repository info: Path: p4broker.datadomain.com:1666, Base path:
>>>>> None, Supports changesets: True
>>>>> >>> Making HTTP GET request to
>>>>> http://reviewboard-dev.datadomain.com/review/api/
>>>>> >>> Making HTTP GET request to
>>>>> http://reviewboard-dev.datadomain.com/review/api/validation/diffs/
>>>>> >>> Cached response for HTTP GET
>>>>> http://reviewboard-dev.datadomain.com/review/api/validation/diffs/
>>>>> expired and was modified
>>>>> >>> Making HTTP POST request to
>>>>> http://reviewboard-dev.datadomain.com/review/api/validation/diffs/
>>>>> Traceback (most recent call last):
>>>>> File "/usr/local/bin/rbt", line 9, in <module>
>>>>> load_entry_point('RBTools==0.7.5', 'console_scripts', 'rbt')()
>>>>> File
>>>>> "/usr/local/lib/python2.7/dist-packages/RBTools-0.7.5-py2.7.egg/rbtools/commands/main.py",
>>>>> line 133, in main
>>>>> command.run_from_argv([RB_MAIN, command_name] + args)
>>>>> File
>>>>> "/usr/local/lib/python2.7/dist-packages/RBTools-0.7.5-py2.7.egg/rbtools/commands/__init__.py",
>>>>> line 622, in run_from_argv
>>>>> exit_code = self.main(*args) or 0
>>>>> File
>>>>> "/usr/local/lib/python2.7/dist-packages/RBTools-0.7.5-py2.7.egg/rbtools/commands/post.py",
>>>>> line 902, in main
>>>>> p4_reviewers =
>>>>> self.tool.get_commit_message(self.revisions)['p4_reviewers']
>>>>> File
>>>>> "/usr/local/lib/python2.7/dist-packages/RBTools-0.7.5-py2.7.egg/rbtools/clients/__init__.py",
>>>>> line 267, in get_commit_message
>>>>> commit_message = self.get_raw_commit_message(revisions)
>>>>> File
>>>>> "/usr/local/lib/python2.7/dist-packages/RBTools-0.7.5-py2.7.egg/rbtools/clients/perforce.py",
>>>>> line 1322, in get_raw_commit_message
>>>>> changelist = revisions['tip']
>>>>> TypeError: 'NoneType' object has no attribute '__getitem__'
>>>>>
>>>>>
>>>>> Is this the expected behavior?
>>>>>
>>>>> Need urgent help as to what could be the problem. Also if somebody can
>>>>> give me the exact command for posting diff file then that would be of
>>>>> great
>>>>> help.
>>>>>
>>>>> RB on OS: Centos 5
>>>>> repository type: Perforce
>>>>>
>>>>> --
>>>>> Supercharge your Review Board with Power Pack:
>>>>> https://www.reviewboard.org/powerpack/
>>>>> Want us to host Review Board for you? Check out RBCommons:
>>>>> https://rbcommons.com/
>>>>> Happy user? Let us know! https://www.reviewboard.org/users/
>>>>> ---
>>>>> You received this message because you are subscribed to the Google
>>>>> Groups "reviewboard" group.
>>>>>
>>>> To unsubscribe from this group and stop receiving emails from it, send
>>>>> an email to reviewboard...@googlegroups.com.
>>>>
>>>>
>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>>
>>>> --
>>>> -David
>>>>
>>> --
>>> Supercharge your Review Board with Power Pack:
>>> https://www.reviewboard.org/powerpack/
>>> Want us to host Review Board for you? Check out RBCommons:
>>> https://rbcommons.com/
>>> Happy user? Let us know! https://www.reviewboard.org/users/
>>> ---
>>> You received this message because you are subscribed to the Google
>>> Groups "reviewboard" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to reviewboard...@googlegroups.com.
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>> --
>> -David
>>
> --
> Supercharge your Review Board with Power Pack:
> https://www.reviewboard.org/powerpack/
> Want us to host Review Board for you? Check out RBCommons:
> https://rbcommons.com/
> Happy user? Let us know! https://www.reviewboard.org/users/
> ---
> You received this message because you are subscribed to the Google Groups
> "reviewboard" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to reviewboard+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
--
-David
--
Supercharge your Review Board with Power Pack:
https://www.reviewboard.org/powerpack/
Want us to host Review Board for you? Check out RBCommons:
https://rbcommons.com/
Happy user? Let us know! https://www.reviewboard.org/users/
---
You received this message because you are subscribed to the Google Groups
"reviewboard" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to reviewboard+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
