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

Fix compilation on CentOS 7 (missing link to libdl) #2849

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

arekfu
Copy link
Contributor

@arekfu arekfu commented Jan 19, 2024

Description

This PR fixes compilation on CentOS 7.

Fixes #2848.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 15) on any C++ source files (if applicable) (not applicable)
  • I have followed the style guidelines for Python source files (if applicable) (not applicable)
  • I have made corresponding changes to the documentation (if applicable) (not applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable) (not sure this is possible?)

@HunterBelanger
Copy link
Contributor

Hi @arekfu, have you tried to see if using the CMAKE_DL_LIBS works instead of adding dl directly (see here) ? In theory this should allow it to play nice with all operating systems.

@arekfu
Copy link
Contributor Author

arekfu commented Jan 19, 2024

Hi @HunterBelanger, thank you for this suggestion! I was actually not aware of CMAKE_DL_LIBS. Let me test this...

Copy link
Contributor

@paulromano paulromano left a comment

Choose a reason for hiding this comment

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

I'm a little confused by this. OpenMC does not make any direct calls to dlopen or other functions from libdl, so it must be a transitive dependency from one of the libraries we explicitly depend on. In that case, it should be the responsibility of whatever library is actually using it to make sure that the dependency on libdl propagates through.

Also side note that CentOS 7 is EOL in six months ⌛

EDIT: looks like I spoke too soon. We do use dlopen for loading custom source libraries, in which case it is appropriate to be linking against libdl.

@arekfu
Copy link
Contributor Author

arekfu commented Jan 23, 2024

Hi @paulromano, I am aware that time is ticking for CentOS 7, but I think that the fix is valid for other platforms, too. I'm more worried about the failing checks. Is that expected?

@paulromano
Copy link
Contributor

@arekfu There is an issue with a new release of scipy that is causing failures in CI. We just merged another PR that restricts the version of scipy for now, and I've just updated your branch to incorporate that. Hopefully tests should be passing now 🤞

Copy link
Contributor

@paulromano paulromano left a comment

Choose a reason for hiding this comment

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

Tests passing now. Thanks @arekfu!

@paulromano paulromano merged commit 09eb33a into openmc-dev:develop Jan 23, 2024
18 checks passed
church89 pushed a commit to openmsr/openmc that referenced this pull request Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compilation failure on CentOS 7
3 participants