easybuild-framework icon indicating copy to clipboard operation
easybuild-framework copied to clipboard

generic sanity-checking of libraries

Open torbjoernk opened this issue 9 years ago • 19 comments

A new key ('libs') is introduced potentially holding a list of maps specifying libraries to be checked in a generic way. Each library map must have a 'name' and a 'kind' while the latter must either be 'shared' or 'static'. The name is only the library name without path and suffix.

Example: To test for a library libmy.so, which might either be in lib/ or in lib64/:

sanity_check_paths = {
  'libs': [{'name': 'libmy', 'kind': 'shared'}]
}

This will look for the following files:

lib/libmy.so
lib/libmy.dyn
lib64/libmy.so
lib64/libmy.dyn

This is done by generating a tuple of these, which is appended to the sanity_check_paths['files'] list.

Todo:

  • [ ] make the library directory and library suffixes configurable
  • [ ] think of better way to specify the libraries in the easyconfig file

BTW: The first commit is a potential bug fix. The lambdas specified in path_keys_and_check where never used.

torbjoernk avatar Oct 12 '15 20:10 torbjoernk

Automatic reply from Jenkins: Can I test this?

hpcugentbot avatar Oct 12 '15 20:10 hpcugentbot

@torbjoernk: currently, this will be a bit verbose in the easyconfig due to requiring the name and kind keys; two alternatives I have in mind are using a tuple rather than a dictionary for the libs entries, or using shared_libs and static_libs keys.

Also, doesn't it make sense to omit the lib part too, since that will always be there?

Example comparison:

sanity_check_paths = {
    'libs': [{'name': 'foo', 'kind': 'shared'}, {'name': 'foo', 'kind': 'static'},
             {'name': 'bar', 'kind': 'shared'}, {'name': 'bar', 'kind': 'static'}],
}

vs

sanity_check_paths = {
    'libs': [('foo', 'shared'), ('foo', 'static'), ('bar', 'shared'), ('bar', 'static')],
}

vs

sanity_check_paths = {
    'shared_libs': ['foo', 'bar'],
    'static_libs': ['foo', 'bar'],
}

boegel avatar Oct 12 '15 20:10 boegel

Jenkins: ok to test

boegel avatar Oct 12 '15 20:10 boegel

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2181/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

hpcugentbot avatar Oct 12 '15 21:10 hpcugentbot

I personally like the last suggested version with separate fields for shared and static libraries. That seems the least verbose, most expressive and least typo-prone version. I'll try to implement that one.

torbjoernk avatar Oct 13 '15 05:10 torbjoernk

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2186/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

hpcugentbot avatar Oct 13 '15 15:10 hpcugentbot

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2267/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

hpcugentbot avatar Oct 29 '15 17:10 hpcugentbot

@torbjoernk: we also need enhanced unit tests for this... Not sure how easy that would be, shout if you need help.

boegel avatar Oct 29 '15 18:10 boegel

I've implement a simple test case for the new library specifiers. Is that the kind of test you had in mind?

torbjoernk avatar Nov 27 '15 09:11 torbjoernk

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2383/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

hpcugentbot avatar Nov 27 '15 09:11 hpcugentbot

I have no idea why this fails on your Jenkins. :confused:

torbjoernk avatar Nov 27 '15 09:11 torbjoernk

Jenkins: test this please

boegel avatar Dec 17 '15 21:12 boegel

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2464/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

hpcugentbot avatar Dec 17 '15 21:12 hpcugentbot

Jenkins: test this please

boegel avatar Jan 09 '16 23:01 boegel

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2488/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

hpcugentbot avatar Jan 09 '16 23:01 hpcugentbot

@torbjoernk: the problem is that you're changing the toy-0.0.eb test easyconfig, and specify that libtoy.so and libtoy.a must be there

In the unit test you add, you make sure they're there, but the sanity check for that easyconfig fails in other tests where it is used.

You should avoid changing the toy-0.0.eb test easyconfig, and just modify the sanity_check_paths parameter in your test, something like this:

# specify that libtoy.a and libtoy.so should also be there, either in lib or lib64
ec['ec']['sanity_check_paths'].update({
    'shared_libs': ['libtoy'],
    'static_libs': ['libtoy'],
})
eb = EB_toy(ec['ec'])
...

boegel avatar Jan 10 '16 11:01 boegel

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2591/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

hpcugentbot avatar Jan 25 '16 07:01 hpcugentbot

@boegel should we move forward with this?

akesandgren avatar Feb 10 '20 09:02 akesandgren

@akesandgren It may be worth picking this up again, but it should be revived and re-evaluated, it's been too long...

boegel avatar Feb 10 '20 10:02 boegel