netcdf-c icon indicating copy to clipboard operation
netcdf-c copied to clipboard

Fix dll builds under mingw-w64

Open mjwoods opened this issue 2 years ago • 6 comments

Building the netcdf dll and related executables under mingw-w64 (with cmake) fails due to an undefined reference to `NC4_show_metadata'. The code contains a workaround for the MS C compiler, and this can be extended to gcc in mingw-w64.

After that, the build fails again due to undefined references to functions in tst_utils.h, which are declared as dllimport when they should be extern.

After fixing tst_utils.h, the build completes and all except 4 tests pass. The failed tests are: 82 - nc_test4_tst_filter (Failed) 83 - nc_test4_tst_specific_filters (Failed) 222 - nczarr_test_run_filter (Failed) 223 - nczarr_test_run_specific_filters (Failed)

All of the failed tests relate to filters, which are not being built under mingw-w64. I think this is because libdl is not available by default on mingw-w64, although it can be installed as an optional package. These tests should probably be disabled if the filters are not built.

mjwoods avatar Apr 13 '22 09:04 mjwoods

I am surprised at this. We routinely test our code using mingw as a github action. Could I request two things:

  1. tell me the ./configure options you are using
  2. look at our mingw action (https://github.com/Unidata/netcdf-c/blob/main/.github/workflows/run_tests_win_mingw.yml) and tell us if we are missing something critical.

DennisHeimbigner avatar Apr 13 '22 19:04 DennisHeimbigner

Additional point: we will not get rid of the EXTERNL because it is the macro that allows us to add necessary __declspec declarations for visual studio. We can however change the definition of EXTERNL so that it treats mingw correctly.

DennisHeimbigner avatar Apr 13 '22 19:04 DennisHeimbigner

Hi @DennisHeimbigner , I was using cmake -DBUILD_SHARED_LIBS=ON -DENABLE_DLL=ON. Now I have tried using the configure script based on your github action, and it does work for me. I'm not sure what is causing the difference, but I'm happy to use configure from now on.

mjwoods avatar Apr 14 '22 05:04 mjwoods

On the EXTERNL issue, I think there are some complications in tst_utils.h. EXTERNL has been used for functions that are not defined in the libnetcdf.dll (or any DLL), so extern on its own is sufficient. But there is (at least) one function (e.g. nc__testurl) which is defined in the libnetcdf.dll, so EXTERNL is needed there to handle the dllimport/dllexport syntax. In other words, there is no single definition of EXTERNL that would handle all definitions in tst_utils.h.

mjwoods avatar Apr 14 '22 06:04 mjwoods

We also EXTERNL some functions so our unit-tests can access them, In any case, we will keep EXTERNL. If you want to modify it, we can do that. It occurs in two places: netcdf.h and ncexternl.h

DennisHeimbigner avatar Apr 14 '22 18:04 DennisHeimbigner

Relevant definition appears to be here: https://github.com/Unidata/netcdf-c/blob/12a9083fb3bcc92fbf3e63549aad490f089ab474/include/netcdf.h#L544-L556 Perhaps cmake is setting DLL_NETCDF when autotools does not?

There's similar bits in include/ncexternl.h, oc2/oc.h, oc2/ocx.h, include/ncuri.h. There's also a couple of files that just to the __declspec(dll...) without the extern bit after.

Out of curiousity, where does autotools set DLL_NETCDF, which determines how the important EXTERNL macro gets defined? CMake definition appears to be here: https://github.com/Unidata/netcdf-c/blob/5ccb71d7a3ab7a9723e3d1412b8e27af7e2b978b/config.h.cmake.in#L106-L110 Does the CMake build succeed if those lines are removed from config.h.cmake.in? It looks like that would trigger gcc's auto-export-everything behavior.

Also, CMake on MinGW is unsupported, and the Autotools CI job explicitly requests that the linker mark every function __declspec(dllexport) because none of the tests link without it, so that system needs an overhaul anyway.

DWesl avatar Aug 20 '22 17:08 DWesl