Skip to content

Don't skip sanity check for --module-only --rebuild#3645

Merged
ocaisa merged 2 commits intoeasybuilders:developfrom
Flamefire:module-build
May 7, 2021
Merged

Don't skip sanity check for --module-only --rebuild#3645
ocaisa merged 2 commits intoeasybuilders:developfrom
Flamefire:module-build

Conversation

@Flamefire
Copy link
Copy Markdown
Contributor

The only difference to now us that the sanity check is run when --rebuild is used.
For "proper" use, i.e. regenerating a module, this does not cause any problems and might even show issues that user was not aware of.
For those that used it for something else, the sanity check can fail and those need to use --force instead.
So that is IMO very low impact.

I'd even say it is a bugfix as the comment states: "allow skipping sanity check too when only generating module and force is used"

@boegelbot

This comment has been minimized.

@smoors
Copy link
Copy Markdown
Contributor

smoors commented Apr 28, 2021

I'm in favor of this change, it's indeed useful to still (re)do the sanity checks with --module-only.
it would be nice if the same could be done for extensions, as the sanity checks for those are also skipped currently.

@boegel boegel added the change label Apr 28, 2021
@boegel boegel added this to the next release (4.3.5?) milestone Apr 28, 2021
@boegel
Copy link
Copy Markdown
Member

boegel commented Apr 28, 2021

@smoors The sanity check for extensions are always skipped when using --module-only right now (even without --force or --rebuild), which is sort of a feature (although you could argue it's a bug).

If we change/fix that, we would definitely need an --skip-exts or something too.

@ocaisa
Copy link
Copy Markdown
Member

ocaisa commented Apr 28, 2021

I'm a bit confused by this because I spent all day yesterday using --module-only --rebuild exactly because it does the sanity checks.

I definitely believe this should be the case, in my case I would like to sanity check the EESSI installations on my own system (since I was building my own module tree stack I was just using --module-only, no --rebuild).

@Flamefire
Copy link
Copy Markdown
Contributor Author

@ocaisa : What exactly is the behavior you want? NOT running the sanity check when module-only is used? You can use --module-only --force for that. Everything else (e.g. --module-only) does run the sanity check (after this change also for --module-only --rebuild)
Maybe bring it up in the conf call in a few mins?

@ocaisa
Copy link
Copy Markdown
Member

ocaisa commented Apr 28, 2021

@ocaisa : What exactly is the behavior you want? NOT running the sanity check when module-only is used? You can use --module-only --force for that. Everything else (e.g. --module-only) does run the sanity check (after this change also for --module-only --rebuild)
Maybe bring it up in the conf call in a few mins?

I updated my comment, I was wrong in what I originally said. I definitely agree that running the sanity check should be done when combining --module-only and --rebuild, see easybuilders/easybuild-easyconfigs#12760 for why I think this would be useful

@smoors
Copy link
Copy Markdown
Contributor

smoors commented Apr 28, 2021

@smoors The sanity check for extensions are always skipped when using --module-only right now (even without --force or --rebuild), which is sort of a feature (although you could argue it's a bug).

that's why I proposed to change it so that --module-only --rebuild does all the checks (including for extensions), and --module-only --force skips all the checks (including for extensions).

If we change/fix that, we would definitely need an --skip-exts or something too.

sure, then you can do --module-only --rebuild --skip-exts to only do the global checks.

@ocaisa
Copy link
Copy Markdown
Member

ocaisa commented May 7, 2021

@Flamefire Do you think you will implement the --skip-exts option? For me this is a nice to have and if you don't have time we can create an issue for it.

Personally I believe you should run the sanity checks for extensions too. The R extensions in R/4.0.0-foss-2020a have changed multiple times, so if you built with 4.3.2 but do a rebuild with 4.3.4, the module will report modules that are in fact not installed.

@Flamefire
Copy link
Copy Markdown
Contributor Author

@ocaisa Makes sense to run the sanity check for extensions too. Not sure if or when I have time for that, so do create an issue for this and if someone works on that soon, then the better

@ocaisa ocaisa merged commit e0aac65 into easybuilders:develop May 7, 2021
@Flamefire Flamefire deleted the module-build branch May 7, 2021 16:32
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.

5 participants