| Message ID | 1471975876-23385-1-git-send-email-sw@weilnetz.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Dear Stefan, Stefan Weil <sw@weilnetz.de> writes: > Commit 50455700092412d90ffaf57ee5d00f38f7d1cc5b added new code which > does not compile for Windows. [...] [include/glib-compat.h] > @@ -48,6 +48,7 @@ static inline gint64 qemu_g_get_monotonic_time(void) > gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout); > #endif > > +#if !defined(_WIN32) > #if !GLIB_CHECK_VERSION(2, 30, 0) > /* Not a 100% compatible implementation, but good enough for most > * cases. Placeholders are only supported at the end of the > @@ -65,8 +66,10 @@ static inline gchar *qemu_g_dir_make_tmp(gchar const *tmpl, GError **error) > g_free(path); > return NULL; > } > + > #define g_dir_make_tmp(tmpl, error) qemu_g_dir_make_tmp(tmpl, error) > #endif /* glib 2.30 */ > +#endif /* !_WIN32 */ This worked fine in my cross-build environment (mingw32-* on Fedora 22) as that has glib 2.44.0. Is there a specific reason you're using a glib version that's at least half a decade old (glib 2.30.0 was released in 2011) on Windows? AFAICT the MSYS2 installer recommended by glib upstream [1] has glib 2.41.1. As for your change: It may fix building qemu itself, but building test-logging should still be broken. Unlike some other tests, it isn't built on POSIX or Linux only. Did "make check" work before my patch in your environment? Sascha [1] http://www.gtk.org/download/windows.php [2] https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-glib2/PKGBUILD
Hi Sascha, On 08/23/16 21:00, Sascha Silbe wrote: > Dear Stefan, > > Stefan Weil <sw@weilnetz.de> writes: > >> Commit 50455700092412d90ffaf57ee5d00f38f7d1cc5b added new code which >> does not compile for Windows. > [...] > > [include/glib-compat.h] >> @@ -48,6 +48,7 @@ static inline gint64 qemu_g_get_monotonic_time(void) >> gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout); >> #endif >> >> +#if !defined(_WIN32) >> #if !GLIB_CHECK_VERSION(2, 30, 0) >> /* Not a 100% compatible implementation, but good enough for most >> * cases. Placeholders are only supported at the end of the >> @@ -65,8 +66,10 @@ static inline gchar *qemu_g_dir_make_tmp(gchar const *tmpl, GError **error) >> g_free(path); >> return NULL; >> } >> + >> #define g_dir_make_tmp(tmpl, error) qemu_g_dir_make_tmp(tmpl, error) >> #endif /* glib 2.30 */ >> +#endif /* !_WIN32 */ > > This worked fine in my cross-build environment (mingw32-* on Fedora 22) > as that has glib 2.44.0. Is there a specific reason you're using a glib > version that's at least half a decade old (glib 2.30.0 was released in > 2011) on Windows? AFAICT the MSYS2 installer recommended by glib > upstream [1] has glib 2.41.1. My Debian build machine has glib 2.28.8 for cross compilation, obviously unchanged since 2011, so it is indeed half a decade old. On my Debian notebook I use a cross glib 2.46.2, so cross compilation will work there without that patch. That greatly reduces the need for the patch. > As for your change: It may fix building qemu itself, but building > test-logging should still be broken. Unlike some other tests, it isn't > built on POSIX or Linux only. Did "make check" work before my patch in > your environment? "make check" for Windows does not work in my cross environment. Regards Stefan
Dear Stefan, Stefan Weil <sw@weilnetz.de> writes: > On 08/23/16 21:00, Sascha Silbe wrote: >> Stefan Weil <sw@weilnetz.de> writes: >> >>> Commit 50455700092412d90ffaf57ee5d00f38f7d1cc5b added new code which >>> does not compile for Windows. >> [...] >> This worked fine in my cross-build environment (mingw32-* on Fedora 22) >> as that has glib 2.44.0. Is there a specific reason you're using a glib >> version that's at least half a decade old (glib 2.30.0 was released in >> 2011) on Windows? AFAICT the MSYS2 installer recommended by glib >> upstream [1] has glib 2.41.1. > > My Debian build machine has glib 2.28.8 for cross compilation, obviously > unchanged since 2011, so it is indeed half a decade old. OK, so it's just your local build environment being outdated? No particular need to use that ancient version? > On my Debian notebook I use a cross glib 2.46.2, so cross compilation > will work there without that patch. > > That greatly reduces the need for the patch. Glad to hear. It would be possibly to support the combination of glib < 2.30.0 AND windows, but only by copying a considerable amount of code from glib. I'd prefer to avoid that if we can help it. >> As for your change: It may fix building qemu itself, but building >> test-logging should still be broken. Unlike some other tests, it isn't >> built on POSIX or Linux only. Did "make check" work before my patch in >> your environment? > > "make check" for Windows does not work in my cross environment. OK, I see. Sascha
On 23 August 2016 at 16:01, Sascha Silbe <silbe@linux.vnet.ibm.com> wrote: > Glad to hear. It would be possibly to support the combination of glib < > 2.30.0 AND windows, but only by copying a considerable amount of code > from glib. I'd prefer to avoid that if we can help it. If we want to raise the minimum glib version requirement for Windows we need to enforce this in configure. (We have had a higher minimum for Windows hosts in the past, so there's precedent for doing it.) thanks -- PMM
On 25 August 2016 at 10:36, Peter Maydell <peter.maydell@linaro.org> wrote: > On 23 August 2016 at 16:01, Sascha Silbe <silbe@linux.vnet.ibm.com> wrote: >> Glad to hear. It would be possibly to support the combination of glib < >> 2.30.0 AND windows, but only by copying a considerable amount of code >> from glib. I'd prefer to avoid that if we can help it. > > If we want to raise the minimum glib version requirement for > Windows we need to enforce this in configure. (We have had > a higher minimum for Windows hosts in the past, so there's > precedent for doing it.) Or we could arrange to skip this test if we're on windows with an old glib I guess. -- PMM
Dear Peter, Peter Maydell <peter.maydell@linaro.org> writes: > On 25 August 2016 at 10:36, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 23 August 2016 at 16:01, Sascha Silbe <silbe@linux.vnet.ibm.com> wrote: >>> Glad to hear. It would be possibly to support the combination of glib < >>> 2.30.0 AND windows, but only by copying a considerable amount of code >>> from glib. I'd prefer to avoid that if we can help it. >> >> If we want to raise the minimum glib version requirement for >> Windows we need to enforce this in configure. (We have had >> a higher minimum for Windows hosts in the past, so there's >> precedent for doing it.) > > Or we could arrange to skip this test if we're on windows > with an old glib I guess. In general I agree with you. In practice test-logging was completely broken on Windows since 2.6.0 (it hard-coded /tmp) and I don't have a suitable environment to test a Windows build, so I'd feel uncomfortable submitting patches addressing this issue myself. However that shouldn't stop anyone else (Stefan perhaps? :) ) from fixing the tests on Windows. I'll gladly review the effects of the corresponding patches on the POSIX side. Sascha
On 08/29/16 17:27, Sascha Silbe wrote: > Dear Peter, > > Peter Maydell <peter.maydell@linaro.org> writes: > >> On 25 August 2016 at 10:36, Peter Maydell <peter.maydell@linaro.org> wrote: >>> On 23 August 2016 at 16:01, Sascha Silbe <silbe@linux.vnet.ibm.com> wrote: >>>> Glad to hear. It would be possibly to support the combination of glib < >>>> 2.30.0 AND windows, but only by copying a considerable amount of code >>>> from glib. I'd prefer to avoid that if we can help it. >>> >>> If we want to raise the minimum glib version requirement for >>> Windows we need to enforce this in configure. (We have had >>> a higher minimum for Windows hosts in the past, so there's >>> precedent for doing it.) >> >> Or we could arrange to skip this test if we're on windows >> with an old glib I guess. > > In general I agree with you. In practice test-logging was completely > broken on Windows since 2.6.0 (it hard-coded /tmp) and I don't have a > suitable environment to test a Windows build, so I'd feel uncomfortable > submitting patches addressing this issue myself. > > However that shouldn't stop anyone else (Stefan perhaps? :) ) from > fixing the tests on Windows. I'll gladly review the effects of the > corresponding patches on the POSIX side. > > Sascha For 2.7, I think we don't need this patch nor any other solution, because the problem is less critical than I thought initially: only builds with old versions of glib are affected. For 2.8, raising the minimum glib version to 2.30.0 would avoid the build problem for normal builds targeting Windows. IMHO it is the simplest and also an acceptable solution, and it also allows removing some conditional code. Fixing the tests for builds targeting Windows is a different issue. Ideally "make test" should work for such builds, no matter whether they run as cross build on Linux (my usual environment) or native on Windows. To simplify things, some tests which are difficult to fix for Windows and which don't test Windows specific code could be omitted. I cannot promise that I'll work on test support for Windows in the near future – if anybody else does that job, I'll be happy, too. Stefan
On 29 August 2016 at 18:51, Stefan Weil <sw@weilnetz.de> wrote: > For 2.7, I think we don't need this patch nor any other solution, because > the problem is less critical than I thought initially: only builds with old > versions of glib are affected. OK -- I have updated the ChangeLog to include this: "For Windows hosts the minimum required glib version is now 2.30. (For all other platforms the minimum glib version remains 2.22.) This is unfortunately not enforced by configure -- if you see a compilation failure regarding the "mkdtemp" function please update your glib version." We can add the "glib minimum is 2.30 for Windows" code to configure for 2.8. (We can't raise the minimum version overall because we want to continue to support the relevant SLES version.) thanks -- PMM
diff --git a/include/glib-compat.h b/include/glib-compat.h index 8d5a7f3..3e025ca 100644 --- a/include/glib-compat.h +++ b/include/glib-compat.h @@ -48,6 +48,7 @@ static inline gint64 qemu_g_get_monotonic_time(void) gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout); #endif +#if !defined(_WIN32) #if !GLIB_CHECK_VERSION(2, 30, 0) /* Not a 100% compatible implementation, but good enough for most * cases. Placeholders are only supported at the end of the @@ -65,8 +66,10 @@ static inline gchar *qemu_g_dir_make_tmp(gchar const *tmpl, GError **error) g_free(path); return NULL; } + #define g_dir_make_tmp(tmpl, error) qemu_g_dir_make_tmp(tmpl, error) #endif /* glib 2.30 */ +#endif /* !_WIN32 */ #if !GLIB_CHECK_VERSION(2, 31, 0) /* before glib-2.31, GMutex and GCond was dynamic-only (there was a separate
Commit 50455700092412d90ffaf57ee5d00f38f7d1cc5b added new code which does not compile for Windows. Signed-off-by: Stefan Weil <sw@weilnetz.de> --- include/glib-compat.h | 3 +++ 1 file changed, 3 insertions(+)