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

Linking issue when building GROMACS #1345

Open
acmnpv opened this issue Feb 9, 2024 · 5 comments
Open

Linking issue when building GROMACS #1345

acmnpv opened this issue Feb 9, 2024 · 5 comments
Labels
discussion General discussion about something

Comments

@acmnpv
Copy link
Contributor

acmnpv commented Feb 9, 2024

Hello, I recently ran into a strange linking issue when building GROMACS 2023.3 with AdaptiveCpp 23.10.0

I build ACpp using gcc-12 against the HIP backend (using ROCm 6.0.0), and then followed up building GROMACS using the same GCC.
As expected from the build requirements from ACpp, it pulled in the context and fiber libraries from boost. But when I later tried to run the GROMACS binaries I saw that they also linked against the boost filesystem libraries, which as far as I understand shouldn't be the case as neither GROMACS nor ACpp have those as a requirement.

This actually caused linking errors on one system I tried it on, where it couldn't find and link against boost filesystem automatically, and crashed when forcing the link.

Any ideas about what could be the cause would be welcome. I wasn't able to reproduce the crash on a different system, but the link against boost filesystem was still there.

I already checked with @al42and but he also wasn't sure what the issue could be.

Thanks for your advice!

@acmnpv acmnpv added the discussion General discussion about something label Feb 9, 2024
@illuhad
Copy link
Collaborator

illuhad commented Feb 9, 2024

Interesting, I think @tom91136 previously mentioned some boost.filesystem dependency that should not be there. IIRC in the discussion with him the consensus was that boost.filesystem is probably transitively pulled in by some dependency of AdaptiveCpp.

Can you check whether boost context/fiber or other libraries that AdaptiveCpp depends on have a dependency on boost.filesystem?

@acmnpv
Copy link
Contributor Author

acmnpv commented Feb 12, 2024

I just checked the fiber and context libraries, and neither of them have filesystem as a dependency themselves.

I think the problem is that there is an explicit search for boost filesystem here:

src/common/CMakeLists.txt:17:find_package(Filesystem REQUIRED Final Experimental Boost)

Which means that it will (as far as I understand the FindFilesystem CMake script) prepare to link to boost filesystem, even though it is not an actual project dependency. I think it shouldn't search for boost filesystem in that line, because the project anyhow requires a C++17 compliant compiler and std library, and that should come with std::filesystem :)

@illuhad
Copy link
Collaborator

illuhad commented Feb 12, 2024

I had assumed this too, but alas, there are many toolchains which in principle have C++17 support, but don't have the filesystem library yet (or need different linking). So the current cmake integration supports std::filesystem, std::experimental::filesystem and boost::filesystem. boost.filesystem should only be required if std::filesystemis not available.

See e.g. #667

@acmnpv
Copy link
Contributor Author

acmnpv commented Feb 12, 2024

Of course this was GROMACS that uncovered this 😸

I'll have a look then to see how to prevent linking boost::filesystem if std::filesystem is available, or if the issue was that the cluster I was building on had a broken stdlib installation that caused boost::filesystem to be picked up.

On a related note, will Acpp change to require a compliant stdlib in the future?

GROMACS changed a while back to require std::filesystem to be available (which was a contentious change in itself that I had to push through) and put the pressure on cluster admins to no longer use default software stacks that are decade old by this point.

@illuhad
Copy link
Collaborator

illuhad commented Feb 12, 2024

Sounds good :)

On a related note, will Acpp change to require a compliant stdlib in the future?

Well, the support for boost.filesystem and std::experimental::filesystem was only intended as a compatibility workaround. I guess we shouldn't be hauling that around for decades ;) I haven't thought about yet when to drop this compatibility support. My current thinking is that in principle it shouldn't hurt to also support these older approaches - assuming the infrastructure functions correctly and does not cause linking problem ;)

acmnpv added a commit to acmnpv/AdaptiveCpp that referenced this issue Feb 17, 2024
std::filesystem has not been found

Small fix for configuration to only try to detect and use boost
filesystem if a previous search for it in the stdlib hasn't been
successful.

Refs AdaptiveCpp#1345
acmnpv added a commit to acmnpv/AdaptiveCpp that referenced this issue Feb 27, 2024
This helps working around systems with broken std::filesystem where you
just want to specify to use boost instead.

Addresses AdaptiveCpp#1345
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion General discussion about something
Projects
None yet
Development

No branches or pull requests

2 participants