From afb0fd4fe975df4c39fafc23e60c5eecda949734 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Thu, 12 Dec 2024 08:19:43 -0800 Subject: [PATCH] add error checking for invalid values Signed-off-by: Jade Abraham --- util/chplenv/chpl_atomics.py | 6 ++++-- util/chplenv/chpl_aux_filesys.py | 3 ++- util/chplenv/chpl_comm.py | 4 +++- util/chplenv/chpl_comm_ofi_oob.py | 5 ++--- util/chplenv/chpl_comm_segment.py | 4 +++- util/chplenv/chpl_comm_substrate.py | 4 +++- util/chplenv/chpl_gmp.py | 3 ++- util/chplenv/chpl_gpu.py | 4 ++-- util/chplenv/chpl_hwloc.py | 3 ++- util/chplenv/chpl_hwloc_pci.py | 3 ++- util/chplenv/chpl_jemalloc.py | 5 ++++- util/chplenv/chpl_launcher.py | 9 ++++++++- util/chplenv/chpl_lib_pic.py | 8 +++----- util/chplenv/chpl_libfabric.py | 4 +++- util/chplenv/chpl_llvm.py | 5 ++++- util/chplenv/chpl_locale_model.py | 8 ++------ util/chplenv/chpl_mem.py | 6 +++++- util/chplenv/chpl_re2.py | 6 +++--- util/chplenv/chpl_tasks.py | 4 +++- util/chplenv/chpl_timers.py | 3 ++- util/chplenv/chpl_unwind.py | 28 +++++++--------------------- util/chplenv/utils.py | 17 +++++++++++++++++ 22 files changed, 86 insertions(+), 56 deletions(-) diff --git a/util/chplenv/chpl_atomics.py b/util/chplenv/chpl_atomics.py index bfa14a264eed..98403a246156 100755 --- a/util/chplenv/chpl_atomics.py +++ b/util/chplenv/chpl_atomics.py @@ -4,7 +4,7 @@ import chpl_comm, chpl_compiler, chpl_platform, overrides from compiler_utils import CompVersion, get_compiler_version, has_std_atomics -from utils import error, memoize, warning +from utils import error, memoize, warning, check_valid_var @memoize @@ -21,7 +21,7 @@ def get(flag='target'): elif atomics_val == 'gasnet': error("CHPL_NETWORK_ATOMICS=gasnet is not supported") elif atomics_val not in valid: - error("CHPL_NETWORK_ATOMICS must be one of " + ','.join(valid)) + check_valid_var("CHPL_NETWORK_ATOMICS", atomics_val, valid) elif atomics_val != 'none' and atomics_val != comm_val: error("CHPL_NETWORK_ATOMICS=%s is incompatible with CHPL_COMM=%s" % (atomics_val, comm_val)) @@ -82,6 +82,8 @@ def get(flag='target'): # we can't use intrinsics, fall back to locks if not atomics_val: atomics_val = 'locks' + else: + check_valid_var("CHPL_ATOMICS", atomics_val, ['cstdlib', 'intrinsics', 'locks']) else: error("Invalid flag: '{0}'".format(flag), ValueError) diff --git a/util/chplenv/chpl_aux_filesys.py b/util/chplenv/chpl_aux_filesys.py index 20f1dfa7b06e..bda54b8b4ae9 100755 --- a/util/chplenv/chpl_aux_filesys.py +++ b/util/chplenv/chpl_aux_filesys.py @@ -4,12 +4,13 @@ from glob import glob import overrides -from utils import memoize +from utils import memoize, check_valid_var @memoize def get(): aux_fs = overrides.get('CHPL_AUX_FILESYS', 'none') + check_valid_var('CHPL_AUX_FILESYS', aux_fs, ['none', 'lustre']) return aux_fs diff --git a/util/chplenv/chpl_comm.py b/util/chplenv/chpl_comm.py index 0cd0cc1d9311..a1305bd9a1ca 100755 --- a/util/chplenv/chpl_comm.py +++ b/util/chplenv/chpl_comm.py @@ -2,7 +2,7 @@ import sys import chpl_platform, overrides -from utils import memoize +from utils import memoize, check_valid_var @memoize @@ -21,6 +21,8 @@ def get(): comm_val = 'gasnet' else: comm_val = 'none' + + check_valid_var("CHPL_COMM", comm_val, ("none", "gasnet", "ofi", "ugni")) return comm_val diff --git a/util/chplenv/chpl_comm_ofi_oob.py b/util/chplenv/chpl_comm_ofi_oob.py index ade8fff3b3e7..6fa8c53a9c3f 100755 --- a/util/chplenv/chpl_comm_ofi_oob.py +++ b/util/chplenv/chpl_comm_ofi_oob.py @@ -6,7 +6,7 @@ import overrides import third_party_utils -from utils import error, memoize +from utils import error, memoize, check_valid_var @memoize def get(): @@ -16,8 +16,7 @@ def get(): oob_val = overrides.get('CHPL_COMM_OFI_OOB') if oob_val: - if oob_val not in ('mpi', 'pmi2', 'sockets'): - error("CHPL_COMM_OFI_OOB must be 'mpi', 'pmi2', or 'sockets'") + check_valid_var("CHPL_COMM_OFI_OOB", oob_val, ("mpi", "pmi2", "sockets")) return oob_val # diff --git a/util/chplenv/chpl_comm_segment.py b/util/chplenv/chpl_comm_segment.py index a9f33331f77a..7da19d385f79 100755 --- a/util/chplenv/chpl_comm_segment.py +++ b/util/chplenv/chpl_comm_segment.py @@ -2,7 +2,7 @@ import sys import chpl_comm, chpl_comm_substrate, chpl_platform, overrides -from utils import memoize +from utils import memoize, check_valid_var @memoize @@ -21,6 +21,8 @@ def get(): segment_val = 'large' else: segment_val = 'everything' + else: + check_valid_var("CHPL_GASNET_SEGMENT", segment_val, ("none", "fast", "large", "everything")) else: segment_val = 'none' return segment_val diff --git a/util/chplenv/chpl_comm_substrate.py b/util/chplenv/chpl_comm_substrate.py index 7df2269b47d4..7fbd72bb082e 100755 --- a/util/chplenv/chpl_comm_substrate.py +++ b/util/chplenv/chpl_comm_substrate.py @@ -2,7 +2,7 @@ import sys import chpl_comm, chpl_platform, overrides -from utils import memoize +from utils import memoize, check_valid_var @memoize @@ -25,6 +25,8 @@ def get(): substrate_val = 'udp' else: substrate_val = 'none' + + check_valid_var("CHPL_COMM_SUBSTRATE", substrate_val, ("none", "aries", "ofi", "ibv", "udp", "mpi")) return substrate_val diff --git a/util/chplenv/chpl_gmp.py b/util/chplenv/chpl_gmp.py index 3c171bb2274e..a37a9ddd61aa 100755 --- a/util/chplenv/chpl_gmp.py +++ b/util/chplenv/chpl_gmp.py @@ -5,7 +5,7 @@ import chpl_compiler, chpl_platform, overrides, third_party_utils from chpl_home_utils import get_chpl_third_party -from utils import memoize, warning, error +from utils import memoize, warning, error, check_valid_var # returns True if CHPL_GMP was set by the user # (i.e. not inferred to be the default) @@ -36,6 +36,7 @@ def get(): else: gmp_val = 'none' + check_valid_var('CHPL_GMP', gmp_val, ['none', 'bundled', 'system']) return gmp_val @memoize diff --git a/util/chplenv/chpl_gpu.py b/util/chplenv/chpl_gpu.py index 41890d8c4699..b7aac70651a8 100644 --- a/util/chplenv/chpl_gpu.py +++ b/util/chplenv/chpl_gpu.py @@ -8,7 +8,7 @@ import chpl_tasks import chpl_home_utils import overrides -from utils import error, warning, memoize, try_run_command, which, is_ver_in_range +from utils import error, warning, memoize, try_run_command, which, is_ver_in_range, check_valid_var def _validate_cuda_version(): return _validate_cuda_version_impl() @@ -234,7 +234,7 @@ def get(): chpl_gpu_env = overrides.get("CHPL_GPU") if chpl_gpu_env: if chpl_gpu_env not in GPU_TYPES: - error("Only {} supported for 'CHPL_GPU'".format(list(GPU_TYPES.keys()))) + check_valid_var("CHPL_GPU", chpl_gpu_env, list(GPU_TYPES.keys())) else: return chpl_gpu_env else: diff --git a/util/chplenv/chpl_hwloc.py b/util/chplenv/chpl_hwloc.py index 2f6a0ee17c4e..52449129b652 100755 --- a/util/chplenv/chpl_hwloc.py +++ b/util/chplenv/chpl_hwloc.py @@ -5,7 +5,7 @@ import optparse import chpl_locale_model, chpl_tasks, chpl_platform, overrides, third_party_utils import chpl_hwloc_pci -from utils import error, memoize, warning, try_run_command, run_command +from utils import error, memoize, warning, try_run_command, run_command, check_valid_var @memoize @@ -18,6 +18,7 @@ def get(): else: hwloc_val = 'none' + check_valid_var('CHPL_HWLOC', hwloc_val, ['none', 'bundled', 'system']) return hwloc_val diff --git a/util/chplenv/chpl_hwloc_pci.py b/util/chplenv/chpl_hwloc_pci.py index 4655ffb943e4..7127f99593a2 100755 --- a/util/chplenv/chpl_hwloc_pci.py +++ b/util/chplenv/chpl_hwloc_pci.py @@ -13,7 +13,7 @@ import chpl_hwloc, chpl_comm, chpl_locale_model, chpl_gpu import overrides -from utils import error, memoize +from utils import error, memoize, check_valid_var @memoize def get(): @@ -33,6 +33,7 @@ def get(): gpu_val = chpl_gpu.get() if gpu_val != 'cpu': pci_val = 'enable' + check_valid_var('CHPL_HWLOC_PCI', pci_val, ['enable', 'disable']) return pci_val diff --git a/util/chplenv/chpl_jemalloc.py b/util/chplenv/chpl_jemalloc.py index 78761c1c6edc..13a5f03d5897 100644 --- a/util/chplenv/chpl_jemalloc.py +++ b/util/chplenv/chpl_jemalloc.py @@ -5,7 +5,7 @@ import chpl_bin_subdir, chpl_compiler, chpl_mem, chpl_platform, overrides, third_party_utils import homebrew_utils -from utils import error, memoize, run_command, warning +from utils import error, memoize, run_command, warning, check_valid_var @memoize @@ -56,6 +56,9 @@ def get(flag='target'): elif mem_val != 'jemalloc' and jemalloc_val != 'none': error("CHPL_JEMALLOC must be 'none' when CHPL_MEM is not jemalloc") + var_name = 'CHPL_{0}_JEMALLOC'.format(flag.upper()) + var_name = 'CHPL_JEMALLOC' if chpl_target_jemalloc is None else var_name + check_valid_var(var_name, jemalloc_val, ["none", "bundled", "system"]) return jemalloc_val diff --git a/util/chplenv/chpl_launcher.py b/util/chplenv/chpl_launcher.py index 9762b3d0bb07..bdf060b66566 100755 --- a/util/chplenv/chpl_launcher.py +++ b/util/chplenv/chpl_launcher.py @@ -3,7 +3,7 @@ import sys import chpl_comm, chpl_comm_substrate, chpl_platform, overrides -from utils import which, error, memoize, warning +from utils import which, error, memoize, warning, check_valid_var def slurm_prefix(base_launcher, platform_val): @@ -69,6 +69,13 @@ def get(): if launcher_val is None: launcher_val = 'none' + gasnet_launchers = ["mpi", "ibv", "ucx", "ofi"] + valid_values = ["none", "amudprun", "smp", "aprun", "slurm-srun"] + valid_values.extend(["lsf-gasnetrun_ibv", "mpirun", "mpirun4ofi", "pals", "pbs-aprun", "pbs-gasnetrun_ibv"]) + valid_values.extend(["gasnetrun_{}".format(l) for l in gasnet_launchers]) + valid_values.extend(["slurm-gasnetrun_{}".format(l) for l in gasnet_launchers]) + check_valid_var("CHPL_LAUNCHER", launcher_val, valid_values) + return launcher_val diff --git a/util/chplenv/chpl_lib_pic.py b/util/chplenv/chpl_lib_pic.py index 4be981b3325c..193186dd133d 100644 --- a/util/chplenv/chpl_lib_pic.py +++ b/util/chplenv/chpl_lib_pic.py @@ -3,14 +3,12 @@ import overrides import chpl_platform -from utils import memoize +from utils import memoize, check_valid_var @memoize def get(): - lib_pic_val = overrides.get('CHPL_LIB_PIC') - if not lib_pic_val: - lib_pic_val = 'none' - + lib_pic_val = overrides.get('CHPL_LIB_PIC', 'none') + check_valid_var("CHPL_LIB_PIC", lib_pic_val, ["none", "pic"]) return lib_pic_val diff --git a/util/chplenv/chpl_libfabric.py b/util/chplenv/chpl_libfabric.py index ea70bfa095d0..8a9117e39f5e 100755 --- a/util/chplenv/chpl_libfabric.py +++ b/util/chplenv/chpl_libfabric.py @@ -6,7 +6,7 @@ import chpl_comm, chpl_comm_debug, chpl_launcher, chpl_platform, chpl_comm_ofi_oob import overrides, third_party_utils -from utils import error, memoize, try_run_command, warning +from utils import error, memoize, try_run_command, warning, check_valid_var @memoize def get(): @@ -23,6 +23,8 @@ def get(): error("CHPL_LIBFABRIC must not be 'none' when CHPL_COMM is ofi") elif platform_val == 'hpe-cray-ex' and libfabric_val != 'system': warning('CHPL_LIBFABRIC!=system is discouraged on HPE Cray EX') + + check_valid_var('CHPL_LIBFABRIC', libfabric_val, ['bundled', 'system']) else: libfabric_val = 'none' diff --git a/util/chplenv/chpl_llvm.py b/util/chplenv/chpl_llvm.py index 673a9e97f2df..30bdd3f12a6c 100755 --- a/util/chplenv/chpl_llvm.py +++ b/util/chplenv/chpl_llvm.py @@ -9,7 +9,7 @@ from chpl_home_utils import get_chpl_third_party, get_chpl_home import chpl_gpu import homebrew_utils -from utils import which, memoize, error, run_command, try_run_command, warning +from utils import which, memoize, error, run_command, try_run_command, warning, check_valid_var from collections import defaultdict # returns a tuple of supported major LLVM versions as strings @@ -641,6 +641,9 @@ def has_compatible_installed_llvm(): @memoize def get(): llvm_val = overrides.get('CHPL_LLVM') + if llvm_val: + check_valid_var("CHPL_LLVM", llvm_val, ['none', 'bundled', 'system']) + if not llvm_val: llvm_val = 'unset' diff --git a/util/chplenv/chpl_locale_model.py b/util/chplenv/chpl_locale_model.py index 7cd09f891ff4..1fa960911558 100755 --- a/util/chplenv/chpl_locale_model.py +++ b/util/chplenv/chpl_locale_model.py @@ -2,17 +2,13 @@ import sys import overrides -from utils import memoize, error +from utils import memoize, error, check_valid_var @memoize def get(): locale_model_val = overrides.get('CHPL_LOCALE_MODEL', 'flat') - - if locale_model_val not in ['flat', 'gpu']: - error('{} is not a valid value for CHPL_LOCALE_MODEL. ' - 'It can only be "flat" or "gpu".'.format(locale_model_val)) - + check_valid_var("CHPL_LOCALE_MODEL", locale_model_val, ["flat", "shared"]) return locale_model_val diff --git a/util/chplenv/chpl_mem.py b/util/chplenv/chpl_mem.py index 3d0009b054af..668d39c54516 100755 --- a/util/chplenv/chpl_mem.py +++ b/util/chplenv/chpl_mem.py @@ -3,7 +3,7 @@ import sys import chpl_platform, overrides -from utils import error, memoize, warning +from utils import error, memoize, warning, check_valid_var @memoize def get(flag='host'): @@ -35,6 +35,10 @@ def get(flag='host'): mem_val = 'jemalloc' else: error("Invalid flag: '{0}'".format(flag), ValueError) + + var_name = 'CHPL_{0}_MEM'.format(flag.upper()) + var_name = 'CHPL_MEM' if chpl_target_mem is None else var_name + check_valid_var(var_name, mem_val, ["cstdlib", "jemalloc"]) return mem_val diff --git a/util/chplenv/chpl_re2.py b/util/chplenv/chpl_re2.py index c02d930f53a0..ba88c017043b 100755 --- a/util/chplenv/chpl_re2.py +++ b/util/chplenv/chpl_re2.py @@ -4,7 +4,7 @@ import chpl_compiler, chpl_platform, overrides, third_party_utils from chpl_home_utils import get_chpl_third_party -from utils import memoize, warning, error +from utils import memoize, warning, error, check_valid_var # returns True if CHPL_RE2 was set by the user @@ -21,14 +21,14 @@ def is_overridden(): @memoize def get(): re2 = overrides.get('CHPL_RE2') - if re2 == "system": - error("CHPL_RE2=system is not supported. Please use CHPL_RE2=bundled or CHL_RE2=none instead.") if not re2: re2_header = os.path.join(get_chpl_third_party(), 're2', 'install', get_uniq_cfg_path(), 'include', 're2', 're2.h') re2 = 'bundled' if os.path.exists(re2_header) else 'none' + + check_valid_var("CHPL_RE2", re2, ["none", "bundled"]) return re2 diff --git a/util/chplenv/chpl_tasks.py b/util/chplenv/chpl_tasks.py index a937a2678e90..972a2267f81a 100755 --- a/util/chplenv/chpl_tasks.py +++ b/util/chplenv/chpl_tasks.py @@ -4,7 +4,7 @@ import chpl_arch, chpl_compiler, chpl_platform, overrides from chpl_home_utils import using_chapel_module from compiler_utils import CompVersion -from utils import memoize +from utils import memoize, check_valid_var @memoize @@ -23,6 +23,8 @@ def get(): tasks_val = 'fifo' else: tasks_val = 'qthreads' + + check_valid_var("CHPL_TASKS", tasks_val, ("fifo", "qthreads")) return tasks_val diff --git a/util/chplenv/chpl_timers.py b/util/chplenv/chpl_timers.py index 0d32868da4a1..74c074f85948 100755 --- a/util/chplenv/chpl_timers.py +++ b/util/chplenv/chpl_timers.py @@ -2,12 +2,13 @@ import sys import overrides -from utils import memoize +from utils import memoize, check_valid_var @memoize def get(): timers_val = overrides.get('CHPL_TIMERS', 'generic') + check_valid_var("CHPL_TIMERS", timers_val, ["generic"]) return timers_val diff --git a/util/chplenv/chpl_unwind.py b/util/chplenv/chpl_unwind.py index ec741a94e63f..ab3171e5e1d0 100755 --- a/util/chplenv/chpl_unwind.py +++ b/util/chplenv/chpl_unwind.py @@ -2,35 +2,21 @@ import sys import chpl_platform, overrides, third_party_utils -from utils import error, memoize, warning +from utils import error, memoize, warning, check_valid_var @memoize def get(): platform_val = chpl_platform.get('target') - linux = platform_val.startswith('linux64') osx = platform_val.startswith('darwin') - val = overrides.get('CHPL_UNWIND') + val = overrides.get('CHPL_UNWIND', 'none') - if val and val not in ('none', 'bundled', 'system'): - error("Invalid CHPL_UNWIND value {0}\n" - "Valid values are none, bundled, or system".format(val)) + if osx and val == 'bundled': + error("Using CHPL_UNWIND=bundled is not supported on Mac OS X." + "\nUse CHPL_UNWIND=system instead.", ValueError) - if linux: - if val == 'bundled': - return 'bundled' - elif val == 'system': - return 'system' - if osx: - if val == 'bundled': - error("Using CHPL_UNWIND=bundled is not supported on Mac OS X." - "\nUse CHPL_UNWIND=system instead.", ValueError) - elif val == 'system': - return 'system' - if val: - return val - else: - return 'none' + check_valid_var('CHPL_UNWIND', val, ['none', 'bundled', 'system']) + return val @memoize def get_uniq_cfg_path(): diff --git a/util/chplenv/utils.py b/util/chplenv/utils.py index ae0512decf6b..467fb402fca6 100644 --- a/util/chplenv/utils.py +++ b/util/chplenv/utils.py @@ -181,6 +181,23 @@ def is_ver_in_range(versionStr, minimumStr, maximumStr): return False +def check_valid_var(varname, value, valid_values): + """Check that a variable is set to a valid value""" + + def join_words(words, conjunction='or'): + if len(words) == 1: + return words[0] + elif len(words) == 2: + return "{0} {2} {1}".format(words[0], words[1], conjunction) + else: + return "{0}, {2} {1}".format(", ".join(words[:-1]), words[-1], conjunction) + + if value not in valid_values: + error( + "{0}={1} is not valid, {0} must be {2}".format( + varname, value, join_words(valid_values) + ) + ) class _UtilsTests(unittest.TestCase): def test_is_ver_in_range(self):