Skip to content

Conversation

Flamefire
Copy link
Contributor

Store where sources have been added to get the corresponding filled structure without relying on partially resolved templates.

Also do not fetch component patches twice

I've also move some stuff around to make the structure clearer:

  • fetch_step now fetches patches instead of doing that (again) in the install step
  • Set properties like dirs and parallel in prepare step as done in EasyBlock
  • Set the finalpath of the first source as stated by the comment. Most (all?) components only have a single source so this shouldn't cause any issues.

Note that this manual setting of finalpath isn't logically correct as start_dir of a component could be the builddir or the extracted source of some component

guess_start_dir will always get the builddir (due to no sources present), potentially with a suffix as specified in the easyconfigs which many do (as foo-1.2.3) because the extracted source isn't available and the finalpath for the 2nd component and all following is wrong until easybuilders/easybuild-framework#4922

But I don't see how to fix this without breaking existing easyconfigs and easyblocks.

Store where sources have been added to get the corresponding filled
structure without relying on partially resolved templates.
@Flamefire Flamefire force-pushed the bundle-fetch-step branch from bc75ff2 to 882177c Compare June 16, 2025 14:56
@Thyre
Copy link
Collaborator

Thyre commented Jun 17, 2025

Regarding

Set the finalpath of the first source as stated by the comment. Most (all?) components only have a single source so this shouldn't cause any issues

easybuilders/easybuild-easyconfigs#21582 sets two sources for the Intel compilers. However, if I understand correctly, finalpath is mostly used for patches, which this PR doesn't have. I'll test this PR with your changes later.

@Flamefire
Copy link
Contributor Author

easybuilders/easybuild-easyconfigs#21582 sets two sources for the Intel compilers. However, if I understand correctly, finalpath is mostly used for patches, which this PR doesn't have. I'll test this PR with your changes later.

Yes it is used for patches and for guess_start_dir. The Bundle easyblock (wrongly) doesn't use it for the latter. Patches can specify a source index to apply in a different sources finalpath. But I think this is broken for everything until easybuilders/easybuild-framework#4922 as the 2nd sources finalpath will be the builddir. So I doubt it's used

I feel like we need a new Bundle easyblock which fixes all the quirks of the current one:

  • finalpaths not determined correctly
  • start_dir of components relative to builddir instead of extracted source
  • skipping of multiple, possibly important, steps like prepare_step

It looks like you can just put an easyconfig into a component and it would work as-if standalone but it doesn't in subtile ways due to this. And I don't see a way to fix it which would be backwards compatible.

So this PR just reduces chances for one surprise

@Thyre
Copy link
Collaborator

Thyre commented Jun 17, 2025

I think we're slowly but surely finding out which parts of the Bundle EasyBlock are broken. I agree, that maybe starting a new Bundle EasyBlock rewriting everything from the ground up might be needed to fix all of these lingering issues. This will take quite some time, and until then, trying to fix or work around the existing issues is a good idea...

@Flamefire
Copy link
Contributor Author

I think we're slowly but surely finding out which parts of the Bundle EasyBlock are broken. I agree, that maybe starting a new Bundle EasyBlock rewriting everything from the ground up might be needed to fix all of these lingering issues. This will take quite some time, and until then, trying to fix or work around the existing issues is a good idea...

As for prepare_step: I cannot see how it would be useful to run this for bundle components as it e.g. sets up the toolchain which I guess would cause issues if done again.

I propose to disallow easyblocks overwriting prepare_step as Bundle components as a workaround. I'm quite sure the current uses are rather "abuse" and configure_step or __init__ can be used instead

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.

3 participants