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

Allow templates in custom_paths & custom_commands sanity-check arguments #4679

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

Flamefire
Copy link
Contributor

E.g. for Python packages we (almost) always want to check for lib/python%(pyshortver)s and potentially add additional paths from EasyConfigs.
Currently a workaround is used that sets the parameter in the easyconfig when it isn't set already which is against the semantics of enhance_sanity_check.

This change allows this in a trivial way.

The added test just uses multi_deps which covers the case without it already, so no need for 2 similar ones.
To make it "realistic" add dummy Python ECs to use as dependencies such that we can use %(pyshortver)s

Requires

I included those PRs here though such that CI passes and either all commits can be reviewed together or just the last one which is the described change.

When using `multi_deps` the dry-run sanity check step should be run
multiple times just as is done in the real build.
However the decision which method to call is made before calling
`_sanity_check_step_multi_deps` and that doesn't check for dry-run.

Factor out a dispatch method that can be called during the iteration of
the multiple dependencies and use that in both cases.
`easyblocks_dir` already exists so no need to re-compute it as `new_dir`
…guments

E.g. for Python packages we (almost) always want to check for
`lib/python%(pyshortver)s` and potentially add additional paths from
EasyConfigs.
Currently a workaround is used that sets the parameter in the easyconfig
when it isn't set already which is against the semantics of `enhance_sanity_check`.
This change allows this in a trivial way.
@boegel boegel added this to the release after 4.9.4 milestone Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants