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

Ability to change siteconfig #234

Closed
nanobowers opened this issue Mar 6, 2019 · 4 comments
Closed

Ability to change siteconfig #234

nanobowers opened this issue Mar 6, 2019 · 4 comments
Milestone

Comments

@nanobowers
Copy link
Contributor

We would like the ability to change the siteconfig from the installation path (which in many cases modules may be installed by root and unable for other users to add user recipes to the siteconfig).
I would propose control using an variable named $MODULES_SITECONFIG that would default to the existing value (@etcdir@/siteconfig.tcl) if the env-variable was not set.

The change is fairly trivial - but I am uncertain how to write the tests for this.

@xdelaruelle
Copy link
Collaborator

For the non-regression tests, I suggest to add them to the testsuite/modules.00-init/120-siteconfig.exp file.

if {[siteconfig_isStderrTty]} {

# no procedure superseding test, already handled in 100-pager tests

# test variable superseding
if { $verbose > 0 } {
    send_user "\tSetup TESTSUITE_ENABLE_SITECONFIG_DEBUG = '1'\n"
}
set env(TESTSUITE_ENABLE_SITECONFIG_DEBUG) 1

set ans [list]
lappend ans "DEBUG CALLING $MODULECMD sh -V"
lappend ans "DEBUG Source site configuration \\($siteconfig_filere\\)"
lappend ans "DEBUG initPager: start pager \\(asked_pager=-, cmd='$install_pager', opts='$install_pageropts'\\)"
lappend ans "Modules Release \[0-9a-zA-Z\.\+\\-\]+ \\(\[0-9\-\]{10}\\)"
testouterr_cmd_re "sh" "-V" "" [join $ans "\n"]

Just after that point, redo the same test multiple times, but each time with a different value set for MODULES_SITECONFIG:

  • MODULES_SITECONFIG=[pwd]/testsuite/example/siteconfig.tcl-1
  • MODULES_SITECONFIG=testsuite/example/siteconfig.tcl-1
  • MODULES_SITECONFIG=/path/to/unexistent/file
  • MODULES_SITECONFIG= (empty string)

Also do these 4 tests, on the other branch of the if {[siteconfig_isStderrTty]} { condition (which means when Modules installation does not come with a siteconfig.tcl file installed in etcdir

@xdelaruelle xdelaruelle added this to the 4.3 milestone Mar 8, 2019
@nanobowers
Copy link
Contributor Author

@xdelaruelle I have created some tests, but a few issues remain.

  • The tests only seem to take one branch of the
    if {[siteconfig_isStderrTty]} {, specifically the second branch
  • For some reason I get failures in the 100-pager.exp, though I made no edits on that file:
Running /home/ben/Dev/modules/nanobowers/modules/testsuite/modules.00-init/100-pager.exp ...
FAIL: -V -D (sh)
FAIL: -V -D --paginate (sh)
FAIL: -V -D --no-pager (sh)
FAIL: -V -D (sh)
FAIL: -V -D --paginate (sh)
FAIL: -V -D --no-pager (sh)
...

any thoughts on what I did to cause this?

  • I am not sure what the behavior of specifying MODULES_SITECONFIG to a missing file should be. Right now, modules will happily continue if the g_siteconfig specifies a non-existent file. It seems that if a user specifies a file, then there should be an expectation of its existence, but I am not sure of the overall project philosophy on this.

@xdelaruelle
Copy link
Collaborator

@xdelaruelle I have created some tests, but a few issues remain.

* The tests only seem to take one branch of the
  `if {[siteconfig_isStderrTty]} {`, specifically the second branch

To go through the first branch, a specific setup should be done by installing the testsuite/example/siteconfig.tcl-1 file in the target installation directory. This could be easily achieved with the following commands:

make install-testsiteconfig-1
export TESTSUITE_ENABLE_SITECONFIG=1
* For some reason I get failures in the 100-pager.exp, though I made no edits on that file:
Running /home/ben/Dev/modules/nanobowers/modules/testsuite/modules.00-init/100-pager.exp ...
FAIL: -V -D (sh)
FAIL: -V -D --paginate (sh)
FAIL: -V -D --no-pager (sh)
FAIL: -V -D (sh)
FAIL: -V -D --paginate (sh)
FAIL: -V -D --no-pager (sh)
...

any thoughts on what I did to cause this?

Pager tests look at Modules initialization debug information and currently expect there the siteconfig file to be the one setup at ./configure time. Should be adapted if a MODULES_SITECONFIG environment variable is found set.

I suggest you not to take that part into account. Once you will push your specific tests in testsuite/modules.00-init/120-siteconfig.exp, I will rework your commit to fix other test files that need to be updated.

@xdelaruelle
Copy link
Collaborator

I am not sure what the behavior of specifying MODULES_SITECONFIG to a missing file should be. Right now, modules will happily continue if the g_siteconfig specifies a non-existent file. It seems that if a user specifies a file, then there should be an expectation of its existence, but I am not sure of the overall project philosophy on this.

Let's follow the same approach than used for MODULERCFILE: no raised error and continue happily if MODULES_SITECONFIG points to a non-existent or more generally a non-readable file.

It makes me think that MODULES_SITECONFIG should be an additional file to source and not a replacement for the default siteconfig file whose location is set at ./configure time. So the default siteconfig should be sourced first, then the one defined by user with MODULES_SITECONFIG variable (so the user siteconfig could rely on variables or procedures defined by default siteconfig).

xdelaruelle pushed a commit to nanobowers/modules that referenced this issue May 9, 2019
Add the ability to define a site-specific configuration file with an
environment variable: MODULES_SITECONFIG. When set, the script file
pointed by the variable is sourced (if readable) after the site-specific
configuration file initially defined in modulecmd.tcl.

Fixes envmodules#234.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants