From 76d18e3a82e88158aa42698d429f46d50d561b21 Mon Sep 17 00:00:00 2001 From: Todd Gamblin <tgamblin@llnl.gov> Date: Thu, 3 Sep 2020 01:10:47 -0700 Subject: [PATCH] bugfix: fix config merge order for OrderdDicts The logic in `config.py` merges lists correctly so that list elements from higher-precedence config files come first, but the way we merge `dict` elements reverses the precedence. Since `mirrors.yaml` relies on `OrderedDict` for precedence, this bug causes mirrors in lower-precedence config scopes to be checked before higher-precedence scopes. We should probably convert `mirrors.yaml` to use a list at some point, but in the meantie here's a fix for `OrderedDict`. - [x] ensuring that keys are ordered correctly in `OrderedDict` by re-inserting keys from the destination `dict` after adding the keys from the source `dict`. Assuming a default spack configuration, if we run this: ```console $ spack mirror add foo https://bar.com ``` Results before this change: ```console $ spack config blame mirrors --- mirrors: /Users/gamblin2/src/spack/etc/spack/defaults/mirrors.yaml:2 spack-public: https://spack-llnl-mirror.s3-us-west-2.amazonaws.com/ /Users/gamblin2/.spack/mirrors.yaml:2 foo: https://bar.com ``` Results after: ```console $ spack config blame mirrors --- mirrors: /Users/gamblin2/.spack/mirrors.yaml:2 foo: https://bar.com /Users/gamblin2/src/spack/etc/spack/defaults/mirrors.yaml:2 spack-public: https://spack-llnl-mirror.s3-us-west-2.amazonaws.com/ ``` --- lib/spack/spack/config.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 425fcec8ee..bbefd8d796 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -944,6 +944,10 @@ def they_are(t): # track keys for marking key_marks = {} + # track this to ensure that source items come *before* dest in + # iteration order with OrderdDicts + dest_keys = [dk for dk in dest.keys() if dk not in source] + for sk, sv in iteritems(source): if _override(sk) or sk not in dest: # if sk ended with ::, or if it's new, completely override @@ -958,6 +962,12 @@ def they_are(t): # to copy mark information on source keys to dest. key_marks[sk] = sk + # ensure that dest keys come after source + for dk in dest_keys: + value = dest[dk] + del dest[dk] + dest[dk] = value + # ensure that keys are marked in the destination. The # key_marks dict ensures we can get the actual source key # objects from dest keys -- GitLab