build: Solve mingw build clash with DATADIR

[libvirt] [PATCH] build: Solve mingw build clash with DATADIR

Posted by Eric Blake 4 years, 8 months ago

Commit fed58d83 was a hack to fix a mingw build failure due to header
inclusion order resulting in a clash over the use of DATADIR, by
repeating a trick made several other times in the past of tweaking
inclusion order until it goes away.  Better is to revert that, and
instead use pragmas to avoid the clash in the first place, regardless
of header ordering, solving it for everyone in the future.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

I tested that both gcc and clang on F29 support this; but it will take
a full CI run to see if everywhere else is okay with it. Thus, it is
not 5.6 material.

 src/util/viratomic.h       | 3 +++
 src/conf/checkpoint_conf.c | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/util/viratomic.h b/src/util/viratomic.h
index 35800dafcd..c6e7668324 100644
--- a/src/util/viratomic.h
+++ b/src/util/viratomic.h
@@ -218,7 +218,10 @@ VIR_STATIC unsigned int virAtomicIntXor(volatile unsigned int *atomic,

 # ifdef VIR_ATOMIC_OPS_WIN32

+#  pragma push_macro("DATADIR") /* If "configmake.h" was included first */
+#  undef DATADIR
 #  include <winsock2.h>
+#  pragma pop_macro("DATADIR")
 #  include <windows.h>
 #  include <intrin.h>
 #  if !defined(_M_AMD64) && !defined (_M_IA64) && !defined(_M_X64)
diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c
index 5ce4cc4853..5f4c275dd8 100644
--- a/src/conf/checkpoint_conf.c
+++ b/src/conf/checkpoint_conf.c
@@ -21,8 +21,6 @@

 #include <config.h>

-#include <unistd.h>
-
 #include "configmake.h"
 #include "internal.h"
 #include "virbitmap.h"
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] build: Solve mingw build clash with DATADIR

Posted by Michal Privoznik 4 years, 8 months ago

On 7/31/19 9:30 PM, Eric Blake wrote:
> Commit fed58d83 was a hack to fix a mingw build failure due to header
> inclusion order resulting in a clash over the use of DATADIR, by
> repeating a trick made several other times in the past of tweaking
> inclusion order until it goes away.  Better is to revert that, and
> instead use pragmas to avoid the clash in the first place, regardless
> of header ordering, solving it for everyone in the future.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> I tested that both gcc and clang on F29 support this; but it will take
> a full CI run to see if everywhere else is okay with it. Thus, it is
> not 5.6 material.
> 
>   src/util/viratomic.h       | 3 +++
>   src/conf/checkpoint_conf.c | 2 --
>   2 files changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] build: Solve mingw build clash with DATADIR

Posted by Eric Blake 4 years, 8 months ago

On 8/7/19 9:59 AM, Michal Privoznik wrote:
> On 7/31/19 9:30 PM, Eric Blake wrote:
>> Commit fed58d83 was a hack to fix a mingw build failure due to header
>> inclusion order resulting in a clash over the use of DATADIR, by
>> repeating a trick made several other times in the past of tweaking
>> inclusion order until it goes away.  Better is to revert that, and
>> instead use pragmas to avoid the clash in the first place, regardless
>> of header ordering, solving it for everyone in the future.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>
>> I tested that both gcc and clang on F29 support this; but it will take
>> a full CI run to see if everywhere else is okay with it. Thus, it is
>> not 5.6 material.

And the full CI run says I failed,
https://travis-ci.org/libvirt/libvirt/jobs/569132417

In file included from ../../src/conf/checkpoint_conf.c:24:

../gnulib/lib/configmake.h:8:17: error: expected identifier or '('
before string constant

    8 | #define DATADIR "/usr/i686-w64-mingw32/sys-root/mingw/share"

      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

make[3]: *** [Makefile:8803: conf/libvirt_conf_la-checkpoint_conf.lo]
Error 1

make[3]: *** Waiting for unfinished jobs....

I'm reverting the patch under the build-breaker rules, while trying to
reproduce the cross-compilation locally rather than relying solely on
CI.  My local testing that proved that #pragma push_macro works with gcc
was obviously not enough; sorry for the churn while I prepare v2.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] build: Solve mingw build clash with DATADIR

Posted by Andrea Bolognani 4 years, 8 months ago

On Wed, 2019-08-07 at 21:15 -0500, Eric Blake wrote:
> I'm reverting the patch under the build-breaker rules, while trying to
> reproduce the cross-compilation locally rather than relying solely on
> CI.  My local testing that proved that #pragma push_macro works with gcc
> was obviously not enough; sorry for the churn while I prepare v2.

If you want to reproduce in the same environment the CI uses, you can
either set up Travis CI for your fork of libvirt if you're on GitHub
(obviously) or reproduce locally with something like

  $ make ci-build@fedora-rawhide CI_CONFIGURE="mingw64-configure"

If you want to try stuff interactively, just use

  $ make ci-shell@fedora-rawhide

instead.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] build: Solve mingw build clash with DATADIR

Posted by Eric Blake 4 years, 8 months ago

On 7/31/19 2:30 PM, Eric Blake wrote:
> Commit fed58d83 was a hack to fix a mingw build failure due to header
> inclusion order resulting in a clash over the use of DATADIR, by
> repeating a trick made several other times in the past of tweaking
> inclusion order until it goes away.  Better is to revert that, and
> instead use pragmas to avoid the clash in the first place, regardless
> of header ordering, solving it for everyone in the future.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> 
> I tested that both gcc and clang on F29 support this; but it will take
> a full CI run to see if everywhere else is okay with it. Thus, it is
> not 5.6 material.

Ping now that we are in 5.7

> 
>  src/util/viratomic.h       | 3 +++
>  src/conf/checkpoint_conf.c | 2 --
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/util/viratomic.h b/src/util/viratomic.h
> index 35800dafcd..c6e7668324 100644
> --- a/src/util/viratomic.h
> +++ b/src/util/viratomic.h
> @@ -218,7 +218,10 @@ VIR_STATIC unsigned int virAtomicIntXor(volatile unsigned int *atomic,
> 
>  # ifdef VIR_ATOMIC_OPS_WIN32
> 
> +#  pragma push_macro("DATADIR") /* If "configmake.h" was included first */
> +#  undef DATADIR
>  #  include <winsock2.h>
> +#  pragma pop_macro("DATADIR")
>  #  include <windows.h>
>  #  include <intrin.h>
>  #  if !defined(_M_AMD64) && !defined (_M_IA64) && !defined(_M_X64)
> diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c
> index 5ce4cc4853..5f4c275dd8 100644
> --- a/src/conf/checkpoint_conf.c
> +++ b/src/conf/checkpoint_conf.c
> @@ -21,8 +21,6 @@
> 
>  #include <config.h>
> 
> -#include <unistd.h>
> -
>  #include "configmake.h"
>  #include "internal.h"
>  #include "virbitmap.h"
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list