Fix needed in zip code in make_bcs: How to test?
mathomp4 opened this issue · 2 comments
Tests of GEOS built using Spack instead of Baselibs found an build issue in make_bcs. Namely, compilation of the zip.c
file failed saying, essentially, "you need to include stdlib.h
".
The solution for that was simple, enable the STDC
ifdef:
and it built.
So, I can propose a fix, but what I need to know is: how would one test this? I am quite unfamiliar with the make_bcs code. From my reading it looks like if you enable a flag, you can get out compressed rasters?
Note that one possible proposed fix I have is to just enable this for non-Baselibs builds in CMake:
# MAT NOTE This should use find_package(ZLIB) but Baselibs currently
# confuses find_package(). This is a hack until Baselibs is
# reorganized.
if (Baselibs_FOUND)
set (INC_ZLIB ${BASEDIR}/include/zlib)
target_include_directories(${this} PRIVATE ${INC_ZLIB})
else ()
find_package(ZLIB)
target_link_libraries(${this} PRIVATE ZLIB::zlib)
endif ()
So in that else
I'd add:
target_compile_definitions(${this} PRIVATE STDC)
Since no one but me (and, well, @climbfuji) are trying out non-Baselibs builds of GEOS, this would affect only us. But my fear is eventually even Baselibs build will hit this issue.
@mathomp4, Thanks for the report. I don't have a clue why make_bcs needs zip.c
, but that doesn't necessarily mean it's useless. (It just says that I'm useless...) In any case, @biljanaorescanin or @weiyuan-jiang could look into the need for zip.c
. Perhaps we don't need it, after all, or there's a workaround using a zip utility that's present on the system (and doesn't need to be built with GEOS-ESM).
how would one test this?
@biljanaorescanin should be able to run make_bcs from a Spack build (with your fix) and verify that nothing in make_bcs broke.
But perhaps we should first establish that we need to build zip.c
in the first place.
For now, I'll push up a PR with the safe fix for the "non-Baselibs" build. That trivially doesn't affect any current make_bcs users as no one but myself are building that way!