Spurious PointlessCommit from 'bzr commit -x' that would only commit deletes

[KarlGoetz]

I was asked to submit a bug by jelmer, i'm including a snippet of the irc convo, and the pastebins at the bottom.

11:55:17 < Kamping_Kaiser> hi all, i just tried to commit the deltion of some files (-x 'd two modified files), and bzr told me an empty commit was pointless http://paste.debian.net/119678/ . does this look like a bug or a feature you you? (wondering if i should go looking for a bug report)
12:00:32 < jelmer_> Kamping_Kaiser, what happens if you run "bzr rm" first?
12:05:24 < Kamping_Kaiser> jelmer_: if i run bzr rm on the tree as is it gives the same error. shall i try restoring the files then bzr rming them?
12:08:12 < jelmer_> Kamping_Kaiser: It should at least give a slightly different error (removed vs missing)
12:10:26 < Kamping_Kaiser> jelmer_: http://paste.debian.net/119679/ this is what i get folling on from the last paste. have i missunderstood your suggestion?
12:11:41 < jelmer_> Kamping_Kaiser, that's odd indeed, please file a bug
12:12:44 < Kamping_Kaiser> jelmer_: should i include the contents of those pastes?
12:12:55 < Kamping_Kaiser> and is there anything else that would be handy? (repo state, etc)
12:13:32 < jelmer_> Kamping_Kaiser: I'm not sure; given I've never seen this and I never use -x I would be inclined to blame -x
12:14:00 < Kamping_Kaiser> ok. i'll tar it up and hang onto it for a few weeks incase its requested
12:14:31 < jelmer> Kamping_Kaiser, thanks :)
12:14:39 < Kamping_Kaiser> np :)

paste 119678 is:
21:52:53 kgoetz@epicfail: ~/Desktop/gnewsense-art/gnewsense-parkes-icon-theme/debian $ bzr ci -x control -x copyright
Committing to: /home/kgoetz/Desktop/gnewsense-art/gnewsense-parkes-icon-theme/
missing debian/README.Debian
missing debian/README.source
missing debian/docs
aborting commit write group: PointlessCommit(No changes to commit)
bzr: ERROR: No changes to commit. Use --unchanged to commit anyhow.
21:52:56 kgoetz@epicfail: ~/Desktop/gnewsense-art/gnewsense-parkes-icon-theme/debian $ bzr st
removed:
  debian/README.Debian
  debian/README.source
  debian/docs
modified:
  debian/control
  debian/copyright

paste 119679 is:
21:55:44 kgoetz@epicfail: ~/Desktop/gnewsense-art/gnewsense-parkes-icon-theme/debian $ bzr rm README.Debian README.source docs
bzr: ERROR: Can't safely remove modified or unknown files:
removed:
  debian/README.Debian
  debian/README.source
modified:
  debian/docs
Use --keep to not delete them, or --force to delete them regardless.
22:04:25 kgoetz@epicfail: ~/Desktop/gnewsense-art/gnewsense-parkes-icon-theme/debian $ bzr st
removed:
  debian/README.Debian
  debian/README.source
modified:
  debian/control
  debian/copyright
  debian/docs
22:04:31 kgoetz@epicfail: ~/Desktop/gnewsense-art/gnewsense-parkes-icon-theme/debian $ rm docs
22:04:34 kgoetz@epicfail: ~/Desktop/gnewsense-art/gnewsense-parkes-icon-theme/debian $ bzr rm README.Debian README.source docs
debian/docs does not exist.
debian/README.source does not exist.
debian/README.Debian does not exist.
22:04:37 kgoetz@epicfail: ~/Desktop/gnewsense-art/gnewsense-parkes-icon-theme/debian $ bzr st
removed:
  debian/README.Debian
  debian/README.source
  debian/docs
modified:
  debian/control
  debian/copyright
22:04:39 kgoetz@epicfail: ~/Desktop/gnewsense-art/gnewsense-parkes-icon-theme/debian $ bbzr ci -x control -x copyright
Committing to: /home/kgoetz/Desktop/gnewsense-art/gnewsense-parkes-icon-theme/
bzr: ERROR: No changes to commit. Please 'bzr add' the files you want to commit, or use --unchanged to force an empty commit.
bzr: warning: some compiled extensions could not be loaded; see <https://answers.launchpad.net/bzr/+faq/703>
22:04:44 kgoetz@epicfail: ~/Desktop/gnewsense-art/gnewsense-parkes-icon-theme/debian $

[KarlGoetz (kgoetz)]

Some version info:

22:19:26 kgoetz@epicfail: ~ $ lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description: Debian GNU/Linux 6.0.1 (squeeze)
Release: 6.0.1
Codename: squeeze
22:19:28 kgoetz@epicfail: ~ $ bbzr --version
Bazaar (bzr) 2.4.0dev4
  from bzr checkout /home/kgoetz/src/bzr
    revision: 5967
    revid: <email address hidden>
    branch nick: bzr
  Python interpreter: /usr/bin/python 2.6.6
  Python standard library: /usr/lib/python2.6
  Platform: Linux-2.6.32-5-686-bigmem-i686-with-debian-6.0.1
  bzrlib: /home/kgoetz/src/bzr/bzrlib
  Bazaar configuration: /home/kgoetz/.bazaar
  Bazaar log file: /home/kgoetz/.bzr.log

Copyright 2005-2011 Canonical Ltd.
http://bazaar.canonical.com/

bzr comes with ABSOLUTELY NO WARRANTY. bzr is free software, and
you may use, modify and redistribute it under the terms of the GNU
General Public License version 2 or later.

Bazaar is part of the GNU Project to produce a free operating system.

bzr: warning: some compiled extensions could not be loaded; see <https://answers.launchpad.net/bzr/+faq/703>
22:19:32 kgoetz@epicfail: ~ $ bzr --version
Bazaar (bzr) 2.1.2
  Python interpreter: /usr/bin/python 2.6.6
  Python standard library: /usr/lib/python2.6
  Platform: Linux-2.6.32-5-686-bigmem-i686-with-debian-6.0.1
  bzrlib: /usr/lib/python2.6/dist-packages/bzrlib
  Bazaar configuration: /home/kgoetz/.bazaar
  Bazaar log file: /home/kgoetz/.bzr.log

Copyright 2005-2010 Canonical Ltd.
http://bazaar-vcs.org/

bzr comes with ABSOLUTELY NO WARRANTY. bzr is free software, and
you may use, modify and redistribute it under the terms of the GNU
General Public License version 2 or later.

Bazaar is part of the GNU Project to produce a free operating system.

[Andrew Bennetts (spiv)]

I think I see the problem in a general sense, although I'm not sure what to do to fix it.

When -x is used, Commit._update_builder_with_changes uses the _populate_from_inventory code rather than the iter_changes code. And it seems that although _populate_from_inventory notices missing files, nothing acts on that. It sets self.deleted_ids, but neither it, _record_unselected, nor _report_and_accumulate_deletes uses that variable. Presumably something needs to happen at about the time _record_unselected happens to inform the builder that the files in deleted_ids have been deleted, but I'm not sure precisely when, nor what the API to use on the commit builder to acheive this.

(As an aside: it's a bit weird that the commit builder appears to be two different kinds of objects in one; the delta-based builder and the whole-inventory-based builder; if configured in the latter mode then AIUI methods like record_delete are not available. Wouldn't two separate classes be clearer? They could still share much of the same code, of course, e.g. via a base class.)

[Andrew Bennetts (spiv)]

Seperately, I wonder if "bzr commit -x" thus has the side-effect of forgetting to commit any deletes at all?

[Andrew Bennetts (spiv)]

A work around in this case would be to instead use shelve to exclude the files:

  bzr shelve --all debian/control debian/copyright
  bzr ci
  bzr unshelve

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

...
> (As an aside: it's a bit weird that the commit builder appears to be two
> different kinds of objects in one; the delta-based builder and the
> whole-inventory-based builder; if configured in the latter mode then
> AIUI methods like record_delete are not available. Wouldn't two
> separate classes be clearer? They could still share much of the same
> code, of course, e.g. via a base class.)

Simply another class doesn't work very well because CommitBuilder is
specific to the Repository implementation.

I think we generally just want to get rid of the whole-inventory-builder
we just need to implement -x on a delta stream.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk4BBvkACgkQJdeBCYSNAANDTQCfRCcPxEwh5A0yAGDAUenYp4EG
cbgAoIkQ2YXTi0lLT03jHT2KmB6yMZU5
=zxxD
-----END PGP SIGNATURE-----