Fix python extension linking (#168)

* Silence annoying FindFFTW warnings

* Fix linker errors at import-time

Python extensions are weakly linked, so linker errors can only be
caught at runtime. This commit fixes any such broken extensions,
and adds automated import testing to ctests to prevent future
regressions.

It also adds a helper function `isce2_add_staticlib` which creates a
position-independent static library suitable for usage in multiple
python extensions. This matches the SCons build scripts more closely
and allows for future extensions to be declared in a more terse and
hygienic fashion.

* Update utilLib scripts

* Update combinedLib scripts

* Update offoutliers lib scripts

* Add asa_im_decode cdll

* Update alos lib scripts

* Show ctest output upon failure

* Update DataAccessor scripts

* Remove TARGET_FILE_BASE_NAME

* Remove incomplete stdproc modules (require stanford code)

Co-authored-by: Ryan Burns <[email protected]>

Author: rtburns-jpl

.circleci/config.yml

@@ -47,7 +47,7 @@ jobs:
             set -ex
             cd /root/project/src/build
             . /opt/conda/bin/activate root
-            ctest
+            ctest --output-on-failure
             ISCE2DIR=$(python3 -c "import os, isce2; print(os.path.dirname(isce2.__file__))" | tail -n 1)
             export PATH=$ISCE2DIR/applications:$PATH
             topsApp.py --help --steps

.cmake/FindFFTW.cmake

@@ -131,8 +131,6 @@ foreach(dtype Float Double LongDouble)
 
         # Tell find_package whether this component was found
         set(FFTW_${component}_FIND_QUIETLY TRUE)
-        find_package_handle_standard_args(FFTW_${component}
-            HANDLE_COMPONENTS REQUIRED_VARS ${libvar} FFTW_INCLUDE_DIRS)
         # Also set the value of the legacy library-variable
         # (Will be set to *-NOTFOUND if not found)
         set(${libvar} ${FFTW_${component}})

.cmake/isce2_helpers.cmake

@@ -10,6 +10,40 @@ macro(isce2_get_dir_prefix)
     string(REPLACE "/" "." dir_prefix ${dir_prefix})
 endmacro()
 
+# Usage: isce2_add_staticlib(name [sources ...])
+# Creates a SCons-like isce2 intermediate library.
+# The actual target will also be available via the namespaced isce2:: alias.
+macro(isce2_add_staticlib name)
+    add_library(${name} STATIC ${ARGN})
+    set_target_properties(${name} PROPERTIES
+        OUTPUT_NAME ${name}
+        POSITION_INDEPENDENT_CODE ON
+        )
+    # add alias matching exported target
+    add_library(isce2::${name} ALIAS ${name})
+endmacro()
+
+# Usage: isce2_add_cdll(libname [sources ...])
+# These libraries are loaded using a hardcoded filename, so this
+# macro simplifies adding target properties to make that possible.
+macro(isce2_add_cdll target)
+    add_library(${target} SHARED ${ARGN})
+    set_target_properties(${target} PROPERTIES
+        PREFIX ""
+        OUTPUT_NAME ${target}
+        SUFFIX .so)
+
+    # If we're the root cmake project (e.g. not add_subdirectory):
+    if("${CMAKE_SOURCE_DIR}" STREQUAL "${PROJECT_SOURCE_DIR}")
+        # override this to also test the resulting extension
+        add_test(NAME load_cdll_${target}
+                 COMMAND ${Python_EXECUTABLE} -c
+                    "from ctypes import cdll; \
+                     cdll.LoadLibrary('$<TARGET_FILE:${target}>')"
+                 )
+    endif()
+endmacro()
+
 # Usage:
 # add_exe_test(main.cpp helpers.F [additional_source.c ...] )
 # or

CMakeLists.txt

@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.12 FATAL_ERROR)
+cmake_minimum_required(VERSION 3.13...3.18)
 
 project(isce2 LANGUAGES C CXX Fortran)
 
@@ -26,6 +26,18 @@ include(TargetMotif)
 include(TargetX11)
 include(UseCython)
 
+# If we're the root cmake project (e.g. not add_subdirectory):
+if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_LIST_DIR}")
+    # override this to also test the resulting extension
+    function(Python_add_library target)
+        _Python_add_library(${target} ${ARGN})
+        add_test(NAME import_${target}
+                 COMMAND ${Python_EXECUTABLE} -c
+                     "import ${target}"
+                 )
+    endfunction()
+endif()
+
 if(NOT DEFINED PYTHON_MODULE_DIR)
     set(PYTHON_MODULE_DIR packages CACHE PATH
         "Python module directory (relative to install prefix)")

README.md

@@ -296,7 +296,7 @@ and then try "scons install" again.
 
 ### CMake (experimental)
 Make sure you have the following prerequisites:
-* CMake ≥ 3.12
+* CMake ≥ 3.13
 * GCC ≥ 4.8  (with C++11 support)
 * Python ≥ 3.5
 * Cython

components/isceobj/LineAccessor/CMakeLists.txt

@@ -1,15 +1,14 @@
-add_library(LineAccessor_static STATIC
+isce2_add_staticlib(LineAccessorLib
     src/ImageAccessor.cpp
     src/LineAccessor.cpp
     src/LineAccessorF.cpp
     )
-target_include_directories(LineAccessor_static PUBLIC include)
-set_property(TARGET LineAccessor_static PROPERTY POSITION_INDEPENDENT_CODE ON)
+target_include_directories(LineAccessorLib PUBLIC include)
 
 Python_add_library(LineAccessor MODULE
     bindings/LineAccessormodule.cpp
     )
-target_link_libraries(LineAccessor PUBLIC LineAccessor_static)
+target_link_libraries(LineAccessor PRIVATE isce2::LineAccessorLib)
 
 InstallSameDir(
     LineAccessor

components/isceobj/Orbit/CMakeLists.txt

@@ -1,4 +1,4 @@
-Python_add_library(orbitHermite MODULE
+isce2_add_cdll(orbitHermite
     src/orbitHermiteC.c
     src/orbithermite.F
     )

components/isceobj/Sensor/CMakeLists.txt

@@ -2,11 +2,7 @@ add_subdirectory(db)
 add_subdirectory(TOPS)
 add_subdirectory(MultiMode)
 
-add_library(asa_im_decode src/asa_im_decode/asa_im_decode.c)
-set_target_properties(asa_im_decode PROPERTIES
-    PREFIX ""
-    OUTPUT_NAME envisat
-    SUFFIX .so)
+isce2_add_cdll(asa_im_decode src/asa_im_decode/asa_im_decode.c)
 
 set(installfiles
     asa_im_decode
@@ -94,6 +90,6 @@ target_include_directories(alos PUBLIC
     include
     src/ALOS_pre_process
     )
-target_link_libraries(alos PUBLIC DataAccessor_static)
+target_link_libraries(alos PUBLIC isce2::DataAccessorLib)
 
 InstallSameDir(${installfiles})

components/isceobj/Util/CMakeLists.txt

@@ -1,91 +1,22 @@
 add_subdirectory(ImageUtil)
 add_subdirectory(geo)
+add_subdirectory(offoutliers)
 
 Python_add_library(offoutliers MODULE
     offoutliers/bindings/offoutliersmodule.cpp
-    offoutliers/src/offoutliers.F
-    offoutliers/src/offoutliersAllocateDeallocate.F
-    offoutliers/src/offoutliersGetState.F
-    offoutliers/src/offoutliersSetState.F
-    offoutliers/src/offoutliersState.F
-    )
-target_include_directories(offoutliers PUBLIC offoutliers/include)
-target_link_libraries(offoutliers PUBLIC
-    stdoel_static
     )
+target_link_libraries(offoutliers PRIVATE isce2::offoutliersLib)
 
 add_definitions(-DHAVE_CONFIG_H -DHAVE_FFTW=1)
-
-add_library(utilLib SHARED
-    src/akima_reg.F
-    src/args_roi.F
-    src/besseldiffs.F
-    src/bilinear.F
-    src/cfft1d_jpl.F
-    src/cfft2d.F
-    src/cffts.F
-    src/config.h
-    src/convert_schdot_to_xyzdot.F
-    src/convert_sch_to_xyz.F
-    src/cross.F
-    src/curvature.F
-    src/derampc.F
-    src/dop.F
-    src/dot.F
-    src/enubasis.F
-    src/fc.F
-    src/fc.f.org
-    src/fftw3stub.c
-    src/fftw3stub.cc
-    src/fortranUtils.f90
-    src/fourn.F
-    src/fournnr.F
-    src/getangs.F
-    src/gettcn_tcvec.F
-    src/hunt.F
-    src/inter_motion.F
-    src/interp.F
-    src/intp_coef.f90
-    src/intpcoefnorm.F
-    src/io.c
-    src/latlon.F
-    src/latlon_nostruct.F
-    #src/lfit.F
-    src/linalg.f90
-    src/lincomb.F
-    src/lookvec.F
-    src/lsq.f90
-    src/matmat.F
-    src/matvec.F
-    src/norm.F
-    src/orrmread1.F
-    src/polint.F
-    src/PowerOfTwo.cc
-    src/quadfit.f90
-    src/radar_to_xyz.F
-    src/rdf_common.inc
-    src/roi_exit.cc
-    src/schbasis.F
-    src/second.c
-    src/sfftw_import.c
-    src/spline.f
-    #src/svd.F
-    #src/svdvecfit9.F
-    #src/svdvecfit.F
-    src/tranmat.F
-    src/uniform_interp.f90
-    src/unitvec.F
-    src/utmtoll.F
-    src/zbrent.F
-    )
+add_subdirectory(src)
 target_include_directories(utilLib PUBLIC
     include
     )
 target_link_libraries(utilLib PRIVATE
     FFTW::Float
     )
 
-add_library(combinedLib SHARED
+isce2_add_staticlib(combinedLib
     Library/geometry/src/geometryModule.F
     Library/linalg3/src/linalg3Module.F
     Library/linalg3/src/linalg3.c
@@ -112,7 +43,7 @@ target_include_directories(combinedlibmodule PUBLIC
     Library/include
     )
 target_link_libraries(combinedlibmodule PUBLIC
-    combinedLib
+    isce2::combinedLib
     )
 
 # Set up fortran module paths
@@ -127,12 +58,6 @@ target_include_directories(combinedLib INTERFACE
     $<$<COMPILE_LANGUAGE:Fortran>:${mdir}>
     )
 
-install(TARGETS
-    utilLib
-    combinedLib
-    LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
-    )
-
 InstallSameDir(
     combinedlibmodule
     offoutliers

components/isceobj/Util/offoutliers/CMakeLists.txt

@@ -0,0 +1,3 @@
+add_subdirectory(src)
+target_include_directories(offoutliersLib PUBLIC include)
+target_link_libraries(offoutliersLib PRIVATE isce2::stdoelLib)

components/isceobj/Util/offoutliers/src/CMakeLists.txt

@@ -0,0 +1,7 @@
+isce2_add_staticlib(offoutliersLib STATIC
+    offoutliers.F
+    offoutliersAllocateDeallocate.F
+    offoutliersGetState.F
+    offoutliersSetState.F
+    offoutliersState.F
+    )

components/isceobj/Util/src/CMakeLists.txt

@@ -0,0 +1,62 @@
+isce2_add_staticlib(utilLib
+    akima_reg.F
+    args_roi.F
+    besseldiffs.F
+    bilinear.F
+    cfft1d_jpl.F
+    cfft2d.F
+    cffts.F
+    config.h
+    convert_schdot_to_xyzdot.F
+    convert_sch_to_xyz.F
+    cross.F
+    curvature.F
+    derampc.F
+    dop.F
+    dot.F
+    enubasis.F
+    fc.F
+    fc.f.org
+    fftw3stub.c
+    fftw3stub.cc
+    fortranUtils.f90
+    fourn.F
+    fournnr.F
+    getangs.F
+    gettcn_tcvec.F
+    hunt.F
+    inter_motion.F
+    interp.F
+    intp_coef.f90
+    intpcoefnorm.F
+    io.c
+    latlon.F
+    latlon_nostruct.F
+    lfit.F
+    linalg.f90
+    lincomb.F
+    lookvec.F
+    lsq.f90
+    matmat.F
+    matvec.F
+    norm.F
+    orrmread1.F
+    polint.F
+    PowerOfTwo.cc
+    quadfit.f90
+    radar_to_xyz.F
+    rdf_common.inc
+    roi_exit.cc
+    schbasis.F
+    second.c
+    sfftw_import.c
+    spline.f
+    svd.F
+    svdvecfit9.F
+    svdvecfit.F
+    tranmat.F
+    uniform_interp.f90
+    unitvec.F
+    utmtoll.F
+    zbrent.F
+    )

components/iscesys/ImageApi/CMakeLists.txt

@@ -1,8 +1,4 @@
-# TODO (subdir-staticlib)
-# This library is in a subdirectory but used by other libraries,
-# so it's compiled as a static library so we don't need to worry
-# about RPATH. Is there a better way to this?
-add_library(DataAccessor_static STATIC
+isce2_add_staticlib(DataAccessorLib
     DataAccessor/src/DataAccessorCaster.cpp
     DataAccessor/src/DataAccessor.cpp
     DataAccessor/src/DataAccessorF.cpp
@@ -18,33 +14,31 @@ add_library(DataAccessor_static STATIC
     InterleavedAccessor/src/Poly1dInterpolator.cpp
     InterleavedAccessor/src/Poly2dInterpolator.cpp
     )
-
-set_property(TARGET DataAccessor_static PROPERTY POSITION_INDEPENDENT_CODE ON)
-target_include_directories(DataAccessor_static PUBLIC
+target_include_directories(DataAccessorLib PUBLIC
     DataAccessor/include
     DataCaster/include
     Factories/include
     InterleavedAccessor/include
     )
-target_link_libraries(DataAccessor_static PUBLIC
-    combinedLib
+target_link_libraries(DataAccessorLib PUBLIC
+    isce2::combinedLib
     )
 
 if(TARGET GDAL::GDAL)
-    target_sources(DataAccessor_static PRIVATE
+    target_sources(DataAccessorLib PRIVATE
         InterleavedAccessor/src/GDALAccessor.cpp
         )
-    target_link_libraries(DataAccessor_static PUBLIC
+    target_link_libraries(DataAccessorLib PUBLIC
         GDAL::GDAL
         )
 else()
-    target_compile_definitions(DataAccessor_static PRIVATE -DHAVE_GDAL=0)
+    target_compile_definitions(DataAccessorLib PRIVATE -DHAVE_GDAL=0)
 endif()
 
 Python_add_library(DataAccessor MODULE
     DataAccessor/bindings/DataAccessormodule.cpp
     )
-target_link_libraries(DataAccessor PRIVATE DataAccessor_static)
+target_link_libraries(DataAccessor PRIVATE isce2::DataAccessorLib)
 
 InstallSameDir(
     Factories/CasterFactory.py