Goddard-Fortran-Ecosystem / pFUnit

Parallel Fortran Unit Testing Framework
Other
173 stars 45 forks source link

CMake: Shared libs are built twice if BUILD_SHARED_LIBS=on #308

Open ZedThree opened 3 years ago

ZedThree commented 3 years ago

This is only a real problem if the Ninja generator is used, as it errors due to two targets trying to build e.g. libfunit.so. Makefiles doesn't seem to care.

The cause is this snippet:

set(pfunit_targets pfunit)
add_library (pfunit)
add_library (PFUNIT::pfunit ALIAS pfunit)
if (BUILD_SHARED_LIBS)
  add_library (pfunit_shared SHARED)
  add_library (PFUNIT::pfunit_shared ALIAS pfunit_shared)
  set_target_properties(pfunit_shared PROPERTIES OUTPUT_NAME pfunit)
  list(APPEND pfunit_targets pfunit_shared)
endif ()

If BUILD_SHARED_LIBS is on, then add_library(pfunit) creates a shared library. This conflicts with setting the OUTPUT_NAME of pfunit_shared to pfunit -- both pfunit and pfunit_shared try to create libpfunit.so, which is an error with Ninja.

(This also applies to funit too)

I think there are two solutions:

  1. Make the first library: add_library(pfunit STATIC), always build the static library and optionally the shared library. You then like PFUNIT::pfunit to alias the shared library if available.
  2. Remove the if (BUILD_SHARED_LIBS) block and only build either the static or the shared. You would probably still need to keep the alias for PFUNIT::pfunit_shared to not break existing code

I'm happy to implement either fix, depending on what you think is best to do. Personally, I'd probably pick option 2, as I don't really ever want both static and shared libraries.

tclune commented 3 years ago

Sorry for the long delay. One might really hope there was a standard CMake convention for this issue, as lots of projects surely encounter the same forces. We really ought to be able to provide both versions of the library with the same base name - lots of projects ship with a foo.a and a foo.so.

I don't think the existing pFUnit documentation directly addresses building shared libraries, so for now I will defer to your choice for solving this. I'll then test with a couple of downstream projects that need the shared libraries to be sure things still work before I role out a release.

ZedThree commented 3 years ago

No worries. Yeah, it's a pain for sure. There are libraries that can handle building both static and shared, SUNDIALS for one. I'll have a look through their CMake again and see how they handle it.