From de8e3d65e06f91f0927e84dbf215a298b73590b8 Mon Sep 17 00:00:00 2001 From: Jussi Pakkanen Date: Thu, 11 Jul 2024 16:26:32 +0300 Subject: [PATCH] Remove option type from OptionKey and get it from OptionStore instead. --- mesonbuild/coredata.py | 22 ++++++---- mesonbuild/options.py | 76 ++++++++++++++-------------------- unittests/allplatformstests.py | 2 +- unittests/internaltests.py | 14 +++---- 4 files changed, 54 insertions(+), 60 deletions(-) diff --git a/mesonbuild/coredata.py b/mesonbuild/coredata.py index 1659d5180..5150e6927 100644 --- a/mesonbuild/coredata.py +++ b/mesonbuild/coredata.py @@ -22,7 +22,7 @@ from .mesonlib import ( pickle_load ) -from .options import OptionKey, OptionType +from .options import OptionKey from .machinefile import CmdLineFileParser @@ -586,8 +586,6 @@ class CoreData: def update_project_options(self, project_options: 'MutableKeyedOptionDictType', subproject: SubProject) -> None: for key, value in project_options.items(): - if not self.optstore.is_project_option(key): - continue if key not in self.optstore: self.optstore.add_project_option(key, value) continue @@ -654,7 +652,7 @@ class CoreData: continue elif k in self.optstore: dirty |= self.set_option(k, v, first_invocation) - elif k.machine != MachineChoice.BUILD and k.type != OptionType.COMPILER: + elif k.machine != MachineChoice.BUILD and not self.optstore.is_compiler_option(k): unknown_options.append(k) if unknown_options: unknown_options_str = ', '.join(sorted(str(s) for s in unknown_options)) @@ -700,9 +698,9 @@ class CoreData: continue # Skip base, compiler, and backend options, they are handled when # adding languages and setting backend. - if k.type in {OptionType.COMPILER, OptionType.BACKEND}: + if self.optstore.is_compiler_option(k) or self.optstore.is_backend_option(k): continue - if k.type == OptionType.BASE and k.as_root() in base_options: + if self.optstore.is_base_option(k) and k.as_root() in base_options: # set_options will report unknown base options continue options[k] = v @@ -908,7 +906,17 @@ class OptionsView(abc.Mapping): # FIXME: This is fundamentally the same algorithm than interpreter.get_option_internal(). # We should try to share the code somehow. key = key.evolve(subproject=self.subproject) - if not key.is_project_hack_for_optionsview(): + if not isinstance(self.original_options, options.OptionStore): + # This is only used by CUDA currently. + # This entire class gets removed when option refactor + # is finished. + if '_' in key.name or key.lang is not None: + is_project_option = False + else: + sys.exit(f'FAIL {key}.') + else: + is_project_option = self.original_options.is_project_option(key) + if not is_project_option: opt = self.original_options.get(key) if opt is None or opt.yielding: key2 = key.as_root() diff --git a/mesonbuild/options.py b/mesonbuild/options.py index f7195173f..013fd1ad6 100644 --- a/mesonbuild/options.py +++ b/mesonbuild/options.py @@ -5,7 +5,6 @@ from collections import OrderedDict from itertools import chain from functools import total_ordering import argparse -import enum from .mesonlib import ( HoldableObject, @@ -39,32 +38,6 @@ genvslitelist = ['vs2022'] buildtypelist = ['plain', 'debug', 'debugoptimized', 'release', 'minsize', 'custom'] -class OptionType(enum.IntEnum): - - """Enum used to specify what kind of argument a thing is.""" - - BUILTIN = 0 - BACKEND = 1 - BASE = 2 - COMPILER = 3 - PROJECT = 4 - -def _classify_argument(key: 'OptionKey') -> OptionType: - """Classify arguments into groups so we know which dict to assign them to.""" - - if key.name.startswith('b_'): - return OptionType.BASE - elif key.lang is not None: - return OptionType.COMPILER - elif key.name in _BUILTIN_NAMES or key.module: - return OptionType.BUILTIN - elif key.name.startswith('backend_'): - assert key.machine is MachineChoice.HOST, str(key) - return OptionType.BACKEND - else: - assert key.machine is MachineChoice.HOST, str(key) - return OptionType.PROJECT - # This is copied from coredata. There is no way to share this, because this # is used in the OptionKey constructor, and the coredata lists are # OptionKeys... @@ -117,21 +90,19 @@ class OptionKey: internally easier to reason about and produce. """ - __slots__ = ['name', 'subproject', 'machine', 'lang', '_hash', 'type', 'module'] + __slots__ = ['name', 'subproject', 'machine', 'lang', '_hash', 'module'] name: str subproject: str machine: MachineChoice lang: T.Optional[str] _hash: int - type: OptionType module: T.Optional[str] def __init__(self, name: str, subproject: str = '', machine: MachineChoice = MachineChoice.HOST, lang: T.Optional[str] = None, - module: T.Optional[str] = None, - _type: T.Optional[OptionType] = None): + module: T.Optional[str] = None): # the _type option to the constructor is kinda private. We want to be # able tos ave the state and avoid the lookup function when # pickling/unpickling, but we need to be able to calculate it when @@ -142,9 +113,6 @@ class OptionKey: object.__setattr__(self, 'lang', lang) object.__setattr__(self, 'module', module) object.__setattr__(self, '_hash', hash((name, subproject, machine, lang, module))) - if _type is None: - _type = _classify_argument(self) - object.__setattr__(self, 'type', _type) def __setattr__(self, key: str, value: T.Any) -> None: raise AttributeError('OptionKey instances do not support mutation.') @@ -155,7 +123,6 @@ class OptionKey: 'subproject': self.subproject, 'machine': self.machine, 'lang': self.lang, - '_type': self.type, 'module': self.module, } @@ -173,8 +140,8 @@ class OptionKey: def __hash__(self) -> int: return self._hash - def _to_tuple(self) -> T.Tuple[str, OptionType, str, str, MachineChoice, str]: - return (self.subproject, self.type, self.lang or '', self.module or '', self.machine, self.name) + def _to_tuple(self) -> T.Tuple[str, str, str, MachineChoice, str]: + return (self.subproject, self.lang or '', self.module or '', self.machine, self.name) def __eq__(self, other: object) -> bool: if isinstance(other, OptionKey): @@ -199,7 +166,7 @@ class OptionKey: return out def __repr__(self) -> str: - return f'OptionKey({self.name!r}, {self.subproject!r}, {self.machine!r}, {self.lang!r}, {self.module!r}, {self.type!r})' + return f'OptionKey({self.name!r}, {self.subproject!r}, {self.machine!r}, {self.lang!r}, {self.module!r})' @classmethod def from_string(cls, raw: str) -> 'OptionKey': @@ -269,7 +236,8 @@ class OptionKey: def is_project_hack_for_optionsview(self) -> bool: """This method will be removed once we can delete OptionsView.""" - return self.type is OptionType.PROJECT + import sys + sys.exit('FATAL internal error. This should not make it into an actual release. File a bug.') class UserOption(T.Generic[_T], HoldableObject): @@ -712,6 +680,7 @@ BUILTIN_DIR_NOPREFIX_OPTIONS: T.Dict[OptionKey, T.Dict[str, str]] = { class OptionStore: def __init__(self): self.d: T.Dict['OptionKey', 'UserOption[T.Any]'] = {} + self.project_options = set() def __len__(self): return len(self.d) @@ -734,6 +703,7 @@ class OptionStore: def add_project_option(self, key: T.Union[OptionKey, str], valobj: 'UserOption[T.Any]'): key = self.ensure_key(key) self.d[key] = valobj + self.project_options.add(key) def set_value(self, key: T.Union[OptionKey, str], new_value: 'T.Any') -> bool: key = self.ensure_key(key) @@ -774,23 +744,39 @@ class OptionStore: def is_project_option(self, key: OptionKey) -> bool: """Convenience method to check if this is a project option.""" - return key.type is OptionType.PROJECT + return key in self.project_options def is_reserved_name(self, key: OptionKey) -> bool: - return not self.is_project_option(key) + if key.name in _BUILTIN_NAMES: + return True + # FIXME, this hack is needed until the lang field is removed from OptionKey. + if key.lang is not None: + return True + if '_' not in key.name: + return False + prefix = key.name.split('_')[0] + # Pylint seems to think that it is faster to build a set object + # and all related work just to test whether a string has one of two + # values. It is not, thank you very much. + if prefix in ('b', 'backend'): # pylint: disable=R6201 + return True + from .compilers import all_languages + if prefix in all_languages: + return True + return False def is_builtin_option(self, key: OptionKey) -> bool: """Convenience method to check if this is a builtin option.""" - return key.type is OptionType.BUILTIN + return key.name in _BUILTIN_NAMES or key.module def is_base_option(self, key: OptionKey) -> bool: """Convenience method to check if this is a base option.""" - return key.type is OptionType.BASE + return key.name.startswith('b_') def is_backend_option(self, key: OptionKey) -> bool: """Convenience method to check if this is a backend option.""" - return key.type is OptionType.BACKEND + return key.name.startswith('backend_') def is_compiler_option(self, key: OptionKey) -> bool: """Convenience method to check if this is a compiler option.""" - return key.type is OptionType.COMPILER + return key.lang is not None diff --git a/unittests/allplatformstests.py b/unittests/allplatformstests.py index 5e1635378..b96925e6a 100644 --- a/unittests/allplatformstests.py +++ b/unittests/allplatformstests.py @@ -3758,9 +3758,9 @@ class AllPlatformTests(BasePlatformTests): User defined options backend : ''' + self.backend_name + ''' + enabled_opt : enabled libdir : lib prefix : /usr - enabled_opt : enabled python : ''' + sys.executable + ''' ''') expected_lines = expected.split('\n')[1:] diff --git a/unittests/internaltests.py b/unittests/internaltests.py index 109f28ca1..bbdf2d9b7 100644 --- a/unittests/internaltests.py +++ b/unittests/internaltests.py @@ -34,7 +34,7 @@ from mesonbuild.mesonlib import ( LibType, MachineChoice, PerMachine, Version, is_windows, is_osx, is_cygwin, is_openbsd, search_version, MesonException, ) -from mesonbuild.options import OptionKey, OptionType +from mesonbuild.options import OptionKey from mesonbuild.interpreter.type_checking import in_set_validator, NoneType from mesonbuild.dependencies.pkgconfig import PkgConfigDependency, PkgConfigInterface, PkgConfigCLI from mesonbuild.programs import ExternalProgram @@ -1704,16 +1704,16 @@ class InternalTests(unittest.TestCase): def test_option_key_from_string(self) -> None: cases = [ - ('c_args', OptionKey('args', lang='c', _type=OptionType.COMPILER)), - ('build.cpp_args', OptionKey('args', machine=MachineChoice.BUILD, lang='cpp', _type=OptionType.COMPILER)), - ('prefix', OptionKey('prefix', _type=OptionType.BUILTIN)), - ('made_up', OptionKey('made_up', _type=OptionType.PROJECT)), + ('c_args', OptionKey('args', lang='c')), + ('build.cpp_args', OptionKey('args', machine=MachineChoice.BUILD, lang='cpp')), + ('prefix', OptionKey('prefix')), + ('made_up', OptionKey('made_up')), # TODO: the from_String method should be splitting the prefix off of # these, as we have the type already, but it doesn't. For now have a # test so that we don't change the behavior un-intentionally - ('b_lto', OptionKey('b_lto', _type=OptionType.BASE)), - ('backend_startup_project', OptionKey('backend_startup_project', _type=OptionType.BACKEND)), + ('b_lto', OptionKey('b_lto')), + ('backend_startup_project', OptionKey('backend_startup_project')), ] for raw, expected in cases: