Skip to content
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.

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

don't raise error when required extensions are not found when installing extensions in parallel #4671

Merged

Conversation

boegel
Copy link
Member

@boegel boegel commented Oct 8, 2024

Raising an error effectively makes it impossible to use --parallel-extensions-install on bundle easyconfigs like R-bundle-CRAN-2024.06-foss-2023b.eb and R-bundle-Bioconductor-3.18-foss-2023a-R-4.3.2.eb.

There's actually no reason to give up when extensions that are required dependencies for an extension that will be installed are not included in exts_list, since they're probably provided via a dependency like R or R-bundle-CRAN.
Note that this is exactly what we do currently when installing extensions sequentially...

If a dependency of a particular extension is totally missing, the installation of that extension will fail as it does when installing sequentially.

Note: for using --parallel-extensions-install on R-bundle-CRAN (or older R easyconfigs that include Rmpi and Rserve), an additional bug fix is required, see:

@boegel
Copy link
Member Author

boegel commented Oct 8, 2024

Hmm, this is not enough, since eventually things will get stuck because no new extensions are being picked up anymore, need to check why that's happening:

Not installing extensions (yet) ━━━━━╸━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 3:19:20

… assumed to be provided by dependencies in install_extensions_parallel
@boegel boegel force-pushed the parallel_exts_no_error_on_missing_deps branch from a879eb9 to 1d31c21 Compare October 15, 2024 16:04
@boegel
Copy link
Member Author

boegel commented Oct 16, 2024

This seems to be working well now with eb R-bundle-Bioconductor-3.18-foss-2023a-R-4.3.2.eb --parallel-extensions-install --experimental, but it does seem to reveal a bug:

ERROR: Shell command failed!
    full command              ->  tar --wildcards --extract --file /data/gent/400/vsc40023/EasyBuild/sources/r/R-bundle-Bioconductor/extensions/RcisTarget_1.22.0.tar --to-stdout '*/DESCRIPTION'
    exit code                 ->  2
    working directory         ->  /tmp/vsc40023/easybuild-framework
    output (stdout + stderr)  ->  /tmp/eb-4106xvti/run-shell-cmd-output/tar-9e1hczim/out.txt
    called from               ->  'required_deps' function in /user/gent/400/vsc40023/easybuild/eb5/easybuild-easyblocks/easybuild/easyblocks/generic/rpackage.py (line 227)

There's no DESCRIPTION file in the RcisTarget source tarball, and we fail hard on that, we probably shouldn't...

The issue is that the DESCRIPTION file is not in a subdirectory in RcisTarget_1.22.0.tar , so tar xfv RcisTarget_1.22.0.tar --to-stdout DESCRIPTION does work just fine.

Always using --to-stdout *DESCRIPTION works too, so I'll strip out the / preceding DESCRIPTION...
edit: done in easybuilders/easybuild-easyblocks#3490

@boegel
Copy link
Member Author

boegel commented Oct 16, 2024

Extensive test report coming up with R-bundle-CRAN-2024.06-foss-2023b.eb + R-bundle-Bioconductor-3.19-foss-2023b-R-4.4.1.eb in easybuilders/easybuild-easyblocks#3490 ...

@boegel boegel marked this pull request as ready for review October 17, 2024 10:32
@boegel
Copy link
Member Author

boegel commented Oct 17, 2024

Extensive test report for bundles of R packages on top of this is available at easybuilders/easybuild-easyblocks#3490 (comment), which shows this is good to go...

easybuild/framework/easyblock.py Outdated Show resolved Hide resolved
@akesandgren akesandgren merged commit 61af40b into easybuilders:5.0.x Oct 21, 2024
39 checks passed
@boegel boegel deleted the parallel_exts_no_error_on_missing_deps branch November 12, 2024 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants