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

Better Handling of Now-Forbidden Modulefile Names #527

Closed
ikirker opened this issue Apr 15, 2024 · 3 comments
Closed

Better Handling of Now-Forbidden Modulefile Names #527

ikirker opened this issue Apr 15, 2024 · 3 comments
Milestone

Comments

@ikirker
Copy link

ikirker commented Apr 15, 2024

Is your feature request related to a problem? Please describe.

We recently switched our default version of modules from the ancient 3.2.6 to 5.3.1 on one of our clusters, and got an uncomfortable surprise when we discovered that our Temurin Java modules weren’t compatible with the new restrictions on naming made by the addition of module variants. Temurin versions are named as, for example, 21.0.2+13, and docs do state that the + character is no longer allowed in module names.

The way we discovered this, though, was that a module named java/temurin-17/17.0.2+8 would load on some partial matches, but not on a full match. This gives strange results like:

$ module path java/temurin-17
/shared/ucl/apps/modulefiles/development/java/temurin-17/17.0.2+8

$ module path java/temurin-17/17
/shared/ucl/apps/modulefiles/development/java/temurin-17/17.0.2+8

$ module path java/temurin-17/17.0
/shared/ucl/apps/modulefiles/development/java/temurin-17/17.0.2+8

$ module path java/temurin-17/17.0.2
ERROR: Unable to locate a modulefile for 'java/temurin-17/17.0.2'

$ module path java/temurin-17/17.0.2+8
ERROR: Unable to locate a modulefile for 'java/temurin-17/17.0.2+8'

$ module load /shared/ucl/apps/modulefiles/development/java/temurin-17/17.0.2+8
Loading /shared/ucl/apps/modulefiles/development/java/temurin-17/17.0.2+8{+8}
  ERROR: Unknown variant '8' specified
  ERROR: Unknown variant '8' specified

Describe the solution you'd like

I guess we’re just going to have to rename the modulefile on our end, but this seems like a bigger consistency problem that could use a fix, or at least a warning of some kind. I’m not sure what it should be, though, and I’m not really familiar with the module variant syntax itself. Clearly modulecmd is identifying and parsing the relevant file as a modulefile, but then has problems with the syntax on use, so perhaps as a first step it would make sense to print a warning or error when a modulefile containing illegal characters is parsed?

Describe alternatives you've considered

I'm happy to discuss alternative and additional ways to handle problems around this, but since the module variant syntax seems fully integrated into the module name handling, I'm not sure what to suggest.

I could imagine CLI options to, for example, force exact matching, or enable/disable variant parsing, but that doesn't solve the weirdness problem in the examples above.

TLDR

Because:

  • partial matching + variant parsing seems to behave weirdly
  • modulecmd parses modulefiles with illegal names
  • modulecmd does not warn the user when it encounters illegal names
  • upgrading from a version without variants to a version with variants can make existing module names unusable

It would be a good idea to:

  • produce a message/error when encountering illegal names
  • look into options to make partial matching + variant parsing behave less weirdly
@xdelaruelle
Copy link
Member

Many thanks for your clear report. I will check if the situation can be improved.

In the mean time, if you do not use variants and other version specifiers, you may disable this feature to avoid the issue you encounter with your modulefile names:

$ module config advanced_version_spec 0

@ikirker
Copy link
Author

ikirker commented Apr 18, 2024

Thank you for the pointer to this option: we'll give this a try since we've not used any of the variants and other advanced specs yet. Maybe once we've learned more about the feature and its use cases, we'll change it back and rename any problematic modules.

@xdelaruelle
Copy link
Member

I have just finished some adaptations to the naming rules for variants:

  • a variant name should only contain characters within the A-Za-z0-9_- range
  • it cannot start with - character
  • the overall name cannot just be a number

In addition, module specification parsing has been adapted to consider chunks starting with + or ~ characters as part of module name and version if these chunks do not correspond to a valid variant name.

As a result the java/temurin-17/17.0.2+8 module specification will now be considered as a module name and version without variant specification.

These changes will be shipped in next feature release (v5.5).

@xdelaruelle xdelaruelle added this to the 5.5.0 milestone May 24, 2024
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