Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

With netcdf-c-4.9.3-rc1, pio fails #3017

Open
edwardhartnett opened this issue Sep 11, 2024 · 6 comments
Open

With netcdf-c-4.9.3-rc1, pio fails #3017

edwardhartnett opened this issue Sep 11, 2024 · 6 comments

Comments

@edwardhartnett
Copy link
Contributor

edwardhartnett commented Sep 11, 2024

With netcdf-c-4.9.3-rc1, pio fails:

/collab1/data/Dusan.Jovic/sufs/simple-ufs/libs/ufslibs/build/pio-prefix/src/pio/src/clib/pio_nc.c:2433:56: error: use of undeclared identifier '_FillValue'
2433 |                     ierr = nc_put_att(file->fh, varid, _FillValue, xtype, 1, fill_valuep);
     |                                                        ^
/collab1/data/Dusan.Jovic/sufs/simple-ufs/libs/ufslibs/build/pio-prefix/src/pio/src/clib/pio_nc.c:2577:52: error: use of undeclared identifier '_FillValue'
2577 |                 ierr = nc_get_att(file->fh, varid, _FillValue, fill_valuep);
     |                                                    ^
2 errors generated.
make[5]: *** [src/clib/CMakeFiles/pioc.dir/build.make:216: src/clib/CMakeFiles/pioc.dir/pio_nc.c.o] Error 1
make[4]: *** [CMakeFiles/Makefile2:1178: src/clib/CMakeFiles/pioc.dir/all] Error 2
make[3]: *** [Makefile:146: all] Error 2

I tried both pio 2.5.10 which is what we currently use and latest pio 2.6.2. Both fail with 4.9.3-rc1.

@WardF
Copy link
Member

WardF commented Sep 19, 2024

I wonder if this is the result of refactoring_FillValue to NC_FillValue, as discussed in #2858 and changed in #2911.

@WardF
Copy link
Member

WardF commented Sep 19, 2024

In the short term, I wonder if I should add an option enable-unsafe-macros or some such that projects like pio and gdal have an option besides a broken API, while also knowing they'll need to change their code eventually.

@edwardhartnett
Copy link
Contributor Author

No add option --disable-legacy-macros. Most users don't want to change their code for this. And won't.

@WardF
Copy link
Member

WardF commented Sep 19, 2024

I agree with the legacy phrasing, I'll make that change. Due to the conflict with modern standard libraries, it feels like it's going to need to be opt-in. As packages like pio and GDAL (GDAL already made the change, actually), there is no need to introduce an unnecessary potential conflict with other libraries. For now, --enable-legacy-macros will be a stop-gap that will also eventually no longer be necessary.

@edwardhartnett
Copy link
Contributor Author

Strongly disagree with the default. Don't break backwards compatibility for this. It's not even really unsafe or a problem, just a convention that's being imperfectly followed. Much code depends on it working as it does now.

@ZedThree
Copy link
Contributor

It's possible to deprecate the macro itself, here's one way to do it (in action):

#ifdef _MSC_VER
#define _FillValue _Pragma ("message( \"'_FillValue' macro is deprecated, please use NC_FillValue\")")  "_FillValue"
#else
#define _FillValue _Pragma ("GCC warning \"'_FillValue' macro is deprecated, please use NC_FillValue\"")  "_FillValue"
#endif

The MSVC solution is not great, I don't think it tells you where it's coming from, but it does tell you something at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants