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

Regression with versionsuffix types #4281

Closed
mboisson opened this issue Jun 13, 2023 · 3 comments · Fixed by #4292
Closed

Regression with versionsuffix types #4281

mboisson opened this issue Jun 13, 2023 · 3 comments · Fixed by #4292
Milestone

Comments

@mboisson
Copy link
Contributor

Commit 0e5ba5c858
introduced a check for string-type for versionsuffix, while None used to be an accepted value for versionsuffix. Our hooks replace many version suffixes with None.

@boegel boegel added this to the next release (4.7.3?) milestone Jun 13, 2023
@Flamefire
Copy link
Contributor

Interestingly every falsy value was accepted. So next to empty string and None also False or an empty list.
I don't see how anything but string and None make sense here

So might be ok to break a potential abuse and only allow None next to the string type.

However the dependency dictionary spec converts a versionsuffix unconditionally into a string:

if key == 'versionsuffix':
depspec[key] = str(dep[key])

So when there versionSuffix: None is used the result will be "None", so maybe we should keep this small breaking change as the error message is very clear and replacing the hook to use an empty string works always.

@mboisson
Copy link
Contributor Author

In the immediate, I will just hot-fix our copy of framework to remove that check (or allow None)

mboisson added a commit to ComputeCanada/easybuild-framework that referenced this issue Jun 16, 2023
@mboisson
Copy link
Contributor Author

ComputeCanada@59d2f54

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

Successfully merging a pull request may close this issue.

3 participants