From 82fa4a350c2c03462d9f51de529b239d981efab2 Mon Sep 17 00:00:00 2001 From: Jonas Jucker Date: Wed, 3 Jan 2024 13:05:29 +0100 Subject: [PATCH 1/9] add variant extra-configure-args for ICON --- repos/c2sm/packages/icon/package.py | 23 +++++++++++++++++++++++ test/integration_test.py | 1 + 2 files changed, 24 insertions(+) diff --git a/repos/c2sm/packages/icon/package.py b/repos/c2sm/packages/icon/package.py index fbdfad4ac2..24e580372e 100755 --- a/repos/c2sm/packages/icon/package.py +++ b/repos/c2sm/packages/icon/package.py @@ -25,6 +25,13 @@ def check_variant_fcgroup(fcgroup): tty.warn('Variant fcgroup needs format GROUP.files.flag') return False +def check_variant_extra_configure_arg(extra_configure_arg): + pattern = re.compile(r'--(enable|disable)-\S+') + if pattern.match(extra_configure_arg) or extra_configure_arg == 'none': + return True + else: + tty.warn(f'The value "{extra_configure_arg}" for the extra_configure_args variant must follow the format "--enable-arg" or "--disable-arg"') + return False class Icon(AutotoolsPackage, CudaPackage): """Icosahedral Nonhydrostatic Weather and Climate Model.""" @@ -146,6 +153,15 @@ class Icon(AutotoolsPackage, CudaPackage): default=False, description='Enable ICON Testbed infrastructure') + variant( + 'extra-config-args', + default='none', + multi=True, + values=check_variant_extra_configure_arg, + description= + 'Inject any configure argument not yet available as variant' + ) + # Optimization Features: variant('loop-exchange', default=True, description='Enable loop exchange') variant('vectorized-lrtm', @@ -618,6 +634,13 @@ def configure_args(self): self.spec['py-gridtools-cpp:data'].headers.directories[0]) config_vars['GT4PYNVCFLAGS'] = config_vars['NVCFLAGS'] + # add configure arguments not yet available as variant + extra_config_args = self.spec.variants['extra-config-args'].value + if extra_config_args != ('none', ): + for x in extra_config_args: + config_args.append(x) + + # Finalize the LIBS variable (we always put the real collected # libraries to the front): config_vars['LIBS'].insert(0, libs.link_flags) diff --git a/test/integration_test.py b/test/integration_test.py index 7880e8bea5..f4e9477498 100644 --- a/test/integration_test.py +++ b/test/integration_test.py @@ -308,6 +308,7 @@ def test_icon(self): spack_spec('icon') spack_spec('icon serialization=create claw=std') spack_spec('icon fcgroup=DACE.externals/dace_icon.-O1') + spack_spec('icon extra_configure_args=--disable-new_feature,--enable-old_config_arg') def test_icon_ham(self): spack_spec('icon-ham') From 4616915c901324544607e3143a5c01445acb3508 Mon Sep 17 00:00:00 2001 From: github-actions Date: Wed, 3 Jan 2024 12:06:00 +0000 Subject: [PATCH 2/9] GitHub Action: Apply Pep8-formatting --- repos/c2sm/packages/icon/package.py | 10 ++++++---- test/integration_test.py | 4 +++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/repos/c2sm/packages/icon/package.py b/repos/c2sm/packages/icon/package.py index 24e580372e..2a55d7576b 100755 --- a/repos/c2sm/packages/icon/package.py +++ b/repos/c2sm/packages/icon/package.py @@ -25,14 +25,18 @@ def check_variant_fcgroup(fcgroup): tty.warn('Variant fcgroup needs format GROUP.files.flag') return False + def check_variant_extra_configure_arg(extra_configure_arg): pattern = re.compile(r'--(enable|disable)-\S+') if pattern.match(extra_configure_arg) or extra_configure_arg == 'none': return True else: - tty.warn(f'The value "{extra_configure_arg}" for the extra_configure_args variant must follow the format "--enable-arg" or "--disable-arg"') + tty.warn( + f'The value "{extra_configure_arg}" for the extra_configure_args variant must follow the format "--enable-arg" or "--disable-arg"' + ) return False + class Icon(AutotoolsPackage, CudaPackage): """Icosahedral Nonhydrostatic Weather and Climate Model.""" @@ -158,8 +162,7 @@ class Icon(AutotoolsPackage, CudaPackage): default='none', multi=True, values=check_variant_extra_configure_arg, - description= - 'Inject any configure argument not yet available as variant' + description='Inject any configure argument not yet available as variant' ) # Optimization Features: @@ -640,7 +643,6 @@ def configure_args(self): for x in extra_config_args: config_args.append(x) - # Finalize the LIBS variable (we always put the real collected # libraries to the front): config_vars['LIBS'].insert(0, libs.link_flags) diff --git a/test/integration_test.py b/test/integration_test.py index f4e9477498..d1e53cad26 100644 --- a/test/integration_test.py +++ b/test/integration_test.py @@ -308,7 +308,9 @@ def test_icon(self): spack_spec('icon') spack_spec('icon serialization=create claw=std') spack_spec('icon fcgroup=DACE.externals/dace_icon.-O1') - spack_spec('icon extra_configure_args=--disable-new_feature,--enable-old_config_arg') + spack_spec( + 'icon extra_configure_args=--disable-new_feature,--enable-old_config_arg' + ) def test_icon_ham(self): spack_spec('icon-ham') From ebac322dd253ff49812614b185dd1d2a4b091426 Mon Sep 17 00:00:00 2001 From: Jonas Jucker Date: Wed, 3 Jan 2024 13:19:33 +0100 Subject: [PATCH 3/9] fix wrong names --- repos/c2sm/packages/icon/package.py | 8 ++++---- test/integration_test.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/repos/c2sm/packages/icon/package.py b/repos/c2sm/packages/icon/package.py index 2a55d7576b..01ea2987de 100755 --- a/repos/c2sm/packages/icon/package.py +++ b/repos/c2sm/packages/icon/package.py @@ -26,13 +26,13 @@ def check_variant_fcgroup(fcgroup): return False -def check_variant_extra_configure_arg(extra_configure_arg): +def check_variant_extra_config_args(extra_config_arg): pattern = re.compile(r'--(enable|disable)-\S+') - if pattern.match(extra_configure_arg) or extra_configure_arg == 'none': + if pattern.match(extra_config_arg) or extra_config_arg == 'none': return True else: tty.warn( - f'The value "{extra_configure_arg}" for the extra_configure_args variant must follow the format "--enable-arg" or "--disable-arg"' + f'The value "{extra_config_arg}" for the extra_config_args variant must follow the format "--enable-arg" or "--disable-arg"' ) return False @@ -161,7 +161,7 @@ class Icon(AutotoolsPackage, CudaPackage): 'extra-config-args', default='none', multi=True, - values=check_variant_extra_configure_arg, + values=check_variant_extra_config_args, description='Inject any configure argument not yet available as variant' ) diff --git a/test/integration_test.py b/test/integration_test.py index d1e53cad26..d67ed6d42e 100644 --- a/test/integration_test.py +++ b/test/integration_test.py @@ -309,7 +309,7 @@ def test_icon(self): spack_spec('icon serialization=create claw=std') spack_spec('icon fcgroup=DACE.externals/dace_icon.-O1') spack_spec( - 'icon extra_configure_args=--disable-new_feature,--enable-old_config_arg' + 'icon extra_config_args=--disable-new_feature,--enable-old_config_arg' ) def test_icon_ham(self): From 171225ddd7d159d625a2c14db1ab90aed10b4216 Mon Sep 17 00:00:00 2001 From: juckerj <39263956+jonasjucker@users.noreply.github.com> Date: Wed, 3 Jan 2024 14:43:10 +0100 Subject: [PATCH 4/9] Update integration_test.py --- test/integration_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration_test.py b/test/integration_test.py index d67ed6d42e..6aa2a44432 100644 --- a/test/integration_test.py +++ b/test/integration_test.py @@ -309,7 +309,7 @@ def test_icon(self): spack_spec('icon serialization=create claw=std') spack_spec('icon fcgroup=DACE.externals/dace_icon.-O1') spack_spec( - 'icon extra_config_args=--disable-new_feature,--enable-old_config_arg' + 'icon extra-config-args=--disable-new_feature,--enable-old_config_arg' ) def test_icon_ham(self): From e87e610f0682b1dcfc99207fc66e79621de8a898 Mon Sep 17 00:00:00 2001 From: Jonas Jucker Date: Thu, 4 Jan 2024 09:38:07 +0100 Subject: [PATCH 5/9] safety check --- repos/c2sm/packages/icon/package.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/repos/c2sm/packages/icon/package.py b/repos/c2sm/packages/icon/package.py index 01ea2987de..2dc7fb8c32 100755 --- a/repos/c2sm/packages/icon/package.py +++ b/repos/c2sm/packages/icon/package.py @@ -641,6 +641,9 @@ def configure_args(self): extra_config_args = self.spec.variants['extra-config-args'].value if extra_config_args != ('none', ): for x in extra_config_args: + # prevent configure-args already available as variant + # to be set through variant extra_config_args + self.validate_extra_config_args(x) config_args.append(x) # Finalize the LIBS variable (we always put the real collected @@ -681,6 +684,22 @@ def fcgroup_to_config_var(self): var[f'ICON_{name}_FCFLAGS'] = [flag] return var + def strip_variant_prefix(self,variant_string): + prefixes = ["--enable-", "--disable-"] + + for prefix in prefixes: + if variant_string.startswith(prefix): + return variant_string[len(prefix):] + + raise ValueError + + def validate_extra_config_args(self,arg): + variant_from_arg = self.strip_variant_prefix(arg) + if variant_from_arg in self.spec.variants: + raise error.SpecError(f'The value "{arg}" for the extra_config_args variant conflicts ' + f'with the existing variant {variant_from_arg}. Set this variant instead.') + + @run_after('configure') def adjust_rttov_macro(self): if '+rttov' in self.spec: From bb401681539e77c7fedf5f6aecea504cc096d28b Mon Sep 17 00:00:00 2001 From: Jonas Jucker Date: Thu, 4 Jan 2024 09:45:18 +0100 Subject: [PATCH 6/9] place another warning --- repos/c2sm/packages/icon/package.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repos/c2sm/packages/icon/package.py b/repos/c2sm/packages/icon/package.py index 2dc7fb8c32..6bd2155202 100755 --- a/repos/c2sm/packages/icon/package.py +++ b/repos/c2sm/packages/icon/package.py @@ -162,7 +162,7 @@ class Icon(AutotoolsPackage, CudaPackage): default='none', multi=True, values=check_variant_extra_config_args, - description='Inject any configure argument not yet available as variant' + description='Inject any configure argument not yet available as variant\nUse this feature cautiously, as injecting non-variant configure arguments may potentially disrupt the build process' ) # Optimization Features: From b0b5a0ffb316271255be7afe2481951cf495d35a Mon Sep 17 00:00:00 2001 From: github-actions Date: Thu, 4 Jan 2024 08:45:48 +0000 Subject: [PATCH 7/9] GitHub Action: Apply Pep8-formatting --- repos/c2sm/packages/icon/package.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/repos/c2sm/packages/icon/package.py b/repos/c2sm/packages/icon/package.py index 6bd2155202..3f703b9588 100755 --- a/repos/c2sm/packages/icon/package.py +++ b/repos/c2sm/packages/icon/package.py @@ -162,7 +162,8 @@ class Icon(AutotoolsPackage, CudaPackage): default='none', multi=True, values=check_variant_extra_config_args, - description='Inject any configure argument not yet available as variant\nUse this feature cautiously, as injecting non-variant configure arguments may potentially disrupt the build process' + description= + 'Inject any configure argument not yet available as variant\nUse this feature cautiously, as injecting non-variant configure arguments may potentially disrupt the build process' ) # Optimization Features: @@ -684,7 +685,7 @@ def fcgroup_to_config_var(self): var[f'ICON_{name}_FCFLAGS'] = [flag] return var - def strip_variant_prefix(self,variant_string): + def strip_variant_prefix(self, variant_string): prefixes = ["--enable-", "--disable-"] for prefix in prefixes: @@ -692,13 +693,14 @@ def strip_variant_prefix(self,variant_string): return variant_string[len(prefix):] raise ValueError - - def validate_extra_config_args(self,arg): + + def validate_extra_config_args(self, arg): variant_from_arg = self.strip_variant_prefix(arg) if variant_from_arg in self.spec.variants: - raise error.SpecError(f'The value "{arg}" for the extra_config_args variant conflicts ' - f'with the existing variant {variant_from_arg}. Set this variant instead.') - + raise error.SpecError( + f'The value "{arg}" for the extra_config_args variant conflicts ' + f'with the existing variant {variant_from_arg}. Set this variant instead.' + ) @run_after('configure') def adjust_rttov_macro(self): From 4128b0e0884f055567a4ef3edd5069fb37cba3af Mon Sep 17 00:00:00 2001 From: Jonas Jucker Date: Thu, 4 Jan 2024 10:00:27 +0100 Subject: [PATCH 8/9] another warning --- repos/c2sm/packages/icon/package.py | 1 + 1 file changed, 1 insertion(+) diff --git a/repos/c2sm/packages/icon/package.py b/repos/c2sm/packages/icon/package.py index 3f703b9588..d7f52c226e 100755 --- a/repos/c2sm/packages/icon/package.py +++ b/repos/c2sm/packages/icon/package.py @@ -646,6 +646,7 @@ def configure_args(self): # to be set through variant extra_config_args self.validate_extra_config_args(x) config_args.append(x) + tty.warn('You use variant extra-config-args. Injecting non-variant configure arguments may potentially disrupt the build process!') # Finalize the LIBS variable (we always put the real collected # libraries to the front): From 37678e20457275c947af66cec6eff996999ce3d0 Mon Sep 17 00:00:00 2001 From: github-actions Date: Thu, 4 Jan 2024 09:00:53 +0000 Subject: [PATCH 9/9] GitHub Action: Apply Pep8-formatting --- repos/c2sm/packages/icon/package.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/repos/c2sm/packages/icon/package.py b/repos/c2sm/packages/icon/package.py index d7f52c226e..2a2afb6e4a 100755 --- a/repos/c2sm/packages/icon/package.py +++ b/repos/c2sm/packages/icon/package.py @@ -646,7 +646,9 @@ def configure_args(self): # to be set through variant extra_config_args self.validate_extra_config_args(x) config_args.append(x) - tty.warn('You use variant extra-config-args. Injecting non-variant configure arguments may potentially disrupt the build process!') + tty.warn( + 'You use variant extra-config-args. Injecting non-variant configure arguments may potentially disrupt the build process!' + ) # Finalize the LIBS variable (we always put the real collected # libraries to the front):