[MERGE] Warn when setting push location is masked by locations.conf (#122286)

[Ian Clatworthy]

Aaron Bentley wrote:
> Hi all,
> 
> A difference between Branch5 (knit) and Branch6 (dirstate-tags) is that
> Branch6 stores all config data in branch.conf.  Branch5 had stored
> push_location in locations.conf.
> 
> Unfortunately, this means that if a branch is upgraded from Branch5 to
> Branch6, its old push_location will override any new location set with
> push --remember.
> 
> This patch adds an optional warning when setting user options to a
> masked value, and enables it for all branch values.
> 
> Aaron

------------------------------------------------------------------------

=== modified file bzrlib/branch.py
--- bzrlib/branch.py
+++ bzrlib/branch.py

This only set_user_option call missed in this module is the one on line
1636. It isn't needed there right, because it's explicitly being put
into locations.conf?

=== modified file bzrlib/config.py
--- bzrlib/config.py
+++ bzrlib/config.py

+        if store in (STORE_GLOBAL, STORE_BRANCH):
+            mask_value = self._get_location_config().get_user_option(name)
+            if mask_value is not None:
+                trace.warning('Value "%s" is masked by "%s" from'
+                              ' locations.conf', value, mask_value)
+            else:
+                if store == STORE_GLOBAL:
+                    branch_config = self._get_branch_data_config()
+                    mask_value = branch_config.get_user_option(name)
+                    if mask_value is not None:
+                        trace.warning('Value "%s" is masked by "%s" from'
+                                      ' branch.conf' % (value, mask_value))
+

Minor gripe - the first trace.warning() separates the substitution
variables with commas while the second one generates the final string
using %. It would be good to be consistent and my leaning is usually
towards the first form, i.e. pushing substitution into the lower UI layers.

Ian C.