1199733 - Reviewboard extension causes `hg commit` to not show an editor

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect)

Description

[Benjamin Smedberg]

STR:
* change something in my mozilla-central tree
* `hg commit`

EXPECTED:
* An editor should show up to edit the commit message

ACTUAL:
* The commit happens but commits with an empty commit message.

I debugged this to a problem in reviewboard when both reviewboard and mq are active:

  /home/bsmedberg/.hg-env/lib64/python2.7/site-packages/mercurial/commands.py(1556)commit()
-> node = cmdutil.commit(ui, repo, commitfunc, pats, opts)
  /home/bsmedberg/.hg-env/lib64/python2.7/site-packages/mercurial/cmdutil.py(2464)commit()
-> return commitfunc(ui, repo, message, matcher, opts)
  /home/bsmedberg/.hg-env/lib64/python2.7/site-packages/mercurial/commands.py(1550)commitfunc()
-> extra=extra)
  /home/bsmedberg/.hg-env/lib64/python2.7/site-packages/hgext/mq.py(3418)commit()
-> editor, extra)
  /home/bsmedberg/version-control-tools/hgext/reviewboard/client.py(1041)commit()
-> return super(reviewboardrepo, self).commit(*args, **kwargs)
  /home/bsmedberg/.hg-env/lib64/python2.7/site-packages/mercurial/localrepo.py(64)wrapper()
-> return orig(repo.unfiltered(), *args, **kwargs)
> /home/bsmedberg/.hg-env/lib64/python2.7/site-packages/mercurial/localrepo.py(1497)commit()

commands.py:commit() calls:
                return repo.commit(message, opts.get('user'), opts.get('date'),
                                   match,
                                   editor=editor,
                                   extra=extra)

mq commit() passes the args by position without a keyword:
            return super(mqrepo, self).commit(text, user, date, match, force,
                                              editor, extra)

reviewboard client.py commit() filters out args[6] into kwargs, but then args = tuple(args[0:5]). I'm pretty sure this should be args[0:6].

Comment 1

[Gregory Szorc [:gps]]

Good sleuthing. Sadly these kinds of bugs with argument proxying between extensions aren't uncommon :/

Comment 2

[Benjamin Smedberg]

Attachment: MozReview Request: Bug 1199733 - Reviewboard extension strips positional argument 6 from commit. If you have both mq and reviewboard enabled this causes all commits to happen without an editor. r?gps

Bug 1199733 - Reviewboard extension strips positional argument 6 from commit. If you have both mq and reviewboard enabled this causes all commits to happen without an editor. r?gps

Comment 3

[Gregory Szorc [:gps]]

Comment on attachment 8654288 [details]
MozReview Request: Bug 1199733 - Reviewboard extension strips positional argument 6 from commit. If you have both mq and reviewboard enabled this causes all commits to happen without an editor. r?gps

https://reviewboard.mozilla.org/r/17635/#review15899

Off by 1 errors strike again.

Comment 4

[Gregory Szorc [:gps]]

I went ahead and landed this for you.