-
Notifications
You must be signed in to change notification settings - Fork 265
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
Unable to use absolute file path on mingw64 with NetCDF 4.8.1 #2124
Comments
I suspect this is the same problem as this: #2105 I had an incorrect understanding of how mingw (msys actually) dealt with paths |
It seems to be related, but here the error is independent of the tests. |
Are you in a position to use this PR: https://github.com/Unidata/netcdf-c/pull/2105/files? |
I tried to build NetCDF (
I got the same error with |
You may have to apply the patches by hand rather than using the commit. |
I used these instructions to test the PR #2105 locally. I can confirm that I have these changes: https://github.com/Unidata/netcdf-c/pull/2105/files
|
I just noticed something, can you change "ALEXAN~1" in that path to the actual full path? |
Unfortunately, I get still the same error (using the cross-compiled NetCDF library).
|
I have no idea why this is failing. One more try; can you move the file to a location |
With |
If you use this path, does it work?
|
I think the problem is my understanding of msys vs mingw. |
Can you see if this patch fixes the problem:
to these lines:
|
When I try a native compilation (within the msys/mingw32 environement), I still get the error above:
With these changes (on top of #2105): diff --git a/libdispatch/dpathmgr.c b/libdispatch/dpathmgr.c
index b8495be7..563762d4 100644
--- a/libdispatch/dpathmgr.c
+++ b/libdispatch/dpathmgr.c
@@ -862,6 +862,8 @@ getlocalpathkind(void)
int kind = NCPD_UNKNOWN;
#ifdef __CYGWIN__
kind = NCPD_CYGWIN;
+#elif __MINGW32__
+ kind = NCPD_WIN;
#elif __MSYS__
kind = NCPD_MSYS;
#elif _MSC_VER /* not _WIN32 */ Or should this patch be tried in cross-compilation? I have to say, I am not so familiar with MSYS vs Mingw32. I am using Linux myself. I am just trying to make sure that my code runs all all major platforms :-) I tried to use
|
It seems that the compiler errors are related to the fact that Note that I use
but then I have this error:
|
The libncpoco was added after 4.8.1 was released. Your best bet might be to go back to |
OK, I understand. On 4.8.1 I applied these changes (the file diff --git a/libdispatch/dpathmgr.c b/libdispatch/dpathmgr.c
index eb450534..a788d74a 100644
--- a/libdispatch/dpathmgr.c
+++ b/libdispatch/dpathmgr.c
@@ -859,6 +859,8 @@ getlocalpathkind(void)
int kind = NCPD_UNKNOWN;
#ifdef __CYGWIN__
kind = NCPD_CYGWIN;
+#elif __MINGW32__
+ kind = NCPD_WIN;
#elif __MSYS__
kind = NCPD_MSYS;
#elif _MSC_VER /* not _WIN32 */
diff --git a/test_common.in b/test_common.in
index 5e36b301..2e09767c 100644
--- a/test_common.in
+++ b/test_common.in
@@ -73,6 +73,14 @@ set -e
# Allow global set -x mechanism for debugging.
if test "x$SETX" = x1 ; then set -x ; fi
+# On MINGW, bash and other POSIX utilities use a mounted root directory,
+# but executables compiled for Windows do not recognise the mount point.
+# Here we ensure that Windows paths are used in tests of Windows executables.
+system=`uname`
+if test "x${system##MINGW*}" = x; then
+ alias pwd='pwd -W'
+fi
+
# We assume that TOPSRCDIR and TOPBUILDDIR are defined
# At the top of this shell script
top_srcdir="$TOPSRCDIR" I compiled with:
(Note the Compilation succeeds! (horray!! :-) ). When I go back at my initial test programs, now all paths I tried do work! I tried these:
However, there are still some issues related to the test suite where I see some failures:
The relevant code: for(k=0;k<len;k++) {
do {rnd = random() % 127;} while(rnd < ' ');
assert(rnd > ' ' && rnd < 127);
s[k] = (char)rnd;
} Should it be the following to exclude the space character? diff --git a/unit_test/tst_xcache.c b/unit_test/tst_xcache.c
index 0c61dd83..d527fcd5 100644
--- a/unit_test/tst_xcache.c
+++ b/unit_test/tst_xcache.c
@@ -93,7 +93,7 @@ generatestrings(int n, unsigned seed)
len = rnd % MAXSTRLEN;
/* generate the characters */
for(k=0;k<len;k++) {
- do {rnd = random() % 127;} while(rnd < ' ');
+ do {rnd = random() % 127;} while(rnd <= ' ');
assert(rnd > ' ' && rnd < 127);
s[k] = (char)rnd;
} After this change, I see this issue in
Thanks a lot for your help! |
Good catch on the < ' '; I will add it to my PR. |
For a start, edit tst_diskless6.c and change #define CLEANUP to #undef CLEANUP. |
ncdump does not like this file (using ncdump 4.7.3 on Ubuntu 20.04):
The file is actually quite small. I made it available here: |
It is a small file containing only the definitions of a couple of dimensions. |
Took al look at it and it is short. |
Try this experiment.
|
Thanks a lot!
Unfortunately, I have now these errors (later in the test suite):
Attached is the corresponding They seem to be related to the fact, that I disabled utilities (ncdump and friends). I will enable |
Here are some "failed" test concerning
I am wondering if the test case is a bit too strict, as I see these differences:
|
I can fix the precision problem. But fixing the e+038 vs e+38 problem is trickier. |
IN looking at the build file, I see some of these are disabled if running under windows. |
Short answer MINGW64_NT-10.0-19041 in the shell that I used so far. Long answer: MSYS2 seems to come with 3 different launchers: MINGW32, MINGW64, MSYS2 (i.e. shells in a terminal with some environment variable setup; https://www.davidegrayson.com/windev/msys2/, https://www.msys2.org/wiki/Launchers/). They have all different
Sorry if this is confusing! |
1. Issue Unidata#2043 * FreeBSD build fails because of conflicts in defining the fileno() function. So removed all extern declarations of fileno. 2. Issue Unidata#2124 * There were a couple of problems here. * I was conflating msys with mingw and they need separate handling of paths. So treat mingw like windows. * memio.c was not always writing the full content of the memory to file. Untested fix by properly accounting for zero size writes. * Fix bug when skipping white space in tst_xcache.c 3. Issue Unidata#2105 * On MINGW, bash and other POSIX utilities use a mounted root directory, but executables compiled for Windows do not recognise the mount point. Ensure that Windows paths are used in tests of Windows executables. 4. Issue Unidata#2132 * Apparently the Intel C compiler on OSX defines isnan etc. So disable declaration in dutil.c under that condition. 5. Fix and re-enable test_rcmerge.sh by allowing override of where to look for .rc files 6. CMakeLists.txt suppresses certain ncdump directory tests because of differences in printing floats/doubles. * Extend the list to include those that also fail under mingw. * Suppress the mingw tests in ncdump/Makefile.am
Fixed by #2138 |
Thank you for this PR! I applied it manually on the version 4.8.1, but I get the error:
But adding, For some strange reasons, the test diff --git a/ncdump/Makefile.am b/ncdump/Makefile.am
index 8b527aa6..244cc7a3 100644
--- a/ncdump/Makefile.am
+++ b/ncdump/Makefile.am
@@ -72,8 +72,8 @@ bom tst_dimsizes nctrunc tst_rcmerge
# Tests for classic and 64-bit offset files.
TESTS = tst_inttags.sh run_tests.sh tst_64bit.sh ref_ctest \
-ref_ctest64 tst_output.sh tst_lengths.sh tst_calendars.sh \
-run_utf8_tests.sh tst_nccopy3.sh tst_nccopy3_subset.sh \
+ref_ctest64 tst_lengths.sh tst_calendars.sh \
+run_utf8_tests.sh tst_nccopy3_subset.sh \
tst_charfill.sh tst_iter.sh tst_formatx3.sh tst_bom.sh \
tst_dimsizes.sh run_ncgen_tests.sh tst_ncgen4_classic.sh test_radix.sh test_rcmerge.sh
@@ -87,10 +87,6 @@ if USE_STRICT_NULL_BYTE_HEADER_PADDING
XFAIL_TESTS += tst_null_byte_padding.sh
endif
-if ! ISCYGWIN
-TESTS += test_unicode_directory.sh test_unicode_path.sh
-endif
-
if LARGE_FILE_TESTS
TESTS += tst_iter.sh
endif
@@ -107,9 +103,9 @@ check_PROGRAMS += tst_vlen_demo
# Tests for netCDF-4 behavior.
TESTS += tst_fileinfo.sh tst_hdf5_offset.sh tst_inttags4.sh \
-tst_netcdf4.sh tst_fillbug.sh tst_netcdf4_4.sh tst_nccopy4.sh \
+tst_netcdf4.sh tst_fillbug.sh tst_nccopy4.sh \
tst_nccopy5.sh tst_grp_spec.sh tst_mud.sh tst_h_scalar.sh tst_formatx4.sh \
-run_utf8_nc4_tests.sh run_back_comp_tests.sh run_ncgen_nc4_tests.sh \
+run_utf8_nc4_tests.sh run_ncgen_nc4_tests.sh \
tst_ncgen4.sh test_scope.sh
# Record interscript dependencies so parallel builds work.
@@ -133,6 +129,16 @@ if ENABLE_CDF5
TESTS += test_keywords.sh
endif
+if ! ISMINGW
+if ! ISCYGWIN
+TESTS += tst_output.sh tst_nccopy3.sh
+TESTS += test_unicode_directory.sh test_unicode_path.sh
+if USE_HDF5
+TESTS += run_back_comp_tests.sh tst_netcdf4_4.sh
+endif
+endif
+endif
+
endif BUILD_TESTSETS
# These files all have to be included with the distribution.
@@ -220,4 +226,5 @@ scope_*.nc copy_scope_*.cdl
# Remove directories
clean-local:
- rm -fr rcmergedir
+ rm -fr rcmergedir rchome
+ I still see the test in
|
Odd, drc.c appears to already contain #include <assert.h>. The other problem is that this Makefile.am test is not working right.
|
Try changing to the following in ncdump/Makefile.am.
and see if that fixes the problem, |
fixed |
With this change (and rerunning ./bootstrap, I am not sure if this was necessary), I can confirm that the test I am wondering if it would not be better to have a CI system testing mingw64. This would speed up significantly the testing and for you it would be much clearer to see what change did results in which outcome. I would be glad to help to set this up.
|
Is it possible to set up a github action to test mingw? |
@DennisHeimbigner Somebody just needs to codify the steps it takes to setup the environment and build netcdf for Windows. But yes, GitHub actions has support for Windows, Linux (ubuntu), and macOS. |
Ward- have any idea how to approach this? |
@dopplershift I'll look, it may be a little stale but these directions were written and distributed when cmake support was originally introduced. The primary issue (at the time, based on the feedback we received) was the overhead of setting up Visual Studio + compiling the required libraries by hand was a big lift for users who wanted to just use the libraries, not necessarily develop against them. I'll take a look at refreshing this and see what libraries still need to be compiled by hand vs. which ones can be linked against or installed by a package management system like @DennisHeimbigner Yes, I'll take a look at the Github Actions platform environments. Saying it has support for 'Windows' is a bit nebulous, insofar as sometimes people say Windows when they mean Visual Studio (which is the habit I'm trying to break), and others use it to mean literally any of the popular development environments (MSYS+MinGW, CMD+MinGW, Powershell+MinGW, Cygwin) existing within a Windows OS environment. In either case, we need to automated more Windows platform testing, regardless of the dev. environment, and perhaps that will be the next priority after the current writing project is in a good place. |
Well in this case GitHub actions supporting Windows means you can start up a VM, that happens to be pre-populated with a bunch of dev packages. You can of course install on top of that pretty much whatever you feel like. |
In this case all we need is a windows machine running mingw. I don't think that requires |
@DennisHeimbigner That is correct. I will take a look at the Github Actions documentation and see what I can set up. |
Here is an attempt from my side: Which is directly based on this: https://www.msys2.org/docs/ci/ Msys2 provides a github action which make this quite easy to do. |
Ok, I have a working mys/mingw action workflow. |
I am tearing my hair out over this. I am beginning to think that msys2 path handling |
In julia we are still using NetCDF 4.7.4 but there are some issues (JuliaGeo/NCDatasets.jl#162, JuliaGeo/NCDatasets.jl#145) in this version which are solved in NetCDF 4.8. The julia package NetCDF.jl from @meggart uses the same NetCDF compiled library and also currently at 4.7.4. It would be great if we could update to a newer version (also for Zarr support). Unfortunately, Windows/mingw prevents us from upgrading. Have some of the changes mentioned in this thread already being integrated in the main branch? |
As far as I know, this is fixed by DennisHeimbigner@55a2643 |
NetCDF 4.8.1
environmental information (i.e. Operating System, compiler info, java version, python version, etc.)
Window 10, mingw64 gcc (version 10.1.0)
a description of the issue with the steps needed to reproduce it
The C file below works with NetCDF 4.7.4 but it produces an error with
Error: No such file or directory
with NetCDF 4.8.1.I am debugging an issue for the Julia NetCDF package which today upgraded to 4.8.1. Linux and Mac OS X are fine.
In Julia, all C packages are cross-compiled from a Linux host. I was able to reproduce the issue indepdently of Julia.
This file is the sample simple_xy_wr.c file with an adapted filename to show the error.
The code is compiled in a MSYS2 shell as follows:
The path
/c/Users/Alexander Barth/.julia/artifacts/ae78b073115f5cca9ab13a23994bdd930ecfe887/
contains the NetCDF 4.8.1 library.Prior to all tests, I checked that the file to be written does not exists (any more).
The text was updated successfully, but these errors were encountered: