Skip to content

Conversation

@Flamefire
Copy link
Contributor

That does additional configuration required for Rust crates with #3995

That does additional configuration required for Rust crates
@boegel
Copy link
Member

boegel commented Dec 3, 2025

@Flamefire Can you clarify (in the PR description, and with a comment in the easyblock) why this is needed?

@Flamefire
Copy link
Contributor Author

It was "just missing". So the question worth a comment would rather by why it does not need to be this way.

If we don't do this we only call the configure_step of PythonPackage, not of Cargo which isn't what we want, do we?

IIRC how the Python MRO works we could always use super().configure_step() and it would end up calling each configure method exactly once. But that would need to be done in the whole stack, i.e. at least also in PythonPackage which seems like a too big of a change right now. We'd likely need to test all easyblocks deriving from PythonPackage and add a (dummy) configure_step in EasyBlock

Currently MRO will find PythonPackage.configure_step´ and runs that. And as that doesn't call any other configure_step` none is run.

@boegel boegel changed the title Call Cargo configure step from CargoPythonPackage Explictly call PythonPackage and Cargo configure step in CargoPythonPackage easyblock Dec 13, 2025
@boegel boegel added enhancement and removed change labels Dec 13, 2025
@boegel
Copy link
Member

boegel commented Dec 15, 2025

It was "just missing". So the question worth a comment would rather by why it does not need to be this way.

If we don't do this we only call the configure_step of PythonPackage, not of Cargo which isn't what we want, do we?

Currently only PythonPackage.configure_step would be called, indeed.

>>> class A():
...     def test(self):
...         print('A.test')
...
>>> class B():
...     def test(self):
...         print('B.test')
...
>>> class C(A,B):
...     pass
...
>>> c=C()
>>> c.test()
A.test

But note that Cargo.configure_step is empty currently, so I'm still not sure what the point is of this PR...

@boegel
Copy link
Member

boegel commented Dec 15, 2025

@Flamefire You are changing Cargo.configure_step in:

So then the changes being made here should be included in that PR. In isolation it just doesn't make much sense to me...

@Flamefire
Copy link
Contributor Author

So then the changes being made here should be included in that PR. In isolation it just doesn't make much sense to me...

#3995 did include this PR, although rather intended for easier testing as merging the PRs individually might be easier for drafting release notes.

@boegel boegel merged commit 27bf1ef into easybuilders:develop Jan 20, 2026
22 checks passed
@boegel
Copy link
Member

boegel commented Jan 20, 2026

@Flamefire Flamefire deleted the configure-cargo-python-pkg branch January 20, 2026 19:30
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