Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make setting CHPL_* environment variables to invalid values an error #26501

Merged
merged 2 commits into from
Mar 26, 2025
Merged

Make setting CHPL_* environment variables to invalid values an error #26501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c38899e
add error checking for invalid values
jabraham17 Dec 12, 2024
f36ab4a
apply reviewer feedback
jabraham17 Mar 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions util/chplenv/chpl_atomics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
Expand Down Expand Up @@ -82,6 +82,9 @@ def get(flag='target'):
# we can't use intrinsics, fall back to locks
if not atomics_val:
atomics_val = 'locks'

check_valid_var("CHPL_ATOMICS", atomics_val, ['cstdlib', 'intrinsics', 'locks'])

else:
error("Invalid flag: '{0}'".format(flag), ValueError)

Expand Down
3 changes: 2 additions & 1 deletion util/chplenv/chpl_aux_filesys.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
5 changes: 3 additions & 2 deletions util/chplenv/chpl_comm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
import re
import shutil

import overrides
from utils import memoize
import chpl_platform, overrides
from utils import memoize, check_valid_var


@memoize
Expand Down Expand Up @@ -38,6 +38,7 @@ def get():
else:
comm_val = 'none'

check_valid_var("CHPL_COMM", comm_val, ("none", "gasnet", "ofi", "ugni"))
return comm_val


Expand Down
49 changes: 23 additions & 26 deletions util/chplenv/chpl_comm_ofi_oob.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import overrides
import third_party_utils

from utils import error, memoize
from utils import error, memoize, check_valid_var

@memoize
def _find_pmi2():
Expand Down Expand Up @@ -41,33 +41,30 @@ def get():
return 'none'

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'")
return oob_val

#
# By default, use PMI2 out-of-band support on Cray X* and HPE Cray EX
# systems, MPI on other Cray systems or with an MPI-based launcher,
# and "sockets" otherwise.
#
platform_val = chpl_platform.get('target')
launcher_val = chpl_launcher.get()
if 'cray-x' in platform_val or chpl_platform.is_hpe_cray('target'):
oob_val = 'pmi2'
elif 'cray-' in platform_val:
oob_val = 'mpi'
elif 'mpi' in launcher_val:
oob_val = 'mpi'
else:
import chpl_compiler
if _find_pmi2() is not None:
oob_val = 'pmi2'
elif "-lpmi2" in chpl_compiler.get_system_link_args('target'):
if not oob_val:
#
# By default, use PMI2 out-of-band support on Cray X* and HPE Cray EX
# systems, MPI on other Cray systems or with an MPI-based launcher,
# and "sockets" otherwise.
#
platform_val = chpl_platform.get('target')
launcher_val = chpl_launcher.get()
if 'cray-x' in platform_val or chpl_platform.is_hpe_cray('target'):
oob_val = 'pmi2'
elif 'cray-' in platform_val:
oob_val = 'mpi'
elif 'mpi' in launcher_val:
oob_val = 'mpi'
else:
oob_val = 'sockets'

import chpl_compiler
if _find_pmi2() is not None:
oob_val = 'pmi2'
elif "-lpmi2" in chpl_compiler.get_system_link_args('target'):
oob_val = 'pmi2'
else:
oob_val = 'sockets'

check_valid_var("CHPL_COMM_OFI_OOB", oob_val, ("mpi", "pmi2", "sockets"))
return oob_val

# returns 2-tuple of lists
Expand Down
3 changes: 2 additions & 1 deletion util/chplenv/chpl_comm_segment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -21,6 +21,7 @@ def get():
segment_val = 'large'
else:
segment_val = 'everything'
check_valid_var("CHPL_GASNET_SEGMENT", segment_val, ("fast", "large", "everything"))
else:
segment_val = 'none'
return segment_val
Expand Down
6 changes: 4 additions & 2 deletions util/chplenv/chpl_comm_substrate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
import sys
import shutil

import chpl_comm, overrides
from utils import memoize
import chpl_comm, chpl_platform, overrides
from utils import memoize, check_valid_var


@memoize
Expand Down Expand Up @@ -38,6 +38,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


Expand Down
3 changes: 2 additions & 1 deletion util/chplenv/chpl_gmp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -36,6 +36,7 @@ def get():
else:
gmp_val = 'none'

check_valid_var('CHPL_GMP', gmp_val, ['none', 'bundled', 'system'])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, this checks it even if it was the default. You could do that everywhere but if not IMO it's better to put this up top with the conditional

if gmp_val:
  check_valid_var('CHPL_GMP', gmp_val, ['none', 'bundled', 'system'])
else;
  ... set default ...

Let's pick one strategy and try to use it throughout.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer checking only if we don't try to infer something. I will adjust accordingly

Copy link
Member Author

@jabraham17 jabraham17 Mar 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take that comment back. When writing this I used always checking more consistently and for good reason. Many of these scripts already have complex logic, this would make it worse. I am going make them all always check the valid values (except for something like checking CHPL_GASNET_SEGMENT only with gasnet)

return gmp_val

@memoize
Expand Down
4 changes: 2 additions & 2 deletions util/chplenv/chpl_gpu.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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:
Expand Down
3 changes: 2 additions & 1 deletion util/chplenv/chpl_hwloc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -18,6 +18,7 @@ def get():
else:
hwloc_val = 'none'

check_valid_var('CHPL_HWLOC', hwloc_val, ['none', 'bundled', 'system'])
return hwloc_val


Expand Down
3 changes: 2 additions & 1 deletion util/chplenv/chpl_hwloc_pci.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand All @@ -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


Expand Down
9 changes: 6 additions & 3 deletions util/chplenv/chpl_jemalloc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -53,11 +53,14 @@ def get(flag='target'):
elif darwin and mem_val == 'jemalloc' and jemalloc_val == 'bundled':
error("CHPL_HOST_JEMALLOC=bundled is not supported on Mac for host builds")

var_name = 'CHPL_{0}_JEMALLOC'.format(flag.upper())
mem_var_name = 'CHPL_{0}_MEM'.format(flag.upper())
if mem_val == 'jemalloc' and jemalloc_val == 'none':
error("CHPL_JEMALLOC must not be 'none' when CHPL_TARGET_MEM is jemalloc")
error("{0} must not be 'none' when {0} is jemalloc".format(var_name, mem_var_name))
elif mem_val != 'jemalloc' and jemalloc_val != 'none':
error("CHPL_JEMALLOC must be 'none' when CHPL_TARGET_MEM is not jemalloc")
error("{0} must be 'none' when {0} is not jemalloc".format(var_name, mem_var_name))

check_valid_var(var_name, jemalloc_val, ["none", "bundled", "system"])
return jemalloc_val


Expand Down
9 changes: 8 additions & 1 deletion util/chplenv/chpl_launcher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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


Expand Down
8 changes: 3 additions & 5 deletions util/chplenv/chpl_lib_pic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
9 changes: 5 additions & 4 deletions util/chplenv/chpl_libfabric.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand All @@ -18,10 +18,11 @@ def get():
libfabric_val = 'system'
else:
libfabric_val = 'bundled'
if libfabric_val == 'none':
error("CHPL_LIBFABRIC must not be 'none' when CHPL_COMM is ofi")
elif chpl_platform.is_hpe_cray('target') and libfabric_val != 'system':

check_valid_var('CHPL_LIBFABRIC', libfabric_val, ['bundled', 'system'])
if chpl_platform.is_hpe_cray('target') and libfabric_val != 'system':
warning('CHPL_LIBFABRIC!=system is discouraged on HPE Cray')

else:
libfabric_val = 'none'

Expand Down
5 changes: 4 additions & 1 deletion util/chplenv/chpl_llvm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'

Expand Down
8 changes: 2 additions & 6 deletions util/chplenv/chpl_locale_model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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", "gpu"])
return locale_model_val


Expand Down
5 changes: 4 additions & 1 deletion util/chplenv/chpl_mem.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'):
Expand Down Expand Up @@ -37,6 +37,9 @@ def get(flag='host'):
mem_val = 'jemalloc'
else:
error("Invalid flag: '{0}'".format(flag), ValueError)

var_name = 'CHPL_{0}_MEM'.format(flag.upper())
check_valid_var(var_name, mem_val, ["cstdlib", "jemalloc"])
return mem_val


Expand Down
Loading