lmod: fix use of custom separator in prepend_path etc.

[SteVwonder]

Fixes: #8736

[SteVwonder]

This PR now includes tests for the generation of the custom separator in lmod's {prepend,append,remove}_path.

[SteVwonder]

@alalazo: the travis tests on this PR are now passing. This is now ready for your (or anyone else's) review.

[alalazo]

💯 For the unit test. A comment on the template (please check and report back) and just a minor and optional remark on style.

[alalazo]

Looking at:

spack/lib/spack/spack/environment.py

Lines 54 to 59 in df1e233

	def __init__(self, name, value, **kwargs):
	self.name = name
	self.value = value
	self.separator = kwargs.get('separator', ':')
	self.args = {'name': name, 'value': value, 'separator': self.separator}
	self.args.update(kwargs)

it seems to me that cmd.separator will always be defined in the template. We can thus avoid the proliferation of conditionals in the template itself, by simply replacing:

prepend_path("{{ cmd.name }}", "{{ cmd.value }}")

with:

prepend_path("{{ cmd.name }}", "{{ cmd.value }}", "{{ cmd.separator }}")

This imo has the benefit of changing only 5 lines and not adding nested conditional logic. Can you try to see if this works?

[SteVwonder]

Sure! That is a much cleaner way of handling it. If it works, would you like me to propagate this change to the tcl module file as well (https://github.com/spack/spack/blob/develop/share/spack/templates/modules/modulefile.tcl#L49)? That is originally where I copied this control flow from.

[SteVwonder]

Reporting back that this does in fact work. 🎉

[alalazo]

Minor comment on style (not sure if everybody in the project here will agree, so feel free to discard this if you don't like it). This and the next condition can be written:

if re.match(r'[a-z]+_path\("COLON"', line):
    ....

without the additional is not None. I find this style more readable.

[alalazo]

LGTM. I'll test drive this asap and then merge.

[alalazo]

@SteVwonder Thanks!