[Qemu-devel] [PATCH for-2.7] wxx: Fix broken build (mkdtemp unavailable)

[Qemu-devel] [PATCH for-2.7] wxx: Fix broken build (mkdtemp unavailable)

[Stefan Weil]

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(+)

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
-- 
2.1.4

Re: [Qemu-devel] [PATCH for-2.7] wxx: Fix broken build (mkdtemp unavailable)

[Sascha Silbe]

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
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641

Re: [Qemu-devel] [PATCH for-2.7] wxx: Fix broken build (mkdtemp unavailable)

[Stefan Weil]

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

Re: [Qemu-devel] [PATCH for-2.7] wxx: Fix broken build (mkdtemp unavailable)

[Sascha Silbe]

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
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641

Re: [Qemu-devel] [PATCH for-2.7] wxx: Fix broken build (mkdtemp unavailable)

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH for-2.7] wxx: Fix broken build (mkdtemp unavailable)

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH for-2.7] wxx: Fix broken build (mkdtemp unavailable)

[Sascha Silbe]

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
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641

Re: [Qemu-devel] [PATCH for-2.7] wxx: Fix broken build (mkdtemp unavailable)

[Stefan Weil]

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

Re: [Qemu-devel] [PATCH for-2.7] wxx: Fix broken build (mkdtemp unavailable)

[Peter Maydell]

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