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

Tarballs may not work due to cmake code not in repo #128

Closed
edwardhartnett opened this issue Sep 29, 2020 · 15 comments
Closed

Tarballs may not work due to cmake code not in repo #128

edwardhartnett opened this issue Sep 29, 2020 · 15 comments
Assignees
Labels
bug Something isn't working

Comments

@edwardhartnett
Copy link
Contributor

Some of the NCEPLIBS repos use code from a cmake repo. These repos will not build from tarball because of this. The cmake code needed for each repo needs to live in that repo.

@aerorahul
Copy link
Contributor

@edwardhartnett Shouldn't the clone be recursive prior to making a tarball?
All codes released with git submodules will have that issue.

@edwardhartnett
Copy link
Contributor Author

@aerorahul yes. Other than the NCEPLIBS overall package, does any other of the NCEPLIBS use submodules?

@aerorahul
Copy link
Contributor

@edwardhartnett yes.
w3emc
wrf_io
EMC_post
wgrib2

All of these have a dependency on NetCDF and the CMakeModules provide a FindNetCDF.cmake.

@edwardhartnett
Copy link
Contributor Author

FindNetCDF.cmake should be in every project that needs it, so that tarball distribution also works.

Are there other cases?

@aerorahul
Copy link
Contributor

That will imply duplication and maintenance nightmare to go look for all the instances of it.

@edwardhartnett
Copy link
Contributor Author

@aerorahul no one said it would be easy. ;-)

CMake has it's peculiarities, and one of them is that it requires a lot of code to do stuff like find netCDF. That's unfortunate, but not a reason to break tarball distributions. Tarballs are the main way software is distributed, so should be supported by NCEPLIBS.

@aerorahul
Copy link
Contributor

aerorahul commented Sep 29, 2020

Not sure if this will work, but looking at the Github CLI, may be there is a way to include the submodule contents in the release tarball.

https://cli.github.com/manual/gh_release_create

I'll do a test later tonight.

@edwardhartnett
Copy link
Contributor Author

@aerorahul awesome, that would be great.

@aerorahul
Copy link
Contributor

umm. That failed spectacularly. GH CLI is not an option.

@edwardhartnett
Copy link
Contributor Author

Well that takes us back to adding a cmake directory and putting FindNetCDF.cmake in every project that needs it. It's a pain, but it's the exact same pain I feel because I have a FindNetCDF.cmake in netcdf-fortran, PIO, and the CCR library. CMake is special that way.

The best answer would be to provide a canonical FindNetCDF to Kitware, and have them make that part of CMake. This is what HDF5 has. So perhaps we can pursue that, but that would take some time.

@aerorahul
Copy link
Contributor

Agreed on the Kitware contribution. Not sure about the time.
We could just open a PR on https://github.com/Kitware/CMake and see what happens. It would be better coming from Unidata though. 🤷‍♂️

Another (faster?) alternative is to have Unidata provide a pkg-config file with its cmake build of netCDF, that enables one to find netCDF without using FindNetCDF.cmake.
The issue with this FindNetCDF.cmake is that it is not designed for all the bells and whistles of NetCDF like dap, etc.

@edwardhartnett
Copy link
Contributor Author

edwardhartnett commented Sep 29, 2020

You keep referring to Unidata. Perhaps you are not aware that netcdf-c is maintained by four people, two of whom are me: https://github.com/Unidata/netcdf-c/graphs/contributors. ;-)

Ward at Unidata is too busy to do this. We must do it if we want it done. If we come up with a good solution, Ward will be happy with it and will endorse it, if that helps.

We can submit a pkg-config PR to netcdf-c, and should do so. That will help moving forward and should be done.

But none of that will change the need to put FindNetCDF.cmake in every project that uses it, for the next few NCEPLIBS releases. ;-)

Eventually we might be able to stop.

@edwardhartnett
Copy link
Contributor Author

BTW one problem I'm having in another project is that FindNetCDF is not smart enough to figure out all the dependent libraries that might be in use by netcdf-c. It's not trivial to figure out!

@edwardhartnett
Copy link
Contributor Author

OK, I have added issues for this to w3emc and wrf_io, where the solution is straightforward.

We will tackle wgrib2 and emc_post but not quite yet. ;-)

@edwardhartnett
Copy link
Contributor Author

I believe this has been accomplished in all repos where I can accomplish it. I cannot change the standard EMC custom of doing tags instead of releases, and requiring that uses use git to get the code, but at least all the NCEPLIBS have working tarballs and can have proper releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants