search

ecmwf / atlas

A library for numerical weather prediction and climate modelling
https://sites.ecmwf.int/docs/atlas
Apache License 2.0
115 stars 42 forks source link

Fix undefined reference due to CGAL dependency #155

Closed matthewrmshin closed 1 year ago

matthewrmshin commented 1 year ago

When atlas is built and installed as a standalone package, I would get undefined reference (for symbols associated with the CGAL dependency, e.g., symbols from libgmp, etc) when a subsequent package needs to link to libatlas.so. The fix seems to solve the issue, by adding the CGAL::CGAL target (if available) to the list of public libs dependency for libatlas.so.

FussyDuck commented 1 year ago

CLA assistant check
All committers have signed the CLA.

matthewrmshin commented 1 year ago

@wdeconinck @tlmquintino please review.

codecov-commenter commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (ea451cb) 78.60% compared to head (d2fa7b1) 78.60%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #155 +/- ## ======================================== Coverage 78.60% 78.60% ======================================== Files 827 827 Lines 60502 60502 ======================================== Hits 47556 47556 Misses 12946 12946 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

wdeconinck commented 1 year ago

This is strange. We have never needed this, and CGAL is a private dependency that should not be public. Which platform etc. do you encounter this with? Can it be reproduced with another platform as well?

matthewrmshin commented 1 year ago

Hi @wdeconinck I first encounter this while building atlas with atlas-orca as standalone packages on MO's Cray XC40 using a GNU compiler configuration. It would be some linker commands in atlas-orca that would fail with undefined references to some symbols such as __gmpn_copyi in libatlas.so.

matthewrmshin commented 1 year ago

@wdeconinck Thanks for looking at this. I'll try to reproduce the original issue again in my environment. I'll report back when done. Please bear with me in the mean time.

matthewrmshin commented 1 year ago

I am pursuing a different method to build and install atlas as a package. The problem here does not appear to be repeatable in my new method, so I'll close this PR for now. Thanks for looking into this.