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

Fix setting of use_szlib #1867

Merged
merged 1 commit into from
Oct 15, 2020
Merged

Fix setting of use_szlib #1867

merged 1 commit into from
Oct 15, 2020

Conversation

gsjaardema
Copy link
Contributor

The USE_SZLIB symbol only appeared one time in this file in the entire project. I think that USE_SZIP is the intended symbol.

The `USE_SZLIB` symbol only appeared one time in this file in the entire project.  I think that `USE_SZIP` is the intended symbol.
@@ -73,7 +73,7 @@ else
has_hdf5="yes"
fi

has_szlib="@USE_SZLIB@"
has_szlib="@USE_SZIP@"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Umm are we really sure?

Yes sometimes it's szip and sometimes szlib. That's because HDF5 makes the same error of sometimes calling it one and sometimes the other. And it causes me a lot of problems!

But are you sure this is a correct replacement?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the only instance of USE_SZLIB in the entire NetCDF code base, so there was no way that it could be set unless I am overlooking something... It is also the symbol that is used for the has_szlib setting in libnetcdf.settings

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarification -- it USE_SZIP is the symbol that is used to set the symbol that is used in libnetcdf.settings

@DennisHeimbigner
Copy link
Collaborator

In nc-config.in, the corresponding symbol tracks to HAS_SZLIB from configure.ac.
In configure.ac, this is set by looking for this:

if test "x$ac_cv_func_H5Z_SZIP" = xyes; then
so, it appears that SZLIB == SZIP.

@DennisHeimbigner
Copy link
Collaborator

There is clearly a difference between the nc-config generated by CMake
and the one generated by Automake. I bet that there are other discrepencies.

@WardF
Copy link
Member

WardF commented Oct 15, 2020

Thanks @gsjaardema . Something has gotten dropped somewhere, this is the second change in the last day that I would swear was merged in but clearly wasn't. Doing some sleuthing locally, and in the meantime I'll get this merged when the travis checks finish. Thanks!

@gsjaardema
Copy link
Contributor Author

@WardF Yes, I thought this used to be working and was surprised when I tried it again and it wasn't...

@WardF WardF merged commit 69a1c36 into Unidata:master Oct 15, 2020
@gsjaardema gsjaardema deleted the patch-46 branch October 15, 2020 17:57
@gsjaardema
Copy link
Contributor Author

@DennisHeimbigner re differences -- One major difference is that --has-nczarr and --has-s3-sdk are only in autotools.

I think the rest of the options are the same...

@edwardhartnett
Copy link
Contributor

Thanks Greg as always for all that you do to make netCDF bullet-proof!! ;-)

@DennisHeimbigner
Copy link
Collaborator

I will add to CMake version.

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

Successfully merging this pull request may close these issues.

4 participants