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

Proposed new way to specify the file format #1222

Closed
DennisHeimbigner opened this issue Nov 24, 2018 · 6 comments
Closed

Proposed new way to specify the file format #1222

DennisHeimbigner opened this issue Nov 24, 2018 · 6 comments
Assignees

Comments

@DennisHeimbigner
Copy link
Collaborator

At some point, we are going to run out of format flags such as NC_NETCDF4, NC_CDF5, etc.
I propose an way to arbitrarily increase the set of formats without running out of flags.
The basic idea is to allow nc_create (or in some cases nc_open) to be called
with a path in the form of a "file://" url form, e.g.
file:///
and then to use the existing tagging mechanism to specify the format.
That is, the url would be suffixed with with fragment tags, one of which specifies
the file format. e.g.
file:///#XXX
where XXX is a keyword for the format like "cdf5" or "netcdf-4", etc.
This has an additional advantage that of allowing user-defined formats to have
a more specific name.

@edhartnett
Copy link
Contributor

Don't forget we already have a plan to increase the number of flags. In fact it is super easy: #864.

The only thing that stops this from occuring is that there is also a proposal, fairly complex, to detect when the netcdf.h file does not match the library version (as HDF5 does). But this proposal, while sound, need not prevent us from increasing the number of flags at the present time.

I agree, we are about to run out.

I do not object to your plan however.

@DennisHeimbigner
Copy link
Collaborator Author

Extending the flag set to 64bits inherently requires testing the library version for
compatibility. Or alternatively, we add extra api functions such as nc_openx() and nc_createx()

@edhartnett
Copy link
Contributor

Not a fan of adding extra functions if it can be avoided.

@edhartnett
Copy link
Contributor

Checking the library version may be a worthwhile idea, but no one has ever guaranteed that netcdf would work with mismatched versions of netcdf.h and the library. Furthermore, I don't see how this could happen much in the real world. The user would have to manually muck about with the header file and move it somewhere else. Why would they do that? And if they did, then they should expect trouble, because most libraries will not respond well to that.

So actually I don't believe it is required. However, I don't object to checking version as an extra precaution.

Changing the mode parameter to long long works and all existing code continues to work as expected with recompile. So it seems like an easy way forward. The NetCDF Backwards Compatibility certificate is fully honored. ;-)

Adding new functions is bad. Too much work for everyone!

Using the file:/// syntax you propose is also fine. It will not break the API. I might suggest something like file:///something.nc?format=netcdf_classic.

@opoplawski
Copy link
Contributor

Please don't add a check on the header version - I've been fighting for years to get HDF5 to drop the header version check. Instead do proper ABI management via soname, symbol versioning, etc.

@WardF
Copy link
Member

WardF commented Nov 25, 2018

This should be handled automatically Ivan the soname versioning; specifically, enduring that precompiled code will not work linking against the newer binary and will instead require a recompilation; that’s the entire point of the soversioning I believe. Changing the flag type and bumping the soversion as defined by the rules at [citation needed, will provide when I’m not traveling] should prevent the issue of older binaries linking against the newer library at runtime. I agree that adding new secondary functions is a step of last resort and can likely be avoided in this case.

@WardF WardF self-assigned this Nov 27, 2018
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

4 participants