merge --preview fails with TypeError: _do_preview() takes exactly 2 arguments (3 given)

Bug #402022 reported by Michael Hudson-Doyle
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Bazaar
Invalid
High
Martin Pool
QBzr
Fix Released
High
Alexander Belchenko

Bug Description

I think this is pretty new. It only happens when there are actually changes to preview.

bzr: ERROR: exceptions.TypeError: _do_preview() takes exactly 2 arguments (3 given)

Traceback (most recent call last):
  File "/home/mwh/canonical/repos/bzr/bzr.dev/bzrlib/commands.py", line 835, in exception_to_return_code
    return the_callable(*args, **kwargs)
  File "/home/mwh/canonical/repos/bzr/bzr.dev/bzrlib/commands.py", line 1030, in run_bzr
    ret = run(*run_argv)
  File "/home/mwh/canonical/repos/bzr/bzr.dev/bzrlib/commands.py", line 647, in run_argv_aliases
    return self.run(**all_cmd_args)
  File "/usr/lib/python2.6/dist-packages/bzrlib/plugins/qbzr/lib/commands.py", line 662, in run
    return bzrlib.builtins.cmd_merge.run(self, *args, **kw)
  File "/home/mwh/canonical/repos/bzr/bzr.dev/bzrlib/builtins.py", line 3701, in run
    return self._do_preview(merger, cleanups)
TypeError: _do_preview() takes exactly 2 arguments (3 given)

bzr 1.18dev on python 2.6.2 (linux2)
arguments: ['/home/mwh/canonical/repos/bzr/bzr.dev/bzr', 'merge', '--preview', '../db-devel/']
encoding: 'UTF-8', fsenc: 'UTF-8', lang: 'en_NZ.UTF-8'
plugins:
  bpm /home/mwh/.bazaar/plugins/bpm [unknown]
  bzrtools /usr/lib/python2.6/dist-packages/bzrlib/plugins/bzrtools [1.17]
  dbus /usr/lib/python2.6/dist-packages/bzrlib/plugins/dbus [unknown]
  ec2test /home/mwh/.bazaar/plugins/ec2test.py [unknown]
  fastimport /home/mwh/.bazaar/plugins/fastimport [unknown]
  l /home/mwh/.bazaar/plugins/l.py [unknown]
  launchpad /home/mwh/canonical/repos/bzr/bzr.dev/bzrlib/plugins/launchpad [1.18dev]
  loggerhead /home/mwh/.bazaar/plugins/loggerhead [1.11]
  lpreview /home/mwh/.bazaar/plugins/lpreview [unknown]
  merged /home/mwh/.bazaar/plugins/merged [unknown]
  netrc_credential_store /home/mwh/canonical/repos/bzr/bzr.dev/bzrlib/plugins/netrc_credential_store [1.18dev]
  plugin_service /home/mwh/.bazaar/plugins/plugin_service.py [unknown]
  pqm /home/mwh/.bazaar/plugins/pqm [1.4dev]
  pybloom /home/mwh/.bazaar/plugins/pybloom [unknown]
  qbzr /usr/lib/python2.6/dist-packages/bzrlib/plugins/qbzr [0.12]
  revnocache /home/mwh/.bazaar/plugins/revnocache [unknown]
  service /home/mwh/.bazaar/plugins/service [unknown]
*** Bazaar has encountered an internal error.
    Please report a bug at https://bugs.launchpad.net/bzr/+filebug
    including this traceback, and a description of what you
    were doing when the error occurred.

Related branches

Changed in bzr:
importance: Undecided → High
status: New → Confirmed
Revision history for this message
Martin Pool (mbp) wrote :

I'm also seeing a similar failure in

ERROR: blackbox.test_merge.TestMerge.test_merge_preview
    _do_preview() takes exactly 2 arguments (3 given)

Changed in bzr:
assignee: nobody → Martin Pool (mbp)
Revision history for this message
Martin Pool (mbp) wrote :

I don't understand how this can be causing a test failure and not pqm failures. Perhaps it's actually caused by a plugin, though not one visible in the traceback.

Indeed the problem is caused by qbzr hooking in here:

(Pdb) p self._do_preview
<bound method cmd_merge._do_preview of <bzrlib.plugins.qbzr.lib.commands.cmd_merge object at 0x18204d0>>

Revision history for this message
Martin Pool (mbp) wrote :

This still occurs with the current tip of qbzr.

It would be nice if qbzr could get what it wants without overriding cmd_merge, which seems likely to cause such breakage, and instead have hooks.

Revision history for this message
Martin Pool (mbp) wrote :

The specific problem is that bzr has added a 'cleanups' parameter to _do_preview, and qbzr is overriding this method and doesn't support it. The idea is that this list can be mutated if more things need to be cleaned, and they'll eventually all be run before the command exits. I suppose for things like dirstate locking this is more reilable than relying on gc - and anyhow we discourage relying on it.

One option would be to put the cleanups into the state of the command object; that's arguably cleaner than passing it around just in case it's needed.

Revision history for this message
Martin Pool (mbp) wrote :

... this approach has the advantage of not requiring a change in qbzr.

The cleanups parameter was added by Aaron recently when doing interactive merges.

Revision history for this message
Martin Pool (mbp) wrote :

I don't think this needs to be fixed in qbzr.

Changed in bzr:
status: Confirmed → Fix Committed
Changed in qbzr:
status: New → Invalid
Revision history for this message
Martin Pool (mbp) wrote :

OK, so Aaron's response in https://code.edge.launchpad.net/~mbp/bzr/402022-merge-preview/+merge/9233 is that he only changed a private API therefore it's qbzr's bug.

Changed in bzr:
status: Fix Committed → Invalid
Changed in qbzr:
status: Invalid → Confirmed
importance: Undecided → High
Changed in qbzr:
assignee: nobody → Alexander Belchenko (bialix)
milestone: none → 0.13
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.