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.

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

Env mod 4 #7536

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
c34fc02
FIX: avoid UnboundLocalError
mgsternberg Mar 20, 2018
3275f5f
fix slumbered flake8
mgsternberg Mar 20, 2018
fa0d4f0
suppress perpetual warning when using env-mod 4.x
mgsternberg Mar 20, 2018
5315a0d
typos; minor re-flow
mgsternberg Mar 20, 2018
359418d
FIX: TypeError/NoneType for blacklist
mgsternberg Mar 20, 2018
1dfceb9
Additional blacklist fortification; TODO on diff(from_dict, to_dict).
mgsternberg Mar 20, 2018
b92df22
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Mar 20, 2018
f35bdbc
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Apr 18, 2018
4578d39
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Apr 25, 2018
b074fe6
comments only in from_sourcing_file(): clarify interference from chil…
mgsternberg Apr 25, 2018
8ad014e
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Apr 27, 2018
95039a8
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg May 15, 2018
24e8a8d
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg May 16, 2018
d45ca4f
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg May 17, 2018
e1ab4dd
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg May 21, 2018
1d9f42a
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Jun 12, 2018
7f680ec
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Jun 13, 2018
c491913
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Jun 14, 2018
b09e620
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Jun 18, 2018
8b3520c
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Jul 3, 2018
7c09d4d
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Jul 12, 2018
a16a12b
roll back get_module_cmd() changes - superseded by #8570
mgsternberg Jul 12, 2018
0e0969a
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Aug 9, 2018
908a0c9
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Aug 13, 2018
fcf23e3
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Aug 14, 2018
600be7e
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Aug 15, 2018
f9ad898
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Aug 16, 2018
e567039
Merge remote branch 'upstream/develop' into env-mod-4
mgsternberg Aug 29, 2018
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
53 changes: 49 additions & 4 deletions lib/spack/spack/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,52 @@ def from_sourcing_file(filename, *args, **kwargs):
env_after = dict((k.encode('utf-8'), v.encode('utf-8'))
for k, v in env_after.items())

# Other variables unrelated to sourcing a file
blacklist.extend(['SHLVL', '_', 'PWD', 'OLDPWD', 'PS2'])
# TODO: Factor out remaining part of this function into something like:
# EnvironmentModifications.diff(from_dict, to_dict,
# clean=..., blacklist=..., whitelist=...)
# (Should be a natural fit, given the class name.)

# Variables unrelated to sourcing a file.
# ---------------------------------------------------------------------
# *This* blacklist is applied on INPUT (sourcing a shell file). It is
# distinct from "environment_blacklist" in modules.yaml, which is
# applied on OUTPUT to a specific module flavor.
# ---------------------------------------------------------------------
blacklist.extend(
Copy link
Member

Choose a reason for hiding this comment

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

If the name seems confusing, what do you think of renaming blacklist and whitelist to something that conveys better their role in this function and remove the comment above?

On a side not: I also slightly prefer to construct a list explicitly rather than splitting a string, given that we have items in it like _.

' SHLVL _ PWD OLDPWD PS2 ENV' # Bash internals
' LOADEDMODULES _LMFILES_ BASH_FUNC_module()' # env-mod-4
''.split())
# ---------------------------------------------------------------------
# Given the way we interpret a file here, namely, by having a child
# shell source it, there may be interference from any sort of
# re-initialization that is done in that shell's dotfiles. Environment
# variables may differ in the child shell from those in the parent
# shell that Spack was called from.
#
# Variables related to a user's chosen Modules implementation are of
# particular concern, since the results of the present function usually
# end up in a modulefile. This can be terribly confusing. Notably, in
# Environment-Modules-4.x, the definition of the "module" shell
# function, and thus the content of "BASH_FUNC_module()", varies
# between interactive and non-interactive shell sessions. See function
# renderAutoinit in
# https://github.com/cea-hpc/modules/blob/master/modulecmd.tcl.in
# (Tcl code), as of 2018-04:
#
# # only redirect module from stderr to stdout when session is
# # attached to a terminal to avoid breaking non-terminal session
# # (scp, sftp, etc)
# if {[isStderrTty]} {
# set fname "_moduleraw"
# } else {
# set fname "module"
# }
#
#
# NB2: "filename" could conceivably use some sort of "module" commands
# internally, but this should have little direct impact here; should
# specific blacklisting be needed, leave that to the caller.
# ---------------------------------------------------------------------

def set_intersection(fullset, *args):
# A set intersection using string literals and regexs
Expand Down Expand Up @@ -458,6 +502,7 @@ def return_separator_if_any(*args):
search = sep.join(after_list[start:end + 1])
except IndexError:
env.prepend_path(x, after)
continue # all set; plus: avoid UnboundLocalError

if search not in before:
# We just need to set the variable to the new value
Expand Down Expand Up @@ -534,8 +579,8 @@ def validate(env, errstream):


def filter_environment_blacklist(env, variables):
"""Generator that filters out any change to environment variables present in
the input list.
"""Generator that filters out any change to environment variables present
in the input list.

Args:
env: list of environment modifications
Expand Down
8 changes: 7 additions & 1 deletion lib/spack/spack/modules/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,13 @@ def environment_modifications(self):
pass
x.name = str(x.name).replace('-', '_')

return [(type(x).__name__, x) for x in env if x.name not in blacklist]
# blacklist WILL be None if the keyword is present in the YAML configs
# but without any entries or with all entries commented out.
if blacklist is None:
tty.debug('blacklist is defined but empty in your configuration.')

return [(type(x).__name__, x) for x in env if (
blacklist is None or x.name not in blacklist)]

@tengine.context_property
def autoload(self):
Expand Down