Differential D17672 Diff 47813 helpers/helperslib/Settings.py

Changeset View

Standalone View

View Options

helpers/helperslib/Settings.py

  • This file was added.
1​import fnmatch
2​import copy
3​from typing import Dict
4​
5​def settings(metadata: Dict, repopath: str, platform: str) -> Dict:
bcooksleyUnsubmitted
Not Done

Could we follow the pattern from BuildSpecs here (which is similar, except it uses separate files per project/platform/etc)

Also, i'm wondering if this module should be called something different to follow the convention of BuildSpecs (although admittedly this could be used for other things as well), perhaps something like ToolingSettings?

bcooksley: Could we follow the pattern from BuildSpecs here (which is similar, except it uses separate…
knaussAuthorUnsubmitted
Not Done

can you rephrase this? I don't get it what do you want. Is it just renaming the function? or the filename?

hopefully this solotion is general enough to be used for more things.

knauss: can you rephrase this? I don't get it what do you want. Is it just renaming the function? or…
bcooksleyUnsubmitted
Not Done

The first paragraph was referring to the function name. Usually we've followed a pattern of Class.load( file ) or equivalent. Settings.settings( ) doesn't really describe what it does (which the pattern of BuildSpecs does)

For the second paragraph, I was referring to the filename (which python treats as the module name).

Based on the code, I agree it should be usable for other things.

bcooksley: The first paragraph was referring to the function name. Usually we've followed a pattern of…
6​ """returns a dictonary of settings for the repo and platform.
7​ It starts with the dict taken from default and updates the dict for every fnmatching key.
8​ For every matching key platform specific entries are searched.
9​ """
10​ entries = []
bcooksleyUnsubmitted
Done

Unless 'default' means something special to a YAML indexer, won't this code fail to pick up the defaults?

bcooksley: Unless 'default' means something special to a YAML indexer, won't this code fail to pick up the…
knaussAuthorUnsubmitted
Done

this works just fine. see line18 here we add default to the beginning, so the dict is initialized with the values of default entry. tested it for akonadi ( default values should been returned) and akonadi-search (special values are returned)

knauss: this works just fine. see line18 here we add default to the beginning, so the dict is…
bcooksleyUnsubmitted
Not Done

I see, okay. I didn't see that at first as it was under the sorting all entries comment.
Could you move the comment beside it to it's own line above entries.insert() to make this clearer?

bcooksley: I see, okay. I didn't see that at first as it was under the sorting all entries comment. Could…
knaussAuthorUnsubmitted
Not Done

well I used a more elegant solution and used "*" entry in yaml file, than we don't need a special entry.

knauss: well I used a more elegant solution and used "*" entry in yaml file, than we don't need a…
11​ for key in metadata:
12​ if fnmatch.fnmatch(repopath, key):
13​ entries.append(key)
14​
15​ # sorting all entries - the entries at the end overwrites the former settings
16​ # len is efficient, but would fail if we have single letter repo names
17​ entries.sort(key=len)
18​ entries.insert(0, 'default') # add default settings at the start
19​
20​ d = dict()
21​ for entry in entries:
22​ try:
23​ content = metadata[entry]
24​ d.update(content)
bcooksleyUnsubmitted
Not Done

Won't this lead to platform keys being imported into the configuration/settings dict we return?

bcooksley: Won't this lead to platform keys being imported into the configuration/settings dict we return?
knaussAuthorUnsubmitted
Not Done

yepp this is a side-effect, but it is just, that the dict has more entries.
Where do I get a list of platforms to strip those keys out afterwards? In helpers/check-platform.py the dict has this None:MacOSX entry (just add a MacOSX:MacOSX entry) - otherwise it is called allPlatforms :D should we move this to helperslib so we can access it from both places?

knauss: yepp this is a side-effect, but it is just, that the dict has more entries. Where do I get a…
bcooksleyUnsubmitted
Done

That map in check-platforms is more to map things to an operating system.
In theory platform names can be any string and there isn't any rules around what is valid/invalid.

I'd suggest perhaps that the YAML gets structured something like this to solve this:

"kde/pim/akonadi-search":
  general:
    gcc_options:
     - "-std=c++11"
     - "-fPIC"
     - "-DQT_NO_KEYWORDS"
  "SUSEQt5.10":   # or overwrite the setting for one specific platform
    gcc_options:
     - "-bla"
bcooksley: That map in check-platforms is more to map things to an operating system. In theory platform…
25​ d.update(content[platform]) # try to load platform specific settings
26​ except KeyError:
27​ pass
28​ return d