From 4e71b3c32b8c98e2c16b8a340212f8477d53c6f3 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Tue, 23 Mar 2021 20:52:49 -0700 Subject: [PATCH] Split environment variable and command line cflags They are supposed to have different behavior. The environment variables apply to both the compiler and linker when the compiler acts as a linker, but the command line ones do not. Fixes #8345 --- mesonbuild/compilers/compilers.py | 11 +++++++---- mesonbuild/coredata.py | 11 +++++++++-- mesonbuild/environment.py | 23 ++++++++++++++++++++++- run_unittests.py | 22 ++++++++++++++++++++++ 4 files changed, 60 insertions(+), 7 deletions(-) diff --git a/mesonbuild/compilers/compilers.py b/mesonbuild/compilers/compilers.py index a37e4da0e..f02a687f1 100644 --- a/mesonbuild/compilers/compilers.py +++ b/mesonbuild/compilers/compilers.py @@ -1226,18 +1226,21 @@ def get_global_options(lang: str, description = 'Extra arguments passed to the {}'.format(lang) argkey = OptionKey('args', lang=lang, machine=for_machine) largkey = argkey.evolve('link_args') + envkey = argkey.evolve('env_args') cargs = coredata.UserArrayOption( description + ' compiler', env.options.get(argkey, []), split_args=True, user_input=True, allow_dups=True) + # the compiler args always gets the environemtn variable arguments + cargs.extend_value(env.options.get(envkey, [])) + largs = coredata.UserArrayOption( description + ' linker', env.options.get(largkey, []), split_args=True, user_input=True, allow_dups=True) - - # This needs to be done here, so that if we have string values in the env - # options that we can safely combine them *after* they've been split + # The linker gets the compiler environment variable only if the comiler + # acts as the linker if comp.INVOKES_LINKER: - largs.set_value(largs.value + cargs.value) + largs.extend_value(env.options.get(envkey, [])) opts: 'KeyedOptionDictType' = {argkey: cargs, largkey: largs} diff --git a/mesonbuild/coredata.py b/mesonbuild/coredata.py index a2c9c1a7b..e8a120004 100644 --- a/mesonbuild/coredata.py +++ b/mesonbuild/coredata.py @@ -243,6 +243,11 @@ class UserArrayOption(UserOption[T.List[str]]): ', '.join(bad), ', '.join(self.choices))) return newvalue + def extend_value(self, value: T.Union[str, T.List[str]]) -> None: + """Extend the value with an additional value.""" + new = self.validate_value(value) + self.set_value(self.value + new) + class UserFeatureOption(UserComboOption): static_choices = ['enabled', 'disabled', 'auto'] @@ -776,8 +781,10 @@ class CoreData: for_machine: MachineChoice, env: 'Environment') -> None: """Add global language arguments that are needed before compiler/linker detection.""" from .compilers import compilers - options = compilers.get_global_options(lang, comp, for_machine, env) - self.add_compiler_options(options, lang, for_machine, env) + # These options are all new at this point, because the compiler is + # responsible for adding its own options, thus calling + # `self.options.update()`` is perfectly safe. + self.options.update(compilers.get_global_options(lang, comp, for_machine, env)) def process_new_compiler(self, lang: str, comp: 'Compiler', env: 'Environment') -> None: from . import compilers diff --git a/mesonbuild/environment.py b/mesonbuild/environment.py index 756dd8193..742dff510 100644 --- a/mesonbuild/environment.py +++ b/mesonbuild/environment.py @@ -797,7 +797,11 @@ class Environment: env_opts: T.DefaultDict[OptionKey, T.List[str]] = collections.defaultdict(list) - for (evar, keyname), for_machine in itertools.product(opts, MachineChoice): + if self.is_cross_build(): + for_machine = MachineChoice.BUILD + else: + for_machine = MachineChoice.HOST + for evar, keyname in opts: p_env = _get_env_var(for_machine, self.is_cross_build(), evar) if p_env is not None: # these may contain duplicates, which must be removed, else @@ -833,6 +837,23 @@ class Environment: env_opts[key].extend(p_list) else: key = OptionKey.from_string(keyname).evolve(machine=for_machine) + if evar in compilers.compilers.CFLAGS_MAPPING.values(): + # If this is an environment variable, we have to + # store it separately until the compiler is + # instantiated, as we don't know whether the + # compiler will want to use these arguments at link + # time and compile time (instead of just at compile + # time) until we're instantiating that `Compiler` + # object. This is required so that passing + # `-Dc_args=` on the command line and `$CFLAGS` + # have subtely differen behavior. `$CFLAGS` will be + # added to the linker command line if the compiler + # acts as a linker driver, `-Dc_args` will not. + # + # We stil use the original key as the base here, as + # we want to inhert the machine and the compiler + # language + key = key.evolve('env_args') env_opts[key].extend(p_list) # Only store options that are not already in self.options, diff --git a/run_unittests.py b/run_unittests.py index 24a020eb3..46eb14f6d 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -5532,6 +5532,28 @@ class AllPlatformTests(BasePlatformTests): self.setconf('-Duse-sub=true') self.build() + def test_env_flags_to_linker(self) -> None: + # Compilers that act as drivers should add their compiler flags to the + # linker, those that do not shouldn't + with mock.patch.dict(os.environ, {'CFLAGS': '-DCFLAG', 'LDFLAGS': '-flto'}): + env = get_fake_env() + + # Get the compiler so we know which compiler class to mock. + cc = env.detect_compiler_for('c', MachineChoice.HOST) + cc_type = type(cc) + + # Test a compiler that acts as a linker + with mock.patch.object(cc_type, 'INVOKES_LINKER', True): + cc = env.detect_compiler_for('c', MachineChoice.HOST) + link_args = env.coredata.get_external_link_args(cc.for_machine, cc.language) + self.assertEqual(sorted(link_args), sorted(['-DCFLAG', '-flto'])) + + # And one that doesn't + with mock.patch.object(cc_type, 'INVOKES_LINKER', False): + cc = env.detect_compiler_for('c', MachineChoice.HOST) + link_args = env.coredata.get_external_link_args(cc.for_machine, cc.language) + self.assertEqual(sorted(link_args), sorted(['-flto'])) + class FailureTests(BasePlatformTests): '''