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

make VEP easyblock compatible with --sanity-check-only #2743

Merged
merged 1 commit into from
Jun 9, 2022

Conversation

boegel
Copy link
Member

@boegel boegel commented Jun 9, 2022

Without this fix, perl_ver is defined as None in sanity_check_step when using --sanity-check-only, because the module is not loaded yet when get_software_root('Perl') is being called.

During an actual build the dependencies are loaded already/still via prepare_step at that point...

Fixes this crash when using --sanity-check-only:

$ eb VEP-105-GCC-11.2.0.eb --sanity-check-only
...

ERROR: Traceback (most recent call last):
  File "/home/example/easybuild/easybuild-framework/easybuild/main.py", line 128, in build_and_install_software
    (ec_res['success'], app_log, err) = build_and_install_one(ec, init_env)
  File "/home/example/easybuild/easybuild-framework/easybuild/framework/easyblock.py", line 4058, in build_and_install_one
    result = app.run_all_steps(run_test_cases=run_test_cases)
  File "/home/example/easybuild/easybuild-framework/easybuild/framework/easyblock.py", line 3941, in run_all_steps
    self.run_step(step_name, step_methods)
  File "/home/example/easybuild/easybuild-framework/easybuild/framework/easyblock.py", line 3776, in run_step
    step_method(self)()
  File "/home/example/easybuild/easybuild-easyblocks/easybuild/easyblocks/v/vep.py", line 120, in sanity_check_step
    perl_libpath = os.path.join('lib', 'perl' + perl_majver, 'site_perl', perl_ver)
  File "/usr/lib64/python3.6/posixpath.py", line 94, in join
    genericpath._check_arg_types('join', a, *p)
  File "/usr/lib64/python3.6/genericpath.py", line 149, in _check_arg_types
    (funcname, s.__class__.__name__)) from None
TypeError: join() argument must be str or bytes, not 'NoneType'

@boegel boegel added the bug fix label Jun 9, 2022
@boegel boegel added this to the next release (4.5.6?) milestone Jun 9, 2022
@boegel boegel marked this pull request as ready for review June 9, 2022 09:19
@boegel boegel force-pushed the vep_sanity_check_only branch from 620f600 to 68bc564 Compare June 9, 2022 09:20
@ocaisa
Copy link
Member

ocaisa commented Jun 9, 2022

Test report by @ocaisa

Overview of tested easyconfigs (in order)

Build succeeded for 0 out of 2 (2 easyconfigs in total)
login1 - Linux Rocky Linux 8.5, x86_64, Intel(R) Xeon(R) CPU E5-2690 v3 @ 2.60GHz, Python 3.6.8
See https://gist.github.com/566da4139a5ada0eb16df36d5cc169f6 for a full test report.

@ocaisa
Copy link
Member

ocaisa commented Jun 9, 2022

@boegel sanity is failing even though the files are actually there:

== sanity checking...
  >> file 'vep' found: FAILED
  >> file 'lib/perl5/site_perl/5.34.0/x86_64-linux-thread-multi/Bio/EnsEMBL/XS.pm' found: FAILED
  >> (non-empty) directory 'modules/Bio/EnsEMBL/VEP' found: FAILED

@ocaisa
Copy link
Member

ocaisa commented Jun 9, 2022

Test report by @ocaisa

Overview of tested easyconfigs (in order)

  • SUCCESS VEP-103.1-GCC-10.2.0.eb
  • SUCCESS VEP-105-GCC-11.2.0.eb

Build succeeded for 2 out of 2 (2 easyconfigs in total)
login1 - Linux Rocky Linux 8.5, x86_64, Intel(R) Xeon(R) CPU E5-2690 v3 @ 2.60GHz, Python 3.6.8
See https://gist.github.com/088f098fe114456486e7a4200fa66bf3 for a full test report.

@ocaisa
Copy link
Member

ocaisa commented Jun 9, 2022

Ah, I had the "wrong" install path set. Hmm, this is actually in weakness in --sanity-check-only, it should be using the module to decide what the root path is for the installation.

@ocaisa ocaisa merged commit f8161b7 into easybuilders:develop Jun 9, 2022
@boegel boegel deleted the vep_sanity_check_only branch June 9, 2022 12:24
@boegel
Copy link
Member Author

boegel commented Jun 9, 2022

Ah, I had the "wrong" install path set. Hmm, this is actually in weakness in --sanity-check-only, it should be using the module to decide what the root path is for the installation.

Hmm, good point. Please open a framework issue on that...

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