netcdf #213

ocefpaf · 2016-03-28T12:18:45Z

Sadly this package was renamed to libnetcdf in the default channel and I guess we should keep that to maintain compatibility.

This is an attempt to update the netcdf package to the latest version and compiled against conda-forge's hdf5.

ocefpaf · 2016-03-28T12:19:48Z

recipes/libnetcdf/bld.bat

+
+:: Remove msvc redist files.
+cd %LIBRARY_BIN%
+rm msvc*.dll


I never tried this recipe on Windows. @rgrout can you take a look please?

it is weird to manually remove the dlls here. Maybe the NetCDF install step copied them in? If so, keep this removal. Otherwise, this is cruft.

These particular lines may not be needed anymore. I can't verify until I get my windows vm repaired.

OK. I am removing them.

msarahan · 2016-03-28T12:30:05Z

recipes/libnetcdf/bld.bat

+set HDF5_DIR=%LIBRARY_PREFIX%\cmake
+
+if "%ARCH%" == "64" (
+  set CMAKE_GENERATOR="Visual Studio 10 2010 Win64"


We need something better here to build for the different MSVC versions. You should be able to use one of the two examples from https://github.com/conda/conda-build/wiki/Windows-recipe-patterns#using-cmake-with-visual-studio

Thanks. I will take a look at that.

kmuehlbauer · 2016-03-28T16:26:46Z

recipes/libnetcdf/build.sh

+# https://www.unidata.ucar.edu/support/help/MailArchives/netcdf/msg11423.html
+export LD_LIBRARY_PATH=$PREFIX/lib
+
+./configure \


Are we bound to use configure here? I had a look at how unidata is testing. They also use a special docker image, but use cmake instead of configure.

Are we bound to use configure here?

Nope. We can try cmake

They also use a special docker image, but use cmake instead of configure.

I will give that a try later. Thanks!

So, I know @groutr mentioned that he tried cmake to build hdf5 and had issues. The issues may also present themselves here so we should be careful. Personally, given hdf5 had issues, I prefer configure, at present.

Really hoped GitHub would handle that link better. Anyways, he was seeing segfaults when using hdf5/h5py with Python when hdf5 was built with cmake. I prefer using configure to segfaults any day. If this situation is any bit similar here, then we would be wise to stick with what works.

@jakirkham Thanks for the heads-up. But it may be of use in sorting out the correct configure switches.

It was weird, and I'm still not sure of the root cause. I'm curious as to why they recommend cmake, yet their official binaries appear to be still using ./configure.

Let's get a stable release out first and then people can play with adding cmake if they so desire. Sound good?

groutr · 2016-03-28T17:55:46Z

recipes/libnetcdf/build.sh

+export LDFLAGS="-L$PREFIX/lib $LDFLAGS"
+
+# https://www.unidata.ucar.edu/support/help/MailArchives/netcdf/msg11423.html
+export LD_LIBRARY_PATH=$PREFIX/lib:$LD_LIBRARY_PATH


And if this is only needed for make check, maybe it should be
LD_LIBRARY_PATH=$PREFIX/lib:$LD_LIBRARY_PATH make check (set LD_LIBRARY_PATH just for make check)
I don't think it should be set during make

Makes sense.

groutr · 2016-03-28T20:50:49Z

recipes/libnetcdf/bld.bat

+set INCLUDE=%LIBRARY_INC%;%INCLUDE%
+set HDF5_DIR=%LIBRARY_PREFIX%\cmake
+
+cmake -G "NMake Makefiles" -DCMAKE_BUILD_TYPE=Release -DCMAKE_PREFIX_PATH=%LIBRARY_PREFIX% -DCMAKE_INSTALL_PREFIX:PATH=%LIBRARY_PREFIX% %SRC_DIR%


cmake needs to be a build dependency for windows, I think.

Not necessarily we do have cmake pre-installed on AppVeyor. Not sure what is the best way to go though.

We should use our cmake, no? Is there some reason not too?

ocefpaf · 2016-04-03T23:00:13Z

Note to self. I removed homebrew from this PR as a test and got:

checking whether the C compiler works... no
configure: error: in `/opt/conda/conda-bld/work':
configure: error: C compiler cannot create executables
See `config.log' for more details

Now I am testing the default channel gcc, but I am not sure that is the way we want to go here.

jakirkham · 2016-04-04T01:24:57Z

recipes/libnetcdf/meta.yaml

+        - m4  # [unix]
+        - msinttypes  # [win]
+        - cmake  # [win]
+        - curl


Where is curl used here?

netcdf depends on curl to support OPeNDAP.

jakirkham · 2016-04-04T01:37:39Z

Might need something like this for Mac. Also, this.

pelson · 2016-04-04T11:15:11Z

What are we reviewing here @ocefpaf. Couldn't you just rename the package in the feedstock, and then rename that feedstock?

ocefpaf · 2016-04-05T01:01:10Z

What are we reviewing here @ocefpaf. Couldn't you just rename the package in the feedstock, and then rename that feedstock?

We don't have a feedstock libnetcdf (nor netcdf). My name comment at the top was just a rant.

dopplershift · 2016-05-20T22:38:10Z

So trying to use non-system libcurl sounds like hell on OSX. I can get further by specifying [not osx] in the meta.yml. I'm currently working around problems with System Integrity Protection on El Cap causing DYLD_LIBRARY_PATH to be ignored by shell scripts (https://developer.apple.com/library/mac/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html#//apple_ref/doc/uid/TP40016462-CH3-SW1); maybe Travis will work though?

dopplershift · 2016-05-20T23:33:50Z

So how about we just use cmake and eliminate autotools? Because this works for build.sh on my OSX system:

#!/bin/bash

cmake -DENABLE_DAP=ON -DCMAKE_INSTALL_PREFIX=$PREFIX .
make
make install

and I can even get it to link against the conda-forge libcurl. I can also say that @WardF (the netCDF maintainer) uses cmake for building. With this script and temporarily removing the hdf4 dependency due to conda-forge/hdf4-feedstock#5, I can get a usable set of netCDF4 binaries.

jakirkham · 2016-05-20T23:54:20Z

recipes/libnetcdf/build.sh

+
+make
+# https://www.unidata.ucar.edu/support/help/MailArchives/netcdf/msg11423.html
+eval ${LIBRARY_SEARCH_VAR}=$PREFIX/lib make check || cat ncdump/test-suite.log


Also, I made a mistake here. Please update this to || { cat ncdump/test-suite.log; exit 1; }.

ocefpaf · 2016-05-21T02:58:04Z

Those commands were in reference to building the FORTRAN library...which is separate from the C-library. There's no way this can work--LIBS="-lnetcdf -lhdf5 -lhdf5_hl" is asking it to link against libnetcdf.so before we've even built it.

Yeah. That was some copy-and-pasta. I should not send PRs while attending a conference 😝

ocefpaf · 2016-05-21T03:05:59Z

So how about we just use cmake and eliminate autotools?

@kmuehlbauer mentioned that at some point. (Sorry, but I cannot recall. We can give it a shot tomorrow.)

With this script and temporarily removing the hdf4 dependency due to conda-forge/hdf4-feedstock#5, I can get a usable set of netCDF4 binaries.

I have been thinking about that about that and I don't have a proper solution yet. We can:

eliminate hdf4 ncdump/ncgen in the hdf4 recipe;
clobber them in the libnetcdf recipe;
avoid using hdf4 in the libnetcdf recipe;
rename hdf4 version of those files.

I will try 2 tomorrow and see how it goes. If anyone have a better idea please let me know.

kmuehlbauer · 2016-05-21T03:24:04Z

@ocefpaf 😁 I had almost forgotten that myself. It was at the beginning of this PR.
Unidata is also using some docker image and cmake to build the libnetcdf in github.com work flow. So this is worth a shot. And we surely can get advice from @WardF, if there will be some problems.

dopplershift · 2016-05-21T03:47:34Z

I don't think conda will let us clobber--my built netcdf4 package didn't have ncdump until I removed the conflict.

ocefpaf · 2016-05-21T10:16:48Z

I would not rely on conda to clobber as conda does the right thing. I would
manually substitute one by the other in the recipe. (Actually the right
thing would be to raise an error ;-)

ocefpaf · 2016-05-22T23:36:37Z

Testing cmake. Everything works locally but I do not know how to build static libs and shared with it. The docs mention that I should BUILD_SHARED_LIBS=OFF to get the static libs, but then we won't get the shared.

Also, there are a few additional options we may use. I need DAP and HDF4, but I have no idea if it is a good idea to enable the rest (JNA, MMap, PNetCDF, and NC-4 Parallel). @dopplershift do you need/recommend testing any of these features?

# Features
--------
NetCDF-2 API:           yes
HDF4 Support:           yes
NetCDF-4 API:           yes
NC-4 Parallel Support:  no
PNetCDF Support:        no
DAP Support:            yes
Diskless Support:       yes
MMap Support:           no
JNA Support:            no

jakirkham · 2016-05-22T23:39:59Z

Testing cmake. Everything works locally but I do not know how to build static libs and shared with it. The docs mention that I should BUILD_SHARED_LIBS=OFF to get the static libs, but then we won't get the shared.

That's fine do 2 loops for building. Once with BUILD_SHARED_LIBS=OFF and once with BUILD_SHARED_LIBS=ON. Make sure to clean each build.

conda-forge-linter · 2016-05-23T00:10:44Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/libnetcdf, recipes/zlib) and found it was in an excellent condition.

conda-forge-linter · 2016-05-23T00:16:44Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/libnetcdf) and found it was in an excellent condition.

ocefpaf · 2016-05-23T00:20:34Z

That's fine do 2 loops for building.

I thought about it, but it sounded "wrong." Anyways... That worked for Linux and OS X 🎉

But Windows, that was working before, is failing now. Not sure what is going on.

(We can skip Windows and open an issue on the feedstock to solve it later.)

[skip appveyor]

jakirkham · 2016-05-23T01:15:39Z

recipes/libnetcdf/build.sh

+make
+# ctest  # Save some time.
+make install
+


I'd add a make clean here. Maybe even a make distclean if they have one.

Done! (No make distclean btw.)

Thanks. Ah, ok.

jakirkham · 2016-05-23T01:17:18Z

That's fine do 2 loops for building.

I thought about it, but it sounded "wrong." Anyways... That worked for Linux and OS X 🎉

We do it for ncurses and it works great. We should probably start doing it for fftw where this is actually recommended (instead we have copy-pasta).

That worked for Linux and OS X 🎉

🎉 🎉 🎉

But Windows, that was working before, is failing now. Not sure what is going on.

(We can skip Windows and open an issue on the feedstock to solve it later.)

Not sure either.

ocefpaf · 2016-05-23T02:00:52Z

@jakirkham if you are OK with this let's merge and iterate in the feedstock. This PR is getting hard to follow.

jakirkham · 2016-05-23T02:37:15Z

LGTM. Thanks @ocefpaf.

jakirkham · 2016-05-23T02:37:59Z

lol I should have renamed before merging. 😆 Oh well.

ocefpaf · 2016-05-23T02:44:39Z

Thanks @jakirkham!

ocefpaf reviewed Mar 28, 2016
View reviewed changes

ocefpaf force-pushed the netcdf branch from 881c465 to ca01520 Compare March 28, 2016 12:27

msarahan reviewed Mar 28, 2016
View reviewed changes

jakirkham changed the title ~~netcdf~~ WIP: netcdf Mar 28, 2016

jakirkham added the Windows label Mar 28, 2016

kmuehlbauer reviewed Mar 28, 2016
View reviewed changes

ocefpaf force-pushed the netcdf branch from ca01520 to 640e8ca Compare March 28, 2016 17:51

groutr reviewed Mar 28, 2016
View reviewed changes

ocefpaf force-pushed the netcdf branch from 640e8ca to e9eeb84 Compare March 28, 2016 17:58

groutr reviewed Mar 28, 2016
View reviewed changes

ocefpaf mentioned this pull request Mar 31, 2016

add netcdf-cxx4 recipe #243

Merged

ocefpaf force-pushed the netcdf branch 2 times, most recently from 910de68 to bad53e7 Compare April 3, 2016 22:48

ocefpaf mentioned this pull request Apr 3, 2016

Cannot install certain packages after h5py without breaking h5py conda-forge/h5py-feedstock#5

Closed

ocefpaf force-pushed the netcdf branch from bad53e7 to b868057 Compare April 3, 2016 22:55

ocefpaf force-pushed the netcdf branch 2 times, most recently from ddb61f4 to 7265e43 Compare April 3, 2016 23:41

jakirkham reviewed Apr 4, 2016
View reviewed changes

kmuehlbauer mentioned this pull request Apr 4, 2016

WIP: Back-port PR #17 conda-forge/gdal-feedstock#27

Merged

jakirkham reviewed May 20, 2016
View reviewed changes

ocefpaf force-pushed the netcdf branch from b892434 to df697ae Compare May 21, 2016 02:53

ocefpaf force-pushed the netcdf branch from c7a1343 to 9ba93c0 Compare May 22, 2016 23:59

ocefpaf force-pushed the netcdf branch from 70e1228 to 26d9160 Compare May 23, 2016 00:16

ocefpaf force-pushed the netcdf branch from 26d9160 to 1efaf7c Compare May 23, 2016 00:23

ocefpaf added 2 commits May 22, 2016 21:25

Add netcdf

1efaf7c

Don't build Windows

71cdacf

[skip appveyor]

jakirkham reviewed May 23, 2016
View reviewed changes

review actions

2aa501d

jakirkham merged commit ec587cf into conda-forge:master May 23, 2016

jakirkham changed the title ~~WIP: netcdf~~ netcdf May 23, 2016

ocefpaf deleted the netcdf branch May 23, 2016 02:45

WardF mentioned this pull request Jun 2, 2016

WIP: Update to 4.3.0 and update pinning conda-forge/netcdf-cxx4-feedstock#9

Merged

netcdf #213

netcdf #213

Conversation

ocefpaf commented Mar 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocefpaf commented Apr 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham commented Apr 4, 2016

pelson commented Apr 4, 2016

ocefpaf commented Apr 5, 2016

dopplershift commented May 20, 2016

dopplershift commented May 20, 2016

Choose a reason for hiding this comment

ocefpaf commented May 21, 2016

ocefpaf commented May 21, 2016

kmuehlbauer commented May 21, 2016

dopplershift commented May 21, 2016 • edited by ocefpaf Loading

ocefpaf commented May 21, 2016 • edited Loading

ocefpaf commented May 22, 2016

jakirkham commented May 22, 2016 • edited Loading

conda-forge-linter commented May 23, 2016

conda-forge-linter commented May 23, 2016

ocefpaf commented May 23, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham commented May 23, 2016

ocefpaf commented May 23, 2016

jakirkham commented May 23, 2016

jakirkham commented May 23, 2016

ocefpaf commented May 23, 2016

dopplershift commented May 21, 2016 •

edited by ocefpaf

Loading

ocefpaf commented May 21, 2016 •

edited

Loading

jakirkham commented May 22, 2016 •

edited

Loading

ocefpaf commented May 23, 2016 •

edited

Loading