Bazaar

Merge lp:~vila/bzr/1331999-test-config-warnings into lp:bzr

  1. 1331999-test-config-warnings
  2. Merge into bzr.dev
Proposed by Vincent Ladeuil
Status: Merged
Approved by: Richard Wilbur
Approved revision: no longer in the source branch.
Merged at revision: 6598
Proposed branch: lp:~vila/bzr/1331999-test-config-warnings
Merge into: lp:bzr
Diff against target: 44 lines (+9/-1)
2 files modified
bzrlib/config.py (+4/-1)
doc/en/release-notes/bzr-2.7.txt (+5/-0)
To merge this branch: bzr merge lp:~vila/bzr/1331999-test-config-warnings
Reviewer Review Type Date Requested Status
Richard Wilbur Approve
Review via email: mp+223701@code.launchpad.net

Commit message

Bzr config should save the changes explicitly when needed

Description of the change

An atexit handler is used to make bzrlib.config easier for users and save changes in any config store.

This has an side-effect for some blackbox tests and leads to warnings to stderr. It has recently been causing issues with autopkgtest.

Reproduced with:

$ ./bzr selftest -s bt.test_config -s bb.test_config
bzr selftest: /home/vila/src/bzr/bugs/1331999-test-config-warnings/bzr
   /home/vila/src/bzr/bugs/1331999-test-config-warnings/bzrlib
   bzr-2.7.0dev1 python-2.7.6 Linux-3.13.0-29-generic-x86_64-with-Ubuntu-14.04-trusty

----------------------------------------------------------------------
Ran 794 tests in 4.406s

OK
Option file in section /tmp/testbzr-6xM4Mq.tmp/bzrlib.tests.blackbox.test_config.TestConfigRemoveOption.test_branch_config_default/work/tree of file:///tmp/testbzr-6xM4Mq.tmp/bzrlib.tests.blackbox.test_config.TestConfigRemoveOption.test_branch_config_default/home/.bazaar/locations.conf was changed from locations to <CREATED>. The <DELETED> value will be saved.
Option file in section DEFAULT of file:///tmp/testbzr-6xM4Mq.tmp/bzrlib.tests.blackbox.test_config.TestConfigRemoveOption.test_bazaar_config_outside_branch/home/.bazaar/bazaar.conf was changed from bazaar to <CREATED>. The <DELETED> value will be saved.
Option file in section /tmp/testbzr-6xM4Mq.tmp/bzrlib.tests.blackbox.test_config.TestConfigRemoveOption.test_locations_config_inside_branch/work/tree of file:///tmp/testbzr-6xM4Mq.tmp/bzrlib.tests.blackbox.test_config.TestConfigRemoveOption.test_locations_config_inside_branch/home/.bazaar/locations.conf was changed from locations to <CREATED>. The <DELETED> value will be saved.
Option file in section /tmp/testbzr-6xM4Mq.tmp/bzrlib.tests.blackbox.test_config.TestConfigRemoveOption.test_branch_config_forcing_branch/work/tree of file:///tmp/testbzr-6xM4Mq.tmp/bzrlib.tests.blackbox.test_config.TestConfigRemoveOption.test_branch_config_forcing_branch/home/.bazaar/locations.conf was changed from locations to <CREATED>. The <DELETED> value will be saved.
Option file in section DEFAULT of file:///tmp/testbzr-6xM4Mq.tmp/bzrlib.tests.blackbox.test_config.TestConfigRemoveOption.test_bazaar_config_inside_branch/home/.bazaar/bazaar.conf was changed from bazaar to <CREATED>. The <DELETED> value will be saved.
Option file in section /tmp/testbzr-6xM4Mq.tmp/bzrlib.tests.blackbox.test_config.TestConfigActive.test_active_in_branch/work/tree of file:///tmp/testbzr-6xM4Mq.tmp/bzrlib.tests.blackbox.test_config.TestConfigActive.test_active_in_branch/home/.bazaar/locations.conf was changed from locations to <CREATED>. The <DELETED> value will be saved.

With this fix:

$ ./bzr selftest -s bt.test_config -s bb.test_config
bzr selftest: /home/vila/src/bzr/bugs/1331999-test-config-warnings/bzr
   /home/vila/src/bzr/bugs/1331999-test-config-warnings/bzrlib
   bzr-2.7.0dev1 python-2.7.6 Linux-3.13.0-29-generic-x86_64-with-Ubuntu-14.04-trusty

----------------------------------------------------------------------
Ran 794 tests in 4.705s

OK

To post a comment you must log in.
Revision history for this message
Richard Wilbur (richard-wilbur) wrote :

Thanks Vincent, for the elegant fix!
+2
P.S. 'sections' looks like it was unused. Is that correct?

review: Approve
Revision history for this message
Vincent Ladeuil (vila) wrote :

> Thanks Vincent, for the elegant fix!
> +2

Thanks ;)

> P.S. 'sections' looks like it was unused. Is that correct?

Yes.

Revision history for this message
Vincent Ladeuil (vila) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/config.py'
2--- bzrlib/config.py 2014-02-14 10:29:49 +0000
3+++ bzrlib/config.py 2014-06-19 09:58:21 +0000
4@@ -3559,7 +3559,6 @@
5 """
6 location_parts = self.location.rstrip('/').split('/')
7 store = self.store
8- sections = []
9 # Later sections are more specific, they should be returned first
10 for _, section in reversed(list(store.get_sections())):
11 if section.id is None:
12@@ -4246,6 +4245,8 @@
13 def _set_config_option(self, name, value, directory, scope):
14 conf = self._get_stack(directory, scope, write_access=True)
15 conf.set(name, value)
16+ # Explicitly save the changes
17+ conf.store.save_changes()
18
19 def _remove_config_option(self, name, directory, scope):
20 if name is None:
21@@ -4254,6 +4255,8 @@
22 conf = self._get_stack(directory, scope, write_access=True)
23 try:
24 conf.remove(name)
25+ # Explicitly save the changes
26+ conf.store.save_changes()
27 except KeyError:
28 raise errors.NoSuchConfigOption(name)
29
30
31=== modified file 'doc/en/release-notes/bzr-2.7.txt'
32--- doc/en/release-notes/bzr-2.7.txt 2014-05-07 22:20:27 +0000
33+++ doc/en/release-notes/bzr-2.7.txt 2014-06-19 09:58:21 +0000
34@@ -72,6 +72,11 @@
35 suite. This can include new facilities for writing tests, fixes to
36 spurious test failures and changes to the way things should be tested.
37
38+* Fix warnings on stderr caused by the atexit handler triggering for the
39+ wrong reason: the 'config' command should explicitly save the changes when
40+ modifying or removing an option and not rely on the atexit
41+ handler. (Vincent Ladeuil, #1331999)
42+
43 * Handle (minor) incompatible change in python 2.7.6 leading to test
44 failures. Only tests are affected. (Vincent Ladeuil, #1303879)
45