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 > <javascript:>> 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 <javascript:>. >> 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.