Unidata / netcdf-c

Official GitHub repository for netCDF-C libraries and utilities.
BSD 3-Clause "New" or "Revised" License
514 stars 262 forks source link

netCDF 4.8.0 ncgen breaks on CDL type "long"? #1977

Open czender opened 3 years ago

czender commented 3 years ago

netCDF 4.8.0 appears to break NCO builds because ncgen no longer supports the type "long" as a synonym for "int" in CDL files. I did not notice this backwards-incompatibility in the 4.8.0 release notes. Is it intentional?

With latest snapshot:

zender@glace:~/nco/data$ ncgen -v
ncgen: option requires an argument -- 'v'
Usage: ncgen [-1] [-3] [-4] [-5] [-6] [-7] [-b] [-d] [-D debuglevel] [-h] [-k kind ] [-l language=b|c|f77|java] [-M <name>] [-n] [-o outfile] [-P] [-x] [-N datasetname] [-L loglevel] [-H] [-W generate whole var upload] [file ... ]
netcdf library version 4.8.1-development of Mar 30 2021 16:12:40 $
zender@glace:~/nco/data$ ncgen -o in.nc in.cdl
Undefined name (line 642): long
zender@glace:~/nco/data$ 

With 4.7.4 and earlier:

zender@sastrugi:~/nco/data$ ncgen -v
ncgen: option requires an argument -- v
Usage: ncgen [-1] [-3] [-4] [-5] [-6] [-7] [-b] [-B buffersize] [-d] [-D debuglevel] [-h] [-k kind ] [-l language=b|c|f77|java] [-M <name>] [-n] [-o outfile] [-P] [-x] [-N datasetname] [-L loglevel] [-H] [file ... ]
netcdf library version 4.7.4 of Dec 16 2020 23:04:20 $
zender@sastrugi:~/nco/data$ ncgen -o in.nc in.cdl
zender@sastrugi:~/nco/data$ 

Input file in.cdl is here: https://github.com/nco/nco/blob/master/data/in.cdl

DennisHeimbigner commented 3 years ago

Did not mean to do this. Not sure how it happened. But I will check it out and see if there was a reason for it.

DennisHeimbigner commented 3 years ago

This really bizarre. I did the in.cdl test in my local master and it worked ok. I did the same test in the netcdf-c-4.8.0 distribution and it failed as you describe. I diffed the ncgen code for the two trees and they are identical.

I have no idea what is going on.

edwardhartnett commented 3 years ago

Happening in the C library instead of ncgen?

czender commented 3 years ago

Thanks for looking into this. Sorry, I had to modify that file to replace the CDL "long" keyword with "int" in order for NCO to build with the new 4.8.0 netCDF libraries used by Azure. Here is a smaller test file:

netcdf in {
variables:
long long_var;
data:
long_var=10;
}
DennisHeimbigner commented 3 years ago

I built both versions using the same set of options on the same os (ubuntu 16) which means same tool chain. At the moment, the only thing I can think of is something textual where building the distribution introduced, say, some '\r' character.

DennisHeimbigner commented 3 years ago

But since I have repeated the error, hopefully I can debug it.

opoplawski commented 3 years ago

@czender I take it that there is no test in nco for this? The nco Fedora package seems to build fine with netcdf 4.8.0 in my testing.

czender commented 3 years ago

Thanks for looking into this @opoplawski. The Fedora build does attempt to create a netCDF4 version of in.cdl and the build.log shows normal behavior when converting:

...
/usr/bin/ncgen -k netCDF-4 -o in_4.nc in.cdl 
make[2]: Leaving directory '/builddir/build/BUILD/nco-4.9.8/data'
Making all in src
...

So I went back and tested ncgen 4.8.0 with all possible binary output formats, and the results are...surprising?: This script (which anyone should be able to replicate by pasting in a window)

cat > ~/in.cdl << 'EOF'
netcdf in {
  variables:
    long long_var;
  data:
    long_var=10;
}
EOF
ncgen -v
ncgen -o ~/in.nc ~/in.cdl
ncgen -3 -o ~/in.nc ~/in.cdl
ncgen -4 -o ~/in.nc ~/in.cdl
ncgen -5 -o ~/in.nc ~/in.cdl
ncgen -6 -o ~/in.nc ~/in.cdl
ncgen -7 -o ~/in.nc ~/in.cdl

leads to these results:

zender@glace:~$ cat > ~/in.cdl << 'EOF'
> netcdf in {
>   variables:
>     long long_var;
>   data:
>     long_var=10;
> }
> EOF
zender@glace:~$ ncgen -v
ncgen: option requires an argument -- 'v'
Usage: ncgen [-1] [-3] [-4] [-5] [-6] [-7] [-b] [-d] [-D debuglevel] [-h] [-k kind ] [-l language=b|c|f77|java] [-M <name>] [-n] [-o outfile] [-P] [-x] [-N datasetname] [-L loglevel] [-H] [-W generate whole var upload] [file ... ]
netcdf library version 4.8.1-development of Mar 30 2021 16:12:40 $
zender@glace:~$ ncgen -o ~/in.nc ~/in.cdl
Undefined name (line 3): long
zender@glace:~$ ncgen -3 -o ~/in.nc ~/in.cdl
zender@glace:~$ ncgen -4 -o ~/in.nc ~/in.cdl
zender@glace:~$ ncgen -5 -o ~/in.nc ~/in.cdl
zender@glace:~$ ncgen -6 -o ~/in.nc ~/in.cdl
zender@glace:~$ ncgen -7 -o ~/in.nc ~/in.cdl
zender@glace:~$ 

So ncgen only breaks (for me) when it gets no binary format option. Hopefully this clue will help @DennisHeimbigner isolate the problem.

DennisHeimbigner commented 3 years ago

I have a solution, I just have not had time to put up the PR. I will try to get to it next.

DennisHeimbigner commented 3 years ago

See PR https://github.com/Unidata/netcdf-c/pull/1984

czender commented 3 years ago

Thanks @DennisHeimbigner. I thought the fix might be a one-liner, that PR shows how little I know!

DennisHeimbigner commented 3 years ago

Ideally it should have been a 1-liner. But in my endless pursuit for complexity, I avoided that :-)