Merge lp:~vila/bzr/stack-remove-unknown into lp:bzr

[Vincent Ladeuil]

Fix obscure and misleading warning when trying to delete an unknown config option.

There is a bug in trunk:

$ ./bzr selftest -s bt.test_config.TestStackRemove.test_remove_unknown -v
running 5 tests...
bzr selftest: /home/vila/src/bzr/bugs/stack-remove-unknown/bzr
   /home/vila/src/bzr/bugs/stack-remove-unknown/bzrlib
   bzr-2.6.0dev3 python-2.7.3 Linux-3.2.0-30-generic-x86_64-with-Ubuntu-12.04-precise

test_config.TestStackRemove.test_remove_unknown(branch_only) OK 86ms
..._config.TestStackRemove.test_remove_unknown(remote_control) OK 22ms
test_config.TestStackRemove.test_remove_unknown(bazaar) OK 1ms
test_config.TestStackRemove.test_remove_unknown(location) OK 1ms
test_config.TestStackRemove.test_remove_unknown(branch) OK 10ms
----------------------------------------------------------------------
Ran 5 tests in 0.130s

OK
Option I_do_not_exist in section . of file:///tmp/testbzr-datShw.tmp/bzrlib.tests.test_config.TestStackRemove.test_remove_unknown%28location%29/home/.bazaar/locations.conf was changed from None to <CREATED>. The <DELETED> value will be saved.
Option I_do_not_exist in section DEFAULT of file:///tmp/testbzr-datShw.tmp/bzrlib.tests.test_config.TestStackRemove.test_remove_unknown%28bazaar%29/home/.bazaar/bazaar.conf was changed from None to <CREATED>. The <DELETED> value will be saved.

this has been introduced in revno 6554.

The fix is to not try to record an option deletion in a section if it
doesn't exist (because the deletion will failed, there is no deletion to
record). Why I missed that when submitting revno 6554 is a bit unclear, the
spurious warnings don't make the tests fail but are enough to demonstrate
both the bug and this proposed fix.

[Martin Packman (gz)]

Seems fine. Does this want a separate test case, rather than just being seen in an absence of kipple on the existing tests?

review: Approve

[Vincent Ladeuil (vila)]

> Seems fine.

Thanks.

> Does this want a separate test case, rather than just being seen
> in an absence of kipple on the existing tests?

I thought about it, but really, the warning caught that mistake and that should be enough.

Even the warning didn't really make sense and there is no unknown territory to explore with a new test.

[Vincent Ladeuil (vila)]

Fix obscure and misleading warning when trying to delete an unknown config option.

[Vincent Ladeuil (vila)]

Meh, that was the commit message and also an additional comment answering mgz's question ;)

[Vincent Ladeuil (vila)]

sent to pqm by email

=== modified file 'bzrlib/config.py'
--- bzrlib/config.py	2012-08-23 13:58:07 +0000
+++ bzrlib/config.py	2012-09-17 08:52:23 +0000
@@ -2944,7 +2944,7 @@
         self.options[name] = value
 
     def remove(self, name):
-        if name not in self.orig:
+        if name not in self.orig and name in self.options:
             self.orig[name] = self.get(name, None)
         del self.options[name]