Fixes for building under MinGW on Windows #9
Conversation
src/compat_wrappers.h
Outdated
| @@ -8,6 +8,10 @@ | |||
|
|
|||
| #include "b-em.h" | |||
|
|
|||
| #ifndef WIN32 | |||
| #define HAVE_STPCPY | |||
| #endif | |||
There was a problem hiding this comment.
I don't think this hunk is correct. You're saying here, that if WIN32 isn't defined, then unconditionally #define HAVE_STPCPY. The problem here is that HAVE_STPCPY will only have been defined via autotools on UNIX, so I don't want this being declared defined if that's not true -- you're effectively clobbering its state here.
There was a problem hiding this comment.
At the moment the MingW build is using a normal makefile not autotools. Perhaps this should just be an inversion of what is currently there, i.e.
#ifdef WIN32
#undef HAVE_STPCPY
#endif
or can that go in the makefile?
Related to that, what's the latest on -Werror and the test for stpcpy? At the moment it is checked with the macro:
AC_CHECK_FUNCS(tdestroy stpcpy)
but if you remember that generates a C test program that generates warnings from the compiler. When -Werror is on, and the same flags are used for the test programs as for the actual compilation of the project, those warnings become errors and the test program fails even on Linux with glibc which has the function.
There was a problem hiding this comment.
I was assuming that stpcpy was present on all non-windows platforms, and that we did't need to test for it in autotools. If that's a bad assumption, then you'll need to add that check, because I have no experience of autotools.
There was a problem hiding this comment.
I don't see how it can be anything other than an inversion -- which makes me question how this is even working at all.
As for AC_CHECK_FUNCS -- no, that's not how it's working -- I'm still splitting that out into AC_CHECK_FUNC individually and setting each test manually to comply with even older autotools which I know are on systems we would claim to support.
As for test programs and -Werror1 -- as I've asked for already, I would want to see precisely what it is about those test programs which is causing -Werror to choke on them, and that relies on someone putting up their config.log file for me to see. We are always in control of the tests which are generated -- and the fact that they're not failing for me (on a fairly old toolchain here) makes me think it's something else. But the config.log will tell me what's going on.
There was a problem hiding this comment.
@hoglet67 Dave, we do already have that check in autotools, though see my comment above about -Werror breaking that check - I think Thomas's point was that the #ifndef as you had it overwrote the result from the autotools check.
If you re-write it I have above that becomes "on Unix-like systems use the value from autotools, on Windows we know it isn't available".
There was a problem hiding this comment.
Ah, I hadn't appreciated this was already in autotools. Is is there by default, or did it just get recently added? I'll invert the logic, as suggested by Steve.
src/vdfs.c
Outdated
| @@ -29,6 +29,7 @@ | |||
| #include "mem.h" | |||
| #include "model.h" | |||
| #include "tube.h" | |||
| #include "compat_wrappers.h" | |||
There was a problem hiding this comment.
Why? It's included via b-em.h
There was a problem hiding this comment.
Ah, I didn't spot that. This line can be removed then. I'll do that tomorrow and re-submit.
|
Thomas, now I'm really confused! This makes sure on Win32 that the compat wrapper is included. It does nothing on Unix, so HAVE_STPCPY as set by autotools. Is this not what you both asked for? I don't want this to depend on other pull requests, if at all possible. |
|
@hoglet67 -- that's fine, providing Windows will cope with that. |
|
@hoglet67 -- I've now merged in @SteveFosdick's changes -- which includes the |
hoglet67
force-pushed
the
db-windows-fixes
branch
from
9f5daaa to
57fb00e
Compare
|
Rebased as requested and added a couple of small fixes so tsearch builds on Windows. Tested on both Windows and Linux. |
|
Looks, good. Thanks, @hoglet67. |
|
Thomas, just a question for my understanding. When you merged these 4 commits, it seems they were re-written with different commit ID. For example, the first one: I'm trying to understand why this was, as this should I think have been a simple fast-forward. It's not a problem, it just means I have to do some additional work to my fork and my local repo to bring them back in step with upstream. I don't think this happened with the previous pull request from me. Any idea why they were re-written? |
Some fixes to get the windows build working again under MinGW.
Note: some work is needed to the VDFS root selection in win.c; the current code compiles bug doesn't allow a directory to be selected.