search

r-windows / rtools-packages

Toolchains and libraries for R-4.0 to R-4.2 (legacy)
https://cran.r-project.org/bin/windows/Rtools/rtools40.html
BSD 3-Clause "New" or "Revised" License
54 stars 36 forks source link

Enable opendap by properly linking static libcurl #259

Closed mjwoods closed 2 years ago

mjwoods commented 2 years ago

For version 4.9.0-1 of the netcdf packages in Rtools, the builds and tests completed successfully. However, subsequent manual testing showed that remote opendap URLs were not recognised by ncdump.

A closer look at the build logs for netcdf 4.9.0-1 showed that libcurl was not being linked, and this was disabling opendap with only a warning (despite --enable-dap being specified for configure). To link libcurl statically, the library dependencies need to be passed to configure via the LIBS variable, and -DCURL_STATICLIB must be added to CPPFLAGS.

Once libcurl is detected properly, configure attempts to find libxml2, before falling back to a bundled ezxml library if libxml2 cannot be used. As libxml2 seems to be preferred, this PR adds libxml2 as a package dependency. Static linking of libxml2 requires adjustments to LIBS and CPPFLAGS similar to those for libcurl.

To give further confidence that opendap is working, we allow testing of remote opendap URLs on the build host. This may occasionally run slowly or timeout if there are problems with Unidata's test server.

I have successfully tested both mingw64 and mingw32 builds with remote URLs, as follows:

ncdump.exe -h https://cida.usgs.gov/thredds/dodsC/stageiv_combined
ncdump.exe -h https://data.nodc.noaa.gov/thredds/dodsC/livnehmodel/1950/Fluxes_Livneh_NAmerExt_15Oct2014.195001.nc
ncdump.exe -h https://cida.usgs.gov/thredds/dodsC/prism_v2

The above tests failed on earlier versions of netcdf for mingw (see https://github.com/mjwoods/RNetCDF/issues/72), so we have made some progress.

jeroen commented 2 years ago

This surprises me a bit because pkg-config should automatically add that macro:

# pkg-config --cflags libcurl
-DCURL_STATICLIB -IC:/rtools40/ucrt64/include

Does your solution also work if instead of hardcoding CPPFLAGS as you have done now, you set it to $(pkg-config --cflags libxml-2.0 libcurl) as you did for LIBS?

mjwoods commented 2 years ago

Good suggestion. I tried again using pkg-config to set CPPFLAGS. makepkg raised a warning about the absolute path C:/rtools40 being found in the package, so I strip it before passing the flags to configure. The resulting package was built and checked successfully, and my ncdump tests with remote URLs worked as well.

I also took the opportunity to include a patch that was added to the netcdf package in mingw-w64 (see https://github.com/msys2/MINGW-packages/pull/11920). The issue has been reported to the upstream netcdf developers as well, so hopefully the patch won't be needed in future releases.

jeroen commented 2 years ago

Also re-deployed v4.9.0 on rwinlib now: https://github.com/rwinlib/netcdf/releases