netcdf-c: depends on curl

[Chrismarsh]

Even though netcdf-c is only supposed to depend on curl when +dap, it doesn't correctly do this until 4.9.3

Unidata/netcdf-c#3016

Unidata/netcdf-c#2907
Unidata/netcdf-c#3018

This looks to impact both autotools and cmake builds.

@skosukhin @WardF

[skosukhin]

I don't know about Cmake but we don't need this for Autotools.

[Chrismarsh]

Given the PR Unidata/netcdf-c#3018 specifically notes fixing the autotools build, I'm unclear why you say the spack autotools build doesn't need this fix.

[skosukhin]

spack-packages/repos/spack_repo/builtin/packages/netcdf_c/package.py

Line 570 in 7bfe695

config_args.append("ac_cv_lib_curl_curl_easy_setopt=no")

[skosukhin]

And no, they didn't fix it.

[Chrismarsh]

I haven't had a chance to try 4.9.3 via #69 yet, but are you saying that 4.9.3's inclusion of Unidata/netcdf-c#2907 and Unidata/netcdf-c#3018 is insufficient to solve the problem?

I've narrowed the depends to only be applicable to the cmake side of things

Waiting for a final review by @skosukhin

[skosukhin]

In many cases, the configure script of the package doesn't care what options the user specified on the command line: it just overrides them and continues, which makes it very hard to catch the edge cases. This also applies to the newly introduced --disable-curl. Even if we disable the "option", it gets enabled when --enable-nczarr, although the latter doesn't need curl.

[Chrismarsh]

Is the zarr w/ S3 support not handled in the autotools like it is in cmake?

If you're pretty convinced the edge cases aren't sufficiently captured by the CMakeLists, would you prefer that this fix removes the 4.9.3 version bound? Or are you ok with how it is now?

[skosukhin]

1. I'm sorry, I don't feel responsible for the CMake builder of the package. As long as the Autotools part is unaffected, I don't have any opinion.
2. We don't deliver the S3 part of the package. We would need a variant for that, I guess.

[Chrismarsh]

I will test the CMake side again with 4.9.3 when #69 is merged. For now, I will keep the version bound with the expectation that it was fixed.

[bernhardkaindl]

I will test the CMake side again with 4.9.3 when #69 is merged. For now, I will keep the version bound with the expectation that it was fixed.
@Chrismarsh, #69 is merged now.

[johnwparent]

Is it feasible to patch this in the NetCDF-C CMake system? I'd rather correct their CMake usage than require an extra dependency that doesn't align with the expectations the variants provide.

[Chrismarsh]

In theory this is fixed in 9.3. I just need to test it still

[Chrismarsh]

@bernhardkaindl @johnwparent I did

 spack install netcdf-c@4.9.3

and there is no curl dependency, so I believe netcdf@4.9.3 resolves this as expected. I will keep the curl dep restricted as is.

$ ~/bin/libtree_x86_64 libnetcdf.so
libnetcdf.so.22
├── libmpi.so.40 [rpath]
│   ├── libopen-pal.so.80 [rpath]
│   │   ├── libpmix.so.2 [rpath]
│   │   │   ├── libz.so.1 [rpath]
│   │   │   ├── libevent_pthreads-2.1.so.7 [rpath]
│   │   │   ├── libevent_core-2.1.so.7 [rpath]
│   │   │   └── libhwloc.so.15 [rpath]
│   │   │       ├── libxml2.so.2 [rpath]
│   │   │       │   ├── liblzma.so.5 [rpath]
│   │   │       │   └── libz.so.1 [rpath]
│   │   │       └── libpciaccess.so.0 [rpath]
│   │   ├── libevent_core-2.1.so.7 [rpath]
│   │   ├── libevent_pthreads-2.1.so.7 [rpath]
│   │   ├── libhwloc.so.15 [rpath]
│   │   ├── libucp.so.0 [rpath]
│   │   │   ├── libuct.so.0 [rpath]
│   │   │   │   ├── libucs.so.0 [rpath]
│   │   │   │   │   └── libucm.so.0 [rpath]
│   │   │   │   └── libucm.so.0 [rpath]
│   │   │   ├── libucm.so.0 [rpath]
│   │   │   └── libucs.so.0 [rpath]
│   │   ├── libuct.so.0 [rpath]
│   │   ├── libucm.so.0 [rpath]
│   │   └── libucs.so.0 [rpath]
│   ├── libpmix.so.2 [rpath]
│   ├── libevent_pthreads-2.1.so.7 [rpath]
│   ├── libevent_core-2.1.so.7 [rpath]
│   ├── libhwloc.so.15 [rpath]
│   ├── libuct.so.0 [rpath]
│   ├── libucm.so.0 [rpath]
│   ├── libucs.so.0 [rpath]
│   └── libucp.so.0 [rpath]
├── libhdf5_hl.so.310 [rpath]
│   ├── libmpi.so.40 [rpath]
│   └── libhdf5.so.310 [rpath]
│       ├── libmpi.so.40 [rpath]
│       └── libz.so.1 [rpath]
├── libhdf5.so.310 [rpath]
├── libz.so.1 [rpath]
├── libblosc.so.1 [rpath]
│   ├── liblz4.so.1 [rpath]
│   ├── libz.so.1 [rpath]
│   └── libzstd.so.1 [rpath]
├── libbz2.so.1.0 [rpath]
├── libsz.so.2 [rpath]
└── libxml2.so.2 [default path]
    ├── libz.so.1 [rpath of 1]
    └── liblzma.so.5 [default path]

[johnwparent]

@becker33 are we good to merge this? Been a couple weeks and users are requesting this land to unbreak some builds.

[Chrismarsh]

It would be great to have this merged. It shows up in some weird situations when a built-against-system curl netcdf is pulled in with python dependencies, e.g., gdal and friends. The python stack then falls apart in totally bizarre ways