Skip to content

extend sanity check step to check whether specific libraries are not linked into installed binaries/libraries#3678

Merged
Micket merged 23 commits intoeasybuilders:developfrom
boegel:sanity_check_libs
May 17, 2021
Merged

extend sanity check step to check whether specific libraries are not linked into installed binaries/libraries#3678
Micket merged 23 commits intoeasybuilders:developfrom
boegel:sanity_check_libs

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented May 14, 2021

This is still work-in-progress, because it now hardcodes openblas as a banned library, which is obviously not the intention...

We should figure out where the list of required/banned libraries will come from.
One obvious way is to provide a dedicated configuration option like --banned-libraries, but we also need a way to pick up additional banned libraries from the toolchain, for example when using FlexiBLAS (since then all the backend libraries should be banned from linking to directly).

This will also be useful to ensure there are no direct links to the OpenSSL libraries provided by the OS (cfr. easybuilders/easybuild-easyconfigs#11895 (comment)), or to libz.so, libcurl.so, etc.

To do:

  • make list of required/banned libraries configurable
  • let toolchain specify additional required/banned libraries somehow
  • tests~~

…) linked into installed binaries/libraries (WIP)
@boegel boegel added this to the next release (4.4.0) milestone May 14, 2021
@boegel boegel force-pushed the sanity_check_libs branch from 6f427ec to 50e3e1a Compare May 15, 2021 10:29
@boegel boegel force-pushed the sanity_check_libs branch from 50e3e1a to b2a3d0b Compare May 15, 2021 10:34
@boegel boegel force-pushed the sanity_check_libs branch from 4e6d51e to ce8f287 Compare May 15, 2021 14:18
@boegel boegel force-pushed the sanity_check_libs branch from 13ecb60 to ff26856 Compare May 15, 2021 18:17
@easybuilders easybuilders deleted a comment from boegelbot May 16, 2021
@boegel
Copy link
Copy Markdown
Member Author

boegel commented May 16, 2021

Tests are failing because the new test easyblock for libtoy isn't being picked up by the tests, but it's not clear to me why...

edit: and then of course you realize the problem... fixed in f657d64

@easybuilders easybuilders deleted a comment from boegelbot May 16, 2021
@boegel boegel marked this pull request as ready for review May 16, 2021 20:33
@boegel boegel changed the title extend sanity check step to check whether specific libraries are (not linked into installed binaries/libraries (WIP) extend sanity check step to check whether specific libraries are (not linked into installed binaries/libraries May 16, 2021
@boegel boegel requested a review from migueldiascosta May 16, 2021 20:33
@boegel boegel changed the title extend sanity check step to check whether specific libraries are (not linked into installed binaries/libraries extend sanity check step to check whether specific libraries are not linked into installed binaries/libraries May 17, 2021
@easybuilders easybuilders deleted a comment from boegelbot May 17, 2021
Copy link
Copy Markdown
Member

@migueldiascosta migueldiascosta left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Copy Markdown
Contributor

@Micket Micket left a comment

Choose a reason for hiding this comment

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

As far as I can tell, this (understandably) requires opt-in, so, it certainly won't help to catch inadvertent linking?

@boegel
Copy link
Copy Markdown
Member Author

boegel commented May 17, 2021

As far as I can tell, this (understandably) requires opt-in, so, it certainly won't help to catch inadvertent linking?

It will help if we opt-in to checking stuff like /usr/lib/libopenssl.*so.* & co, on generoso for example.

I don't think we should be hardcoding things like this, it should be left as opt-in imho...

It does catch linking the libopenblas directly when FlexiBLAS is being used as a toolchain component though, see banned_linked_shared_libs in easybuild/toolchains/linalg/flexiblas.py.

Copy link
Copy Markdown
Contributor

@Micket Micket left a comment

Choose a reason for hiding this comment

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

lgtm

@Micket Micket merged commit d0db210 into easybuilders:develop May 17, 2021
@boegel boegel deleted the sanity_check_libs branch May 17, 2021 16:49
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.

3 participants