bugfix: without this, create_symlink would only be called based on the CLI argument, not the .yml setting #509
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e CLI argument, not the .yml setting
|
Makes perfect sense - thank you for catching this! |
vsoch
added a commit
that referenced
this pull request
Jun 2, 2022
* adding testing of a symlink tree install * unset symlink_home * missing module load * adding symlink_tree setting to indicate to always set also adding support for custom config on the fly with -c and for removing symlinks on uninstall * missing command argument * symlink_home -> symlink_tree and --symlink-tree instead of --symlink * forgot to add dest * ensure that we cleanup symlink directory of .version and empty * ensure we dont create symlink version unless tcl and default version is true * do not require symlink_base to be defined we can have a default of $root_dir/symlinks instead * bugfix: without this, create_symlink would only be called based on the CLI argument, not the .yml setting (#509) * verison bump * Minor refactoring of `check_symlink` (#510) * No need to create the symlink base directory here since it will be created by `create_symlink` * Make this part of the code symmetric with self.create_symlink() * Fixed a truncated sentence * running black * Skip `module` in the symlinks (#511) * Implemented "default_version" for TCL * Use write_version_file for the symlink tree too * Skip `module.tcl` in the symlinks This is done by symlinking `<software>/<version>` itself to `<namespace>/<software>/<version>/module.tcl`. For the directory of the wrapper scripts to be correctly found, the symlink has to be resolved, but TCL's `file normalize` won't normalise the filename. So, we need to use `file readlink` instead, but only on real symlinks because it raises an error. * Symlink to module.lua when possible which is when default_version==True (default_version==False can't be made to work with symlinks). * Added a `--force` option to `shpc install` to force overwriting existing symlinks The name `--force` is generic, so that other things could be forced through it, not just overwriting symlinks. Also added an info message if a symlink is overwritten, which can be hidden with the `--quiet` flag. * Made `force` optional * Forgot the variable for substitution * The "delete" command was superseded by "uninstall" in #6 * Added `--no-symlink-tree` to override the config file `--symlink-tree` now also overrides the config file * Make it explicit we are expecting yes or no * add force and symlink arg back in * do not pin black * Completion of the symlink feature (#539) * Removed leftover from an earlier implementation of `default_version` * bugfix: True and False are deprecated (but still valid) * Lmod symlinks can now skip "module" too thanks to @marcodelapierre * Added the missing newline character at the end of the file * bugfix: the return needs to happen after the creation of the symlink * `symlink_base` should allow environment variable expansion, like the other directories * Cannot consider using symlinks without the base defined * The two classes are meant to address exactly this * bugfix: the symlink needs to be cleaned regardless of where the modules are held * rmdir_to_base does the upwards clean-up correctly * No need to complain if symlinks are not enabled in the first place * bugfix: the caller of write_version_file needs to build the directory path All other calls were already doing it, this was the only exception. The change is required because not all .version files are in $module_base * bugfix: .version needs to be updated in the symlink tree too * bugfix: when uninstalling the last version of a tool there is no directory any more * Make the settings object upgrade legacy values so that the code only needs to know about the current ones * Expanded the function to deal with files and symlinks * Adding we can just use a filesystem loader and then from_string instead * Message update cf #502 (comment) * saving start of work on views this adds an shpc view command and refactors the symlink nomenclature to instead b about views! I need to write documentation and make a few additional commands to load a view from file and write tests for views too. * adding changes to docs and tests for views! * spelling mistake and missing tests helpers file * force needs to be specific for uninstall * remove deprecated symlink-tree * adding support to install from a file and list installed modules * legacy values is broken - remove for now * fixing bug with module version files - cannot do any funny business with creating module class on the fly! * module .version in symlink only should exist for lmod * fixing bug with uninstall and adding shpc view list * adding support to generate a view module a user can now do shpc view add <var> <val> to add customizations to a view! In practice this means that installed modules need to make an attempt to load the view and given that it is a symbolic link, it should find the file (at least we hope). This means in practice one view should be loaded at once, otherwise you will have two conflicting .view_module. * ensure we only use try-load for modules >= 4.8. I am also updating the testing to run for both an older and newer version (with and without support) * fixing bug with test I had changed the client variable to be module_sys instead of module to match the rest of the library, and forgot to update it in the tests helper script. * adding support for shpc view remove <name> <var> <value> and I am also adding better testing to look at both the config and file content of view_module in both cases Signed-off-by: vsoch <[email protected]> Co-authored-by: vsoch <[email protected]> Co-authored-by: Matthieu Muffato <[email protected]> Co-authored-by: Matthieu Muffato <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This solves the bug I had described in #502 (review) . The global setting
symlink_treewas ignored, and the only way to get the symlinks was the CLI option.In the end, it's got nothing to do with
symlink-treevssymlink_tree.create_symlinkis controlled by aif symlinkwheresymlinkis the CLI option only. Onlycheck_symlinkmerges it with the value fromshpc/settings.yml. With this change, the merge is done a level up ininstallitself, and benefits bothcreate_symlinkandif symlink