Reset runtest for components in a bundle

[ocaisa]

Follow up to #3748 for the case where it is have been shown to be problematic (components in a Pythonbundle)

[ocaisa]

@boegelbot please test @ jsc-zen3
EB_ARGS="purge_dups-1.2.6-foss-2023b.eb"

[boegelbot]

@ocaisa: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de

PR test command 'if [[ develop != 'develop' ]]; then EB_BRANCH=develop ./easybuild_develop.sh 2> /dev/null 1>&2; EB_PREFIX=/home/boegelbot/easybuild/develop source init_env_easybuild_develop.sh; fi; EB_PR=3775 EB_ARGS="purge_dups-1.2.6-foss-2023b.eb" EB_REPO=easybuild-easyblocks EB_BRANCH=develop /opt/software/slurm/bin/sbatch --job-name test_PR_3775 --ntasks=8 ~/boegelbot/eb_from_pr_upload_jsc-zen3.sh' executed!

• exit code: 0
• output:

Submitted batch job 6692

Test results coming soon (I hope)...

Details

- notification for comment with ID 2972323554 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

[boegelbot]

Test report by @boegelbot

Overview of tested easyconfigs (in order)

• SUCCESS purge_dups-1.2.6-foss-2023b.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
jsczen3c1.int.jsc-zen3.fz-juelich.de - Linux Rocky Linux 9.5, x86_64, AMD EPYC-Milan Processor (zen3), Python 3.9.21
See https://gist.github.com/boegelbot/c495d727cc74b01904b44871ac4059fd for a full test report.

[Flamefire]

I don't think this is the right solution. PythonPackage has a default of runtest = True which is valid.

So I think if you set this to None it would change the default, specifically set (see FooBundle below).

The problem IMO is (only?) PythonBundle changing defaults and they are (IMO correctly) propagated to components. With #3748 the runtest setting is now honored for components causing issues in Easyconfigs not prepared for that.

I think it might be better to remove the entry if present so components use their easyblock specific defaults.

In general it makes me wonder if this is the correct approach: Why would we reset/change a single property but not any other? What makes runtest special except that it is now honored?
IMO it is rather the easyconfigs that need changing/updating.

Consider for example a class FooBundle(Bundle) that sets runtest=True as the default to have it set for all components but with this change this is no longer possibly, is it?

Maybe we should rather only copy specific entries from the parent config and let default_component_specs handle the rest. Of course this will likely break other easyconfigs not prepared for that. I guess PythonBundle would work fine as it copies it's own settings to exts_default_options so components are left alon

[ocaisa]

I don't think what you're saying is exactly correct, this doesn't change the default values of the parent easyblock, it changes what is inherited by the components. The default value for runtest is None, as set in framework. All this does is restore that default, the easyblock used by the components can still override that, as can also be done by default_component_specs.

[ocaisa]

If you want to make a new easyblock that sets a default value for runtest, that is still possible, but it now needs explicit effort.

I think that it's unintuitive to have all the extra options of the parent easyblock passed to the component easyblock, but that's what is currently there. Fixing the issue with running tests for components just exposed that it can be problematic.

[Flamefire]

I don't think what you're saying is exactly correct, this doesn't change the default values of the parent easyblock, it changes what is inherited by the components. The default value for runtest is None, as set in framework. All this does is restore that default, the easyblock used by the components can still override that, as can also be done by default_component_specs.

IIRC on construction of an easyblock instance from an easyconfig it will only take the defaults if the easyconfig doesn't specify a value. But now you do specify a value: None.
So if you have a PythonPackage component runtest will no longer default to True but None as set here.
So component easyblocks can't overwrite the default via extra_options anymore. Or am I missing anything?

If you want to make a new easyblock that sets a default value for runtest, that is still possible, but it now needs explicit effort.

You mean that it now needs to use default_component_specs instead of extra_opts?

I think that it's unintuitive to have all the extra options of the parent easyblock passed to the component easyblock, but that's what is currently there. Fixing the issue with running tests for components just exposed that it can be problematic.

Fully agreed.

[ocaisa]

I could check it's value and explicitly log that I'm doing the reset, with advice about how to work around it?

[ocaisa]

I don't think what you're saying is exactly correct, this doesn't change the default values of the parent easyblock, it changes what is inherited by the components. The default value for runtest is None, as set in framework. All this does is restore that default, the easyblock used by the components can still override that, as can also be done by default_component_specs.

IIRC on construction of an easyblock instance from an easyconfig it will only take the defaults if the easyconfig doesn't specify a value. But now you do specify a value: None. So if you have a PythonPackage component runtest will no longer default to True but None as set here. So component easyblocks can't overwrite the default via extra_options anymore. Or am I missing anything?

Oh, you might be right about that, I should actually check. If removing it instead has the desired effect I'm fine with that. I am just afraid that that may also have unintended consequences.

[Flamefire]

Oh, you might be right about that, I should actually check.

Should be easy to check. And I guess there aren't many easyblocks changing the default so the (remaining) impact (of the now run test step) should be low.

I could check it's value and explicitly log that I'm doing the reset, with advice about how to work around it?

Might still be a good idea. How many Bundle-inherited easyblocks do we have changing the default of runtest? And how would the log detect the difference of intended vs unintended? I.e. how can we change PythonBundle to not trigger that message?
I have no idea so I'd add a comment here that this is a workaround for existing easyconfigs after a newly introduced change and in EB 6 only copy a couple options, if any at all.

[boegel]

lgtm

oops: nevermind, I overlooked the whole discussion...

needs another look

[boegel]

Test report by @boegel

Overview of tested easyconfigs (in order)

• SUCCESS FlexiBLAS-3.4.5-GCC-14.2.0.eb
• SUCCESS matplotlib-3.8.2-gfbf-2023b.eb

Build succeeded for 2 out of 2 (2 easyconfigs in total)
node3611.doduo.os - Linux RHEL 9.4, x86_64, AMD EPYC 7552 48-Core Processor (zen2), Python 3.9.18
See https://gist.github.com/boegel/a74cf3da82b5655dcd7f32278593a5bc for a full test report.

[ocaisa]

@Flamefire I've updated this a bit and tested it with some print statements when setting the default_component_easyblock to PythonPackage:

== processing EasyBuild easyconfig /home/ocaisa/EasyBuild/easybuild-easyblocks/easybuild/easyblocks/generic/purge_dups-1.2.6-foss-2023b.eb
== Running parse hook...
== Running parse hook...
INITIAL_VALUE._config = [True, 'Run unit tests.', (1, 'easyblock-specific')]
INITIAL_VALUE = True
RESET_VALUE._config = [None, 'Indicates if a test should be run after make; should specify argument after make (for e.g.,"test" for make test)', (3, 'build')]
RESET_VALUE = None
FINAL_VALUE._config = [True, 'Run unit tests.', (1, 'easyblock-specific')]
FINAL_VALUE = True
ERROR: Failed to get application instance for purge_dups (easyblock: PythonBundle): Use of unknown easyconfig parameter 'files_to_copy' when setting parameter value to '['bin', 'scripts']'

This works as much as I think it should. It still allows the component easyblock to make default changes to the value of runtest.

[ocaisa]

@boegelbot please test @ jsc-zen3
EB_ARGS="purge_dups-1.2.6-foss-2023b.eb"

[boegelbot]

@ocaisa: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de

PR test command 'if [[ develop != 'develop' ]]; then EB_BRANCH=develop ./easybuild_develop.sh 2> /dev/null 1>&2; EB_PREFIX=/home/boegelbot/easybuild/develop source init_env_easybuild_develop.sh; fi; EB_PR=3775 EB_ARGS="purge_dups-1.2.6-foss-2023b.eb" EB_REPO=easybuild-easyblocks EB_BRANCH=develop /opt/software/slurm/bin/sbatch --job-name test_PR_3775 --ntasks=8 ~/boegelbot/eb_from_pr_upload_jsc-zen3.sh' executed!

• exit code: 0
• output:

Submitted batch job 6760

Test results coming soon (I hope)...

Details

- notification for comment with ID 2976731242 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

[boegelbot]

Test report by @boegelbot

Overview of tested easyconfigs (in order)

• SUCCESS purge_dups-1.2.6-foss-2023b.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
jsczen3c3.int.jsc-zen3.fz-juelich.de - Linux Rocky Linux 9.5, x86_64, AMD EPYC-Milan Processor (zen3), Python 3.9.21
See https://gist.github.com/boegelbot/7ecbc1e7a0835ed4e4598e659b384316 for a full test report.

[ocaisa]

@boegelbot please test @ jsc-zen3
EB_ARGS="purge_dups-1.2.6-foss-2023b.eb FlexiBLAS-3.4.5-GCC-14.2.0.eb matplotlib-3.8.2-gfbf-2023b.eb --installpath /tmp/$USER/ebpr-3775"

[Flamefire]

@Flamefire I've updated this a bit and tested it with some print statements when setting the default_component_easyblock to PythonPackage:

This works as much as I think it should. It still allows the component easyblock to make default changes to the value of runtest.

Ok, should work well enough.

[boegelbot]

@ocaisa: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de

PR test command 'if [[ develop != 'develop' ]]; then EB_BRANCH=develop ./easybuild_develop.sh 2> /dev/null 1>&2; EB_PREFIX=/home/boegelbot/easybuild/develop source init_env_easybuild_develop.sh; fi; EB_PR=3775 EB_ARGS="purge_dups-1.2.6-foss-2023b.eb FlexiBLAS-3.4.5-GCC-14.2.0.eb matplotlib-3.8.2-gfbf-2023b.eb --installpath /tmp/$USER/ebpr-3775" EB_REPO=easybuild-easyblocks EB_BRANCH=develop /opt/software/slurm/bin/sbatch --job-name test_PR_3775 --ntasks=8 ~/boegelbot/eb_from_pr_upload_jsc-zen3.sh' executed!

• exit code: 0
• output:

Submitted batch job 6767

Test results coming soon (I hope)...

Details

- notification for comment with ID 2976783869 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

[ocaisa]

@boegelbot please test @ jsc-zen3
EB_ARGS="purge_dups-1.2.6-foss-2023b.eb"

[boegelbot]

@ocaisa: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de

PR test command 'if [[ develop != 'develop' ]]; then EB_BRANCH=develop ./easybuild_develop.sh 2> /dev/null 1>&2; EB_PREFIX=/home/boegelbot/easybuild/develop source init_env_easybuild_develop.sh; fi; EB_PR=3775 EB_ARGS="purge_dups-1.2.6-foss-2023b.eb" EB_REPO=easybuild-easyblocks EB_BRANCH=develop /opt/software/slurm/bin/sbatch --job-name test_PR_3775 --ntasks=8 ~/boegelbot/eb_from_pr_upload_jsc-zen3.sh' executed!

• exit code: 0
• output:

Submitted batch job 6772

Test results coming soon (I hope)...

Details

- notification for comment with ID 2976837810 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

[boegelbot]

Test report by @boegelbot

Overview of tested easyconfigs (in order)

• SUCCESS purge_dups-1.2.6-foss-2023b.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
jsczen3c2.int.jsc-zen3.fz-juelich.de - Linux Rocky Linux 9.5, x86_64, AMD EPYC-Milan Processor (zen3), Python 3.9.21
See https://gist.github.com/boegelbot/f3ce1842ece8a0345d323e6889393d1b for a full test report.

[boegelbot]

Test report by @boegelbot

Overview of tested easyconfigs (in order)

• SUCCESS matplotlib-3.8.2-gfbf-2023b.eb
• SUCCESS purge_dups-1.2.6-foss-2023b.eb
• SUCCESS AOCL-BLAS-5.0-GCC-14.2.0.eb
• SUCCESS FlexiBLAS-3.4.5-GCC-14.2.0.eb

Build succeeded for 4 out of 4 (3 easyconfigs in total)
jsczen3c2.int.jsc-zen3.fz-juelich.de - Linux Rocky Linux 9.5, x86_64, AMD EPYC-Milan Processor (zen3), Python 3.9.21
See https://gist.github.com/boegelbot/7b6b43388d43b62cf41a09c077f34a7b for a full test report.