ensure shapelib is found on linux and macos

[Chrismarsh]

This ensure that shapelib is found on macos and linux. Without there are a bunch of link time errors:

src/ninja/./libninja.so: undefined reference to DBFOpen'
ninja/./libninja.so: undefined reference to DBFWriteIntegerAttribute'
[snip]

[masonwillman]

@Chrismarsh did you test this on macos and linux? After the merge, my linux build is unable to find shapelib_DIR automatically. I remember having issues with this, which is why the shapelib was written as it was in the CmakeLists.txt file.

[Chrismarsh]

I was only able to build master on linux with these changes; otherwise it wouldn't link (error snippet above). I'm surprised it linked without shapelib.

Does find_package(shapelib CONFIG REQUIRED) not correctly find shapelib for you?

[Chrismarsh]

How were you specifying shapelib_DIR before? Does setting -DCMAKE_PREFIX_PATH to your shplib install dir help?

[masonwillman]

It's possible shapelib was not being properly linked before. My understanding is that shapelib is a dependency we get from GDAL. When using your changes, I get the following Cmake error:

CMake Error at CMakeLists.txt:91 (find_package):
  Could not find a package configuration file provided by "shapelib" with any
  of the following names:

    shapelibConfig.cmake
    shapelib-config.cmake

  Add the installation prefix of "shapelib" to CMAKE_PREFIX_PATH or set
  "shapelib_DIR" to a directory containing one of the above files.  If
  "shapelib" provides a separate development package or SDK, be sure it has
  been installed.

I had to install the shapelib through sudo apt install libshp-dev and then add the following code to properly find the library:

find_package(PkgConfig REQUIRED)
pkg_check_modules(SHAPELIB REQUIRED shapelib)

include_directories(${SHAPELIB_INCLUDE_DIRS})
link_directories(${SHAPELIB_LIBRARY_DIRS})

Even with the dev library, I was unable to find the shapelibConfig.cmake files on my system, which should be using our 22.04 dependency script, so I was unable to set the shapelib_DIR path.

[masonwillman]

I should add that prior to this merge, our team has been able to build properly on Linux without any issues.

[Chrismarsh]

Ahhh ok I see somewhat what is going on:

• GDAL vendors a shapelib CMake build error with external shapelib OSGeo/gdal#5711
• GDAL name mangles this internal lib so GDAL can be safety used alongside an external shapelib (e.g., this Shapelib Building with RENAME_INTERNAL_SHAPELIB_SYMBOLS=OFF causes compile error OSGeo/gdal#8532)
• I guess your GDAL sets RENAME_INTERNAL_SHAPELIB_SYMBOLS=FALSE such that your GDAL gives the symbols and it links.

Whereas, I build a GDAL that sets DGDAL_USE_INTERNAL_LIBS=FALSE to ensure I link against external libs and not GDAL internal libs (I build GDAL via Spack). In doing so, I don't have RENAME_INTERNAL_SHAPELIB_SYMBOLS=FALSE, so I don't get the symbols and WN fails to link. I think depending on this behaviour of GDAL to non-name mangle is probably brittle.

Oddly, the Debian build rules /don't/ seem to set that flag, which is further confusing. Maybe it isn't GDAL. It is unclear where this symbol is trickling in for you.

I was unable to find the shapelibConfig.cmake files on my system

Odd it doesn't ship with that. I build from source using cmake so I get that file in the install (as expected). Does finding via the non CONFIG search work? does pkgconfig find it?

[masonwillman]

I tried find_package(shapelib REQUIRED), but that prompts the same CMake error I described before. Pkgconfig was able to properly find shapelib. These were the code segments I changed to make it build:

Project CMakeLists.txt

# Chris
find_package(shapelib CONFIG REQUIRED)

# Mine
find_package(PkgConfig REQUIRED)
pkg_check_modules(SHAPELIB REQUIRED shapelib)

include_directories(${SHAPELIB_INCLUDE_DIRS})
link_directories(${SHAPELIB_LIBRARY_DIRS})

Ninja CMakeLists.txt

# Chris 
set(LINK_LIBS ${NETCDF_LIBRARIES_C} 
              ${GDAL_LIBRARY} 
              ${CURL_LIBRARIES}
              ${Boost_LIBRARIES}
              shapelib::shp)

# Mine
set(LINK_LIBS ${NETCDF_LIBRARIES_C} 
              ${GDAL_LIBRARY} 
              ${CURL_LIBRARIES}
              ${Boost_LIBRARIES}
              ${SHAPELIB_LIBRARIES})

While I was able to build and install on Linux, I did not run the installation and properly test if the libraries were working. I wanted to reach out and see what was causing the discrepancy first.

[Chrismarsh]

Ok that's good pkgconfig can find it. I think you should have the detection try both pkgconfig and the cmake config so-as not to break the cmake build of shapelib. If you wrap it manually into a target if found find pkgconfig, the set(LINK_LIBS ... shouldn't need to changed.

[masonwillman]

You're proposing a workflow where we check for cmake config, and if that fails, then check for the pkgconfig. We'd have to add an additional step, which would be to check for shapelib through GDAL.

I'm surprised this hasn't come up in the previous versions. For example, in the tagged 3.12.2 release, I don't believe the CMakeLists.txt references the shapelib library anywhere. Do you know why that might be the case? Without the find package, I'm unsure how the CMake has been identifying your shapelib library.

[Chrismarsh]

I've poked around this:

• I'm looking at my 3.12 build and libninja.so has that libshape symbol:

$ nm -D libninja.so | grep -w SHPWriteObject
00000000001f8cb0 T SHPWriteObject

But nothing links to a libshp.so. Gives statically linked vibes, but I don't even have libshp.a on this machine as far as I can tell. I ran a loop over the libninja.so ldd dependencies to see if anything was bringing in that symbol, but nothing is.

• My gdal does not export that symbol
• There is nothing in Spack that depends on shapelib, so unlikley to be a transitive
• I checked the CMakeCache.txt from the 3.12 build, no SHAPELIB or similar exists
• I checked every .so and transitive .so I'm linking against for those symbols:

find . -type f \( -name '*.so' -o -name '*.so.*' \) \
  -exec nm -D -A {} + 2>/dev/null | grep -w SHPWriteObject
./linux-icelake/windninja-3.13.0.1-iwsnr4jzreqhglbwluwvcso4atw522dm/lib/libninja.so:00000000001f8cb0 T SHPWriteObject
./linux-icelake/windninja-master-kesejfzyc3gojwqxfp5me4wgkaevucks/lib/libninja.so:                 U SHPWriteObject
./linux-icelake/windninja-3.12.1-geoehbar6ji2f7xtts5jqhqjrew2xicx/lib/libninja.so:00000000001f12b0 T SHPWriteObject
./linux-icelake/shapelib-1.6.3-efohyjfhqei74ty7fq44uc7nr4nigpz7/lib/libshp.so.1.6.3:0000000000005810 T SHPWriteObject

The master U links against the libshp.so directly. The rest seem to have been statically linked somehow.

• I've looked at the output of the build and there is no obvious culprit

I think the answer is that WindNinja used to vendor a shapelib version that was removed
c826023#diff-df699d870a7f30e7332875f059474ae68dc08e48ea53dbd0a1e2ec293e83651a

which still exists in 3.12 tag
https://github.com/firelab/windninja/blob/3.12.2/src/ninja/dbfopen.cpp

Do you still have those files in your local tree?

[masonwillman]

Good catch! My local files match what is in the master branch, which has commit c826023. Therefore, I don't have the WindNinja shapelib to use for my build.

Not sure what makes the most sense for our build. Perhaps we should adjust our build process to build GDAL and Shapelib separately and find them separately in CMake. Any thoughts on this Chris?

[Chrismarsh]

So, how is master linking for you if you don't have those files providing those symbol implimentations? Can you check your libgdal.so for that symbol just to full confirm?

Any thoughts on this Chris?

• I would personally not depend on GDAL shipping that symbol and rather just use cmake to find gdal and shapelib directly.
• I would write a FindShapeLib.cmake that uses pkg-config as one of the hints and then bundles up the target.
• If GDAL does ship those symbols (which I must admit I assume is low chance) and you also link against shapelib, it'll have a duplicate symbol error/

[masonwillman]

My libgdal.so does have the shaplib symbols. My libninja.so has the symbol undefined. That implies to me that WindNinja is using the symbol packaged with GDAL.

$ nm -D /usr/local/lib/libgdal.so | grep -w SHPWriteObject
000000000119f3c0 T SHPWriteObject

$ nm -D /usr/local/lib/libninja.so | grep -w SHPWriteObject
                 U SHPWriteObject

I agree that it would be better to use cmake to find gdal and shapelib directly. We could also vendor shapelib with WindNinja, like we did in the past. However, I could see potential cons with that method, for example, our developers having to maintain those files if shapelib updates in the future.

[Chrismarsh]

Wild. Well, that explains that mystery. Why ubuntu ships gdal configured like this makes no sense.
I've checked with a few conda installs (including with other people), and those doesn't have that symbol exported.
I've also checked some other non-system installs of gdal built via other distribution systems, and those also don't export that symbol.

It seems like an uncommon configuration to rely on.

Vendoring seems like a pain and also reduces flexibility if libninja.so is linked against by another application that itself uses libshape

I wonder if you can use try_compile to test if the found gdal resolves the symbol, and if not, then go look for ShapeFile?

[masonwillman]

I think using something like try_compile would be the way to go. I plan to work on this Monday and will have you test the new build instructions to ensure it works for your environment. I would rather not rely on it shipping with GDAL. I think we may have been getting lucky ever since we deleted our version of shapelib.

Thanks for all the investigation and help with this! I appreciate having some more insight into our CMake build process.