-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix handling of v2 hashes #18522
base: dev
Are you sure you want to change the base?
Fix handling of v2 hashes #18522
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, this can't be right, why did you remove the build parsing ?
I only remove it from the function that parses targets from a string (comma separated list of tools and versions) which is used in The remaining code (e.g. the mulled container resolver etc) keeps the build as part of the version. I think that the tests which I added in my first commit show this (the version part of the hash differs when builds are added, i.e. the build is considered as part of the version when computing the hash). Wondering if this got a bit confusing when we merged |
1c0354c
to
727850b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I think this looks good to me now!
Anything else to do here? |
ea3180c
to
aa2660f
Compare
to verify if the build is considered as part of the version or not
the build information that may be contained in requirement versions is considered as part of the version in some places of the code - mulled container resolvers - mulled-build-tool - [planemo](https://github.com/galaxyproject/planemo/blob/b3e12a334e3d358ce30bf2c8c3f8d5a7a7e08136/planemo/conda.py#L116) in others it isn't: - mulled-hash - mulled-build-files - mulled_build which are the files that use the `target_str_to_targets` function. With this change the build info in version strings is always considered as part of the version. `mulled-build-files` is used in the multi-package-containers repo which led to wrong hashes in up to approx 50 tools (check with `grep "=[^,]*=" combinations/* | wc -l`).
aa2660f
to
f5310f6
Compare
Added tests first for the mulled container resolvers here 5b3c4bd
Then fixed (and more tests) here: e7bca83
See details in e7bca83
I'm tempted to backport this to 24.1 in order to make this effective in the multi-package-container repo.
How to test the changes?
(Select all options that apply)
License