Skip to content

Commit

Permalink
do not switch config type when merging configs
Browse files Browse the repository at this point in the history
There is a subtle bug in the IncludeHandler that makes the config either
a dict or an OrderedDict. If a single config is provided (that also
means no lockfile), the raw result of the config parser is taken, which
is a dict (since Python 3.6). If any merging operation of multiple
configs is performed, this is converted into a OrderedDict, as otherwise
the order of the keys is not preserved for Python < 3.6.

As we only support Python 3.6+, this bug likely does not have any
influence on the semantics of the config, but it does lead to
inconsistent results when dumping the config again (as reported in #118).
An OrderedDict is always serialized in order, a standard dict is sorted
if not specified otherwise. This also affected the pretty-printed merged
config on debug.

Instead, we now solely rely on the preserved include order of Python
dicts from Python 3.6 on (in 3.6 it is a implementation detail, later it
became part of the spec). From Python 3.6 on there is no reason to use
OrderedDict anymore, so better avoid it to be more robust when
interfacing with other libraries (like json, pyyaml).

Signed-off-by: Felix Moessbauer <[email protected]>
Signed-off-by: Jan Kiszka <[email protected]>
  • Loading branch information
fmoessbauer authored and jan-kiszka committed Dec 21, 2024
1 parent df267f4 commit f38bc1e
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 7 deletions.
4 changes: 1 addition & 3 deletions docs/userguide/project-configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,7 @@ be overwritten by the current file. While merging all the dictionaries are
merged recursively while preserving the order in which the entries are added to
the dictionary. This means that ``local_conf_header`` entries are added to the
``local.conf`` file in the same order in which they are defined in the
different include files. Note that the order of the configuration file entries
is not preserved within one include file, because the parser creates normal
unordered dictionaries.
different include files.

Including configuration files via the command line
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down
6 changes: 4 additions & 2 deletions kas/includehandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,15 +259,17 @@ def _internal_include_handler(filename, repo_path):

def _internal_dict_merge(dest, upd):
"""
Merges upd recursively into a copy of dest as OrderedDict
Merges upd recursively into a copy of dest. The order is preserved
as in the original dict as dict-insertion orders are preserved from
Python 3.6 onwards.
If keys in upd intersect with keys in dest we will do a manual
merge (helpful for non-dict types like FunctionWrapper).
"""
if (not isinstance(dest, Mapping)) \
or (not isinstance(upd, Mapping)):
raise IncludeException('Cannot merge using non-dict')
dest = OrderedDict(dest)
dest = dest.copy()
updkeys = list(upd.keys())
if set(list(dest.keys())) & set(updkeys):
for key in updkeys:
Expand Down
9 changes: 7 additions & 2 deletions kas/libcmds.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import logging
import shutil
import os
import sys
import pprint
import configparser
import json
Expand Down Expand Up @@ -549,8 +550,12 @@ def execute(self, ctx):
# now fetch everything with complete config
repos_fetch(ctx.config.get_repos())

logging.debug('Configuration from config file:\n%s',
pprint.pformat(ctx.config.get_config()))
if sys.version_info < (3, 8):
config_str = pprint.pformat(ctx.config.get_config())
else:
config_str = pprint.pformat(ctx.config.get_config(),
sort_dicts=False)
logging.debug('Configuration from config file:\n%s', config_str)


class ReposCheckout(Command):
Expand Down

0 comments on commit f38bc1e

Please sign in to comment.