Skip to content

take into list of crates for easyconfigs using Cargo-based easyblock when determining checksums for patches #21416

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.

Sign up for GitHub

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

Closed
boegel wants to merge 1 commit into from
Closed

take into list of crates for easyconfigs using Cargo-based easyblock when determining checksums for patches #21416

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions test/easyconfigs/easyconfigs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import easybuild.main as eb_main
import easybuild.tools.options as eboptions
from easybuild.base import fancylogger
from easybuild.easyblocks.generic.cargo import Cargo
from easybuild.easyblocks.generic.configuremake import ConfigureMake
from easybuild.easyblocks.generic.pythonpackage import PythonPackage
from easybuild.framework.easyblock import EasyBlock
Expand Down Expand Up @@ -1507,6 +1508,12 @@ def template_easyconfig_test(self, spec):
failing_checks.append("OpenSSL should not be listed as OS dependency")

src_cnt = len(ec['sources'])

# if easyblock derives from Cargo easyblock we need to take into account list of crates,
# which get added to list of sources, to correctly determine number of checksums for sources (incl. crates)
if isinstance(app, Cargo):
src_cnt += len(ec.get('crates', []))
Copy link
Contributor

@Flamefire Flamefire Sep 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we use the actual list of sources as will be done by the checksums step?
I.e. app.cfg['sources'] as discussed in the other PR/issue

Copy link
Member Author

@boegel boegel Sep 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not since the change in easybuilders/easybuild-easyblocks#3448, because ec['sources'] no longer contains also the crates since that change, that's exactly why this fix is required

Copy link
Contributor

@Flamefire Flamefire Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since that change app.cfg['sources'] should be populated with the crates while ec['sources'] should not. Or did I miss anything? See the comment

x = app_class(EasyConfig(file_content))
x.cfg <-- always do this

Copy link
Contributor

@Flamefire Flamefire Sep 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created an alternative PR: #21419

Taking the sources, patches and checksums from app.cfg there and in a single place.


patch_checksums = ec['checksums'][src_cnt:]

# make sure all patch files are available
Expand Down