Skip to content

Commit 3eded6c

Browse files
trwstrws
authored
strip -Werror: all specific or none (spack#30284)
Add a config option to strip `-Werror*` or `-Werror=*` from compile lines everywhere. ```yaml config: keep_werror: false ``` By default, we strip all `-Werror` arguments out of compile lines, to avoid unwanted failures when upgrading compilers. You can re-enable `-Werror` in your builds if you really want to, with either: ```yaml config: keep_werror: all ``` or to keep *just* specific `-Werror=XXX` args: ```yaml config: keep_werror: specific ``` This should make swapping in newer versions of compilers much smoother when maintainers have decided to enable `-Werror` by default.
1 parent 6a89eef commit 3eded6c

File tree

5 files changed

+127
-3
lines changed

5 files changed

+127
-3
lines changed

‎lib/spack/env/cc‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,8 @@ input_command="$*"
401401
# command line and recombine them with Spack arguments later. We
402402
# parse these out so that we can make sure that system paths come
403403
# last, that package arguments come first, and that Spack arguments
404-
# are injected properly.
404+
# are injected properly. Based on configuration, we also strip -Werror
405+
# arguments.
405406
#
406407
# All other arguments, including -l arguments, are treated as
407408
# 'other_args' and left in their original order. This ensures that
@@ -440,6 +441,29 @@ while [ $# -ne 0 ]; do
440441
continue
441442
fi
442443

444+
if [ -n "${SPACK_COMPILER_FLAGS_KEEP}" ] ; then
445+
# NOTE: the eval is required to allow `|` alternatives inside the variable
446+
eval "\
447+
case '$1' in
448+
$SPACK_COMPILER_FLAGS_KEEP)
449+
append other_args_list "$1"
450+
shift
451+
continue
452+
;;
453+
esac
454+
"
455+
fi
456+
if [ -n "${SPACK_COMPILER_FLAGS_REMOVE}" ] ; then
457+
eval "\
458+
case '$1' in
459+
$SPACK_COMPILER_FLAGS_REMOVE)
460+
shift
461+
continue
462+
;;
463+
esac
464+
"
465+
fi
466+
443467
case "$1" in
444468
-isystem*)
445469
arg="${1#-isystem}"

‎lib/spack/spack/build_environment.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,17 @@ def clean_environment():
242242
# show useful matches.
243243
env.set('LC_ALL', build_lang)
244244

245+
remove_flags = set()
246+
keep_flags = set()
247+
if spack.config.get('config:flags:keep_werror') == 'all':
248+
keep_flags.add('-Werror*')
249+
else:
250+
if spack.config.get('config:flags:keep_werror') == 'specific':
251+
keep_flags.add('-Werror=*')
252+
remove_flags.add('-Werror*')
253+
env.set('SPACK_COMPILER_FLAGS_KEEP', '|'.join(keep_flags))
254+
env.set('SPACK_COMPILER_FLAGS_REMOVE', '|'.join(remove_flags))
255+
245256
# Remove any macports installs from the PATH. The macports ld can
246257
# cause conflicts with the built-in linker on el capitan. Solves
247258
# assembler issues, e.g.:

‎lib/spack/spack/config.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,9 @@
105105
'build_stage': '$tempdir/spack-stage',
106106
'concretizer': 'clingo',
107107
'license_dir': spack.paths.default_license_dir,
108+
'flags': {
109+
'keep_werror': 'none',
110+
},
108111
}
109112
}
110113

‎lib/spack/spack/schema/config.py‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,16 @@
9191
'additional_external_search_paths': {
9292
'type': 'array',
9393
'items': {'type': 'string'}
94-
}
94+
},
95+
'flags': {
96+
'type': 'object',
97+
'properties': {
98+
'keep_werror': {
99+
'type': 'string',
100+
'enum': ['all', 'specific', 'none'],
101+
},
102+
},
103+
},
95104
},
96105
'deprecatedProperties': {
97106
'properties': ['module_roots'],

‎lib/spack/spack/test/cc.py‎

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212

1313
import pytest
1414

15+
import spack.build_environment
16+
import spack.config
17+
import spack.spec
1518
from spack.paths import build_env_path
1619
from spack.util.environment import set_env, system_dirs
1720
from spack.util.executable import Executable, ProcessError
@@ -129,7 +132,9 @@ def wrapper_environment():
129132
SPACK_TARGET_ARGS="-march=znver2 -mtune=znver2",
130133
SPACK_LINKER_ARG='-Wl,',
131134
SPACK_DTAGS_TO_ADD='--disable-new-dtags',
132-
SPACK_DTAGS_TO_STRIP='--enable-new-dtags'):
135+
SPACK_DTAGS_TO_STRIP='--enable-new-dtags',
136+
SPACK_COMPILER_FLAGS_KEEP='',
137+
SPACK_COMPILER_FLAGS_REMOVE='-Werror*',):
133138
yield
134139

135140

@@ -157,6 +162,21 @@ def check_args(cc, args, expected):
157162
assert expected == cc_modified_args
158163

159164

165+
def check_args_contents(cc, args, must_contain, must_not_contain):
166+
"""Check output arguments that cc produces when called with args.
167+
168+
This assumes that cc will print debug command output with one element
169+
per line, so that we see whether arguments that should (or shouldn't)
170+
contain spaces are parsed correctly.
171+
"""
172+
with set_env(SPACK_TEST_COMMAND='dump-args'):
173+
cc_modified_args = cc(*args, output=str).strip().split('\n')
174+
for a in must_contain:
175+
assert a in cc_modified_args
176+
for a in must_not_contain:
177+
assert a not in cc_modified_args
178+
179+
160180
def check_env_var(executable, var, expected):
161181
"""Check environment variables updated by the passed compiler wrapper
162182
@@ -642,6 +662,63 @@ def test_no_ccache_prepend_for_fc(wrapper_environment):
642662
common_compile_args)
643663

644664

665+
def test_keep_and_remove(wrapper_environment):
666+
werror_specific = ['-Werror=meh']
667+
werror = ['-Werror']
668+
werror_all = werror_specific + werror
669+
with set_env(
670+
SPACK_COMPILER_FLAGS_KEEP='',
671+
SPACK_COMPILER_FLAGS_REMOVE='-Werror*',
672+
):
673+
check_args_contents(cc, test_args + werror_all, ['-Wl,--end-group'], werror_all)
674+
with set_env(
675+
SPACK_COMPILER_FLAGS_KEEP='-Werror=*',
676+
SPACK_COMPILER_FLAGS_REMOVE='-Werror*',
677+
):
678+
check_args_contents(cc, test_args + werror_all, werror_specific, werror)
679+
with set_env(
680+
SPACK_COMPILER_FLAGS_KEEP='-Werror=*',
681+
SPACK_COMPILER_FLAGS_REMOVE='-Werror*|-llib1|-Wl*',
682+
):
683+
check_args_contents(
684+
cc,
685+
test_args + werror_all,
686+
werror_specific,
687+
werror + ["-llib1", "-Wl,--rpath"]
688+
)
689+
690+
691+
@pytest.mark.parametrize('cfg_override,initial,expected,must_be_gone', [
692+
# Set and unset variables
693+
('config:flags:keep_werror:all',
694+
['-Werror', '-Werror=specific', '-bah'],
695+
['-Werror', '-Werror=specific', '-bah'],
696+
[],
697+
),
698+
('config:flags:keep_werror:specific',
699+
['-Werror', '-Werror=specific', '-bah'],
700+
['-Werror=specific', '-bah'],
701+
['-Werror'],
702+
),
703+
('config:flags:keep_werror:none',
704+
['-Werror', '-Werror=specific', '-bah'],
705+
['-bah'],
706+
['-Werror', '-Werror=specific'],
707+
),
708+
])
709+
@pytest.mark.usefixtures('wrapper_environment', 'mutable_config')
710+
def test_flag_modification(cfg_override, initial, expected, must_be_gone):
711+
spack.config.add(cfg_override)
712+
env = spack.build_environment.clean_environment()
713+
env.apply_modifications()
714+
check_args_contents(
715+
cc,
716+
test_args + initial,
717+
expected,
718+
must_be_gone
719+
)
720+
721+
645722
@pytest.mark.regression('9160')
646723
def test_disable_new_dtags(wrapper_environment, wrapper_flags):
647724
with set_env(SPACK_TEST_COMMAND='dump-args'):

0 commit comments

Comments
 (0)