add big-endian CI run and fix endian issues in test and hdf5var.c#3285
add big-endian CI run and fix endian issues in test and hdf5var.c#3285edhartnett wants to merge 40 commits intoUnidata:mainfrom
Conversation
…an Ubuntu 22.04 GCC 11
…t supported in NCZarr
|
Have you tried running the tests like this through a local |
|
No, I was happier to get them running on GitHub since it can be made part of the CI, to prevent future regressions. The endiness bugs were quite subtle. |
|
I'll need to take a look and check the timing; the last time I ran similar tests through |
|
This CI run? It's not the slowest in the current netcdf-c CI collection. The MacOS runs take longer. This run finishes in about 30 minutes. It could be sped up by using --disable-utilities, but since it is not the slowest, it seems better to test the utilities on big-endian anyway. |
|
I see now it ran, thanks. Agreed re: testing the utilities. |
|
After reviewing this, I'm going to delay merging it; there are still some test failures if we enable ncZarr and DAP testing. For an easy way to examine these locally, assuming you have access to The |
|
I may be misunderstanding something, but all tests are passing with this PR. Are there additional tests or configurations you’re running that I might not be aware of? This fix is entirely within the netCDF-4 codebase and is unrelated to Zarr or DAP. If you have a moment, please take a look at the specific line in hdf5var.c. It’s actually correcting an old mistake of mine from many years ago—one I’ve been trying to track down and resolve properly. I’m aware there have been CI instabilities, particularly on Windows platforms. Many of those issues are being addressed in other open PRs. However, some of those fixes will need to be merged in order to stabilize CI. In several cases, the failures appear to be dependency-related bugs that surface under parallel execution, especially on slower platforms like Windows. If you’re seeing CI instability, a good first step would be merging #3273. That PR addresses a dependency issue where a test script can run concurrently with the two programs that generate the file it depends on. The fix ensures the build system waits for those programs to complete before running the test script. I’m trying to understand any concerns that are preventing this PR from being merged. Since this issue stems from my earlier mistake, I’d really appreciate the opportunity to correct it. I’m confident this is the right fix, and I’m happy to address any questions or concerns you may have. Thank you for taking the time to review it. |
|
Ed, the PR disables nczarr and dap tests; if you re-enable them, you'll see that big-endian related issues persist. The change in |
|
OK, but this PR does not claim to fix ALL endianness issues, just the HDF5 ones, and that is a worthy goal. So none of the Zarr or DAP endianness issues are closed by this PR. Note that the PR does not disable any existing testing, it adds big-endian testing which was not present before. The Zarr and DAP problems are totally separate issues. Once we have this PR merged and the big endian test for HDF5 working, I'll be happy to swing by again and take a look at the zarr and dap code. It's hard to solve all bugs at once. This PR solves a big bug, and closes many issues. It's certainly a big step forward. Let's get it merged and them I will move on to the other big-endian issues. |
Part of #3283
Fixes #3282
Fixes #3286
Fixes #3284
Fixes #3287
Fixes #1687
Fixes #3062
Fixes #1896
Fixes #2770
Fixes #1802
Fixes #2696
Fixes #2670
This adds a big endian workflow to the CI system. It turns up some bugs, so this is just a draft PR until I get the fixes...
Also fix the run_reclaim_tests problem in the autotools build, so that I can run with --disable-utilities.
These changes were generated with AI assistance but with human in the loop (me!) for every decision. I have personally reviewed every line of change in the PR and it is all of benefit to netcdf-c. These changes represent a serious attempt to improve netCDF and are in accordance with project practices and customs.