[PATCH] tests: Compile virgdbusmock.c with GIO_COMPILATION enabled

Michal Privoznik posted 1 patch 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/4ba4b8a79ec3221088cfea65be5f48ba92e3925f.1679921260.git.mprivozn@redhat.com
tests/meson.build | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
[PATCH] tests: Compile virgdbusmock.c with GIO_COMPILATION enabled
Posted by Michal Privoznik 1 year, 1 month ago
There are couple of g_dbus_*() functions we provide an
alternative implementation for in our virgdbusmock.c. However,
these functions are declared in gio/gdbusconnection.h as:

  GIO_AVAILABLE_IN_ALL
  GDBusConnection  *g_bus_get_sync (GBusType            bus_type,
                                    GCancellable       *cancellable,
                                    GError            **error);

where GIO_AVAILABLE_IN_ALL is declared as (in
/gio/gio-visibility.h):

  #if (defined(_WIN32) || defined(__CYGWIN__)) && !defined(GIO_STATIC_COMPILATION)
  #  define _GIO_EXPORT __declspec(dllexport)
  #  define _GIO_IMPORT __declspec(dllimport)
  #elif __GNUC__ >= 4
  #  define _GIO_EXPORT __attribute__((visibility("default")))
  #  define _GIO_IMPORT
  #else
  #  define _GIO_EXPORT
  #  define _GIO_IMPORT
  #endif
  #ifdef GIO_COMPILATION
  #  define _GIO_API _GIO_EXPORT
  #else
  #  define _GIO_API _GIO_IMPORT
  #endif

  #define _GIO_EXTERN _GIO_API extern

  #define GIO_AVAILABLE_IN_ALL _GIO_EXTERN

Now, on mingw the functions we mock are declared with dllimport
attribute which makes the compiler unhappy:

  ../tests/virgdbusmock.c:25:24: error: 'g_bus_get_sync'
  redeclared without dllimport attribute: previous dllimport
  ignored [-Werror=attributes]

The solution is to do what glib does when it compiles the gio
module: set GIO_COMPILATION macro which in turn annotates the
function with dllexport attribute.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 tests/meson.build | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/meson.build b/tests/meson.build
index 0fd3bc62cf..6be806f4ae 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -65,6 +65,10 @@ else
 endif
 
 
+virgdbusmock_dep = declare_dependency(
+  compile_args: [ '-DGIO_COMPILATION' ]
+)
+
 # mock_libs:
 #   each entry is a dictionary with following items:
 #   * name - mock library name which is also used as default source file name (required)
@@ -78,7 +82,7 @@ mock_libs = [
   { 'name': 'virdnsmasqmock' },
   { 'name': 'virfilecachemock' },
   { 'name': 'virfirewallmock' },
-  { 'name': 'virgdbusmock' },
+  { 'name': 'virgdbusmock', 'deps': [ virgdbusmock_dep] },
   { 'name': 'virhostcpumock' },
   { 'name': 'virhostdevmock' },
   { 'name': 'virnetdaemonmock' },
-- 
2.39.2
Re: [PATCH] tests: Compile virgdbusmock.c with GIO_COMPILATION enabled
Posted by Andrea Bolognani 1 year, 1 month ago
On Mon, Mar 27, 2023 at 02:47:40PM +0200, Michal Privoznik wrote:
> There are couple of g_dbus_*() functions we provide an
> alternative implementation for in our virgdbusmock.c. However,
> these functions are declared in gio/gdbusconnection.h as:
>
>   GIO_AVAILABLE_IN_ALL
>   GDBusConnection  *g_bus_get_sync (GBusType            bus_type,
>                                     GCancellable       *cancellable,
>                                     GError            **error);
>
> where GIO_AVAILABLE_IN_ALL is declared as (in
> /gio/gio-visibility.h):
>
>   #if (defined(_WIN32) || defined(__CYGWIN__)) && !defined(GIO_STATIC_COMPILATION)
>   #  define _GIO_EXPORT __declspec(dllexport)
>   #  define _GIO_IMPORT __declspec(dllimport)
>   #elif __GNUC__ >= 4
>   #  define _GIO_EXPORT __attribute__((visibility("default")))
>   #  define _GIO_IMPORT
>   #else
>   #  define _GIO_EXPORT
>   #  define _GIO_IMPORT
>   #endif
>   #ifdef GIO_COMPILATION
>   #  define _GIO_API _GIO_EXPORT
>   #else
>   #  define _GIO_API _GIO_IMPORT
>   #endif
>
>   #define _GIO_EXTERN _GIO_API extern
>
>   #define GIO_AVAILABLE_IN_ALL _GIO_EXTERN
>
> Now, on mingw the functions we mock are declared with dllimport
> attribute which makes the compiler unhappy:
>
>   ../tests/virgdbusmock.c:25:24: error: 'g_bus_get_sync'
>   redeclared without dllimport attribute: previous dllimport
>   ignored [-Werror=attributes]
>
> The solution is to do what glib does when it compiles the gio
> module: set GIO_COMPILATION macro which in turn annotates the
> function with dllexport attribute.

I will point out that GIO_COMPILATION is not intended to be used
outside of GLib: it signals that the gio module is in the process of
being built, which can result (as is the case here) in different
behavior compared to what you'd see when building *against* gio.

So defining it as part of building libvirt is quite yucky, and I
wouldn't be surprised if this trick stopped working or ended up
causing other unintended consequences in the future.

Unfortunately, I also don't really have a better alternative to
suggest, so I guess it is what it is :)

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] tests: Compile virgdbusmock.c with GIO_COMPILATION enabled
Posted by Michal Prívozník 1 year, 1 month ago
On 3/28/23 10:33, Andrea Bolognani wrote:
> On Mon, Mar 27, 2023 at 02:47:40PM +0200, Michal Privoznik wrote:
>> There are couple of g_dbus_*() functions we provide an
>> alternative implementation for in our virgdbusmock.c. However,
>> these functions are declared in gio/gdbusconnection.h as:
>>
>>   GIO_AVAILABLE_IN_ALL
>>   GDBusConnection  *g_bus_get_sync (GBusType            bus_type,
>>                                     GCancellable       *cancellable,
>>                                     GError            **error);
>>
>> where GIO_AVAILABLE_IN_ALL is declared as (in
>> /gio/gio-visibility.h):
>>
>>   #if (defined(_WIN32) || defined(__CYGWIN__)) && !defined(GIO_STATIC_COMPILATION)
>>   #  define _GIO_EXPORT __declspec(dllexport)
>>   #  define _GIO_IMPORT __declspec(dllimport)
>>   #elif __GNUC__ >= 4
>>   #  define _GIO_EXPORT __attribute__((visibility("default")))
>>   #  define _GIO_IMPORT
>>   #else
>>   #  define _GIO_EXPORT
>>   #  define _GIO_IMPORT
>>   #endif
>>   #ifdef GIO_COMPILATION
>>   #  define _GIO_API _GIO_EXPORT
>>   #else
>>   #  define _GIO_API _GIO_IMPORT
>>   #endif
>>
>>   #define _GIO_EXTERN _GIO_API extern
>>
>>   #define GIO_AVAILABLE_IN_ALL _GIO_EXTERN
>>
>> Now, on mingw the functions we mock are declared with dllimport
>> attribute which makes the compiler unhappy:
>>
>>   ../tests/virgdbusmock.c:25:24: error: 'g_bus_get_sync'
>>   redeclared without dllimport attribute: previous dllimport
>>   ignored [-Werror=attributes]
>>
>> The solution is to do what glib does when it compiles the gio
>> module: set GIO_COMPILATION macro which in turn annotates the
>> function with dllexport attribute.
> 
> I will point out that GIO_COMPILATION is not intended to be used
> outside of GLib: it signals that the gio module is in the process of
> being built, which can result (as is the case here) in different
> behavior compared to what you'd see when building *against* gio.
> 
> So defining it as part of building libvirt is quite yucky, and I
> wouldn't be surprised if this trick stopped working or ended up
> causing other unintended consequences in the future.

That's something we're already used to since switching to glib. This but
another quirk that we have to deal with.

> 
> Unfortunately, I also don't really have a better alternative to
> suggest, so I guess it is what it is :)
> 

Maybe don't build mocks on mingw? Do they even work, or better: can they?

Michal
Re: [PATCH] tests: Compile virgdbusmock.c with GIO_COMPILATION enabled
Posted by Andrea Bolognani 1 year, 1 month ago
On Tue, Mar 28, 2023 at 12:27:12PM +0200, Michal Prívozník wrote:
> On 3/28/23 10:33, Andrea Bolognani wrote:
> > Unfortunately, I also don't really have a better alternative to
> > suggest, so I guess it is what it is :)
>
> Maybe don't build mocks on mingw? Do they even work, or better: can they?

It looks like virgdbusmock is used by

  networkxml2firewalltest
  virfirewalltest
  virsystemdtest
  virpolkittest

The first three are Linux-only; the last one is ELF-only, which
Windows isn't and I *think* even throwing MinGW into the mix doesn't
change that. In any case, enabling polkit support on Windows is
explicitly blocked at the Meson level, so that test will never be
built on that platform.

It looks like it might be feasible to sidestep the issue entirely by
simply not builing this specific mock on Windows after all.

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] tests: Compile virgdbusmock.c with GIO_COMPILATION enabled
Posted by Michal Prívozník 1 year, 1 month ago
On 3/28/23 12:58, Andrea Bolognani wrote:
> On Tue, Mar 28, 2023 at 12:27:12PM +0200, Michal Prívozník wrote:
>> On 3/28/23 10:33, Andrea Bolognani wrote:
>>> Unfortunately, I also don't really have a better alternative to
>>> suggest, so I guess it is what it is :)
>>
>> Maybe don't build mocks on mingw? Do they even work, or better: can they?
> 
> It looks like virgdbusmock is used by
> 
>   networkxml2firewalltest
>   virfirewalltest
>   virsystemdtest
>   virpolkittest
> 
> The first three are Linux-only; the last one is ELF-only, which
> Windows isn't and I *think* even throwing MinGW into the mix doesn't
> change that. In any case, enabling polkit support on Windows is
> explicitly blocked at the Meson level, so that test will never be
> built on that platform.
> 
> It looks like it might be feasible to sidestep the issue entirely by
> simply not builing this specific mock on Windows after all.
> 

NB, this is already pushed, so whatever comes out of this discussion
will need to revert that commit.

Now - Windows PE - are we really sure that it can't work on Windows? I
mean, I only have a vague knowledge on PE, but IIRC then the dynamic
linker does symbol resolution that's very similar to ELF. IOW, the first
.dll to provide the symbol (i.e. our mock) wins the race.

I'm not saying our mock works on Windows as is. Heck, we don't even run
!mock tests on Windows. We run no tests there. But if PE was able to do
mocking, then not compiling this would be pity.

Michal

Re: [PATCH] tests: Compile virgdbusmock.c with GIO_COMPILATION enabled
Posted by Andrea Bolognani 1 year, 1 month ago
On Tue, Mar 28, 2023 at 02:29:10PM +0200, Michal Prívozník wrote:
> On 3/28/23 12:58, Andrea Bolognani wrote:
> > It looks like virgdbusmock is used by
> >
> >   networkxml2firewalltest
> >   virfirewalltest
> >   virsystemdtest
> >   virpolkittest
> >
> > The first three are Linux-only; the last one is ELF-only, which
> > Windows isn't and I *think* even throwing MinGW into the mix doesn't
> > change that. In any case, enabling polkit support on Windows is
> > explicitly blocked at the Meson level, so that test will never be
> > built on that platform.
> >
> > It looks like it might be feasible to sidestep the issue entirely by
> > simply not builing this specific mock on Windows after all.
>
> NB, this is already pushed, so whatever comes out of this discussion
> will need to revert that commit.

I'm aware of that. I don't even mind if the currently merged solution
is what ships with 9.2.0 and a better one, assuming we can come up
with it, is implemented afterwards.

> Now - Windows PE - are we really sure that it can't work on Windows? I
> mean, I only have a vague knowledge on PE, but IIRC then the dynamic
> linker does symbol resolution that's very similar to ELF. IOW, the first
> .dll to provide the symbol (i.e. our mock) wins the race.
>
> I'm not saying our mock works on Windows as is. Heck, we don't even run
> !mock tests on Windows. We run no tests there. But if PE was able to do
> mocking, then not compiling this would be pity.

I'm not sure what the "PE" you talk about is.

Anyway my point is that, at least as far as I can tell, all of the
test programs that use virgdbusmock are effectively no-op on Windows,
which means that mocking gdbus on Windows should not be necessary and
thus not building the mock library on that platform should be viable.

Other mocks might work on Windows, I'm not sure. I haven't
investigated, and I'm not making blanket statements about the topic.
I'm only discussing this specific mock, which is the one currently
causing us grief and which looks like we could simply stop building
on Windows without losing anything in the process.

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] tests: Compile virgdbusmock.c with GIO_COMPILATION enabled
Posted by Michal Prívozník 1 year, 1 month ago
On 3/28/23 14:54, Andrea Bolognani wrote:
> On Tue, Mar 28, 2023 at 02:29:10PM +0200, Michal Prívozník wrote:
>> On 3/28/23 12:58, Andrea Bolognani wrote:
>>> It looks like virgdbusmock is used by
>>>
>>>   networkxml2firewalltest
>>>   virfirewalltest
>>>   virsystemdtest
>>>   virpolkittest
>>>
>>> The first three are Linux-only; the last one is ELF-only, which
>>> Windows isn't and I *think* even throwing MinGW into the mix doesn't
>>> change that. In any case, enabling polkit support on Windows is
>>> explicitly blocked at the Meson level, so that test will never be
>>> built on that platform.
>>>
>>> It looks like it might be feasible to sidestep the issue entirely by
>>> simply not builing this specific mock on Windows after all.
>>
>> NB, this is already pushed, so whatever comes out of this discussion
>> will need to revert that commit.
> 
> I'm aware of that. I don't even mind if the currently merged solution
> is what ships with 9.2.0 and a better one, assuming we can come up
> with it, is implemented afterwards.
> 
>> Now - Windows PE - are we really sure that it can't work on Windows? I
>> mean, I only have a vague knowledge on PE, but IIRC then the dynamic
>> linker does symbol resolution that's very similar to ELF. IOW, the first
>> .dll to provide the symbol (i.e. our mock) wins the race.
>>
>> I'm not saying our mock works on Windows as is. Heck, we don't even run
>> !mock tests on Windows. We run no tests there. But if PE was able to do
>> mocking, then not compiling this would be pity.
> 
> I'm not sure what the "PE" you talk about is.
> 
> Anyway my point is that, at least as far as I can tell, all of the
> test programs that use virgdbusmock are effectively no-op on Windows,
> which means that mocking gdbus on Windows should not be necessary and
> thus not building the mock library on that platform should be viable.
> 
> Other mocks might work on Windows, I'm not sure. I haven't
> investigated, and I'm not making blanket statements about the topic.
> I'm only discussing this specific mock, which is the one currently
> causing us grief and which looks like we could simply stop building
> on Windows without losing anything in the process.
> 

Ha, so after many failed attempts I am able to compile and *RUN* our
test suite under wine. I needed to hack the meson cross file mingw
ships. Anyway, what I'm seeing is couple of failed tests:


 14/213 libvirt / domaincapstest         FAIL  0.84s   exit status 1
 27/213 libvirt / sockettest             FAIL  0.42s   exit status 1
 28/213 libvirt / sysinfotest            FAIL  0.44s   exit status 1
 29/213 libvirt / storagevolxml2xmltest  FAIL  0.46s   exit status 1
 38/213 libvirt / vircryptotest          FAIL  0.42s   exit status 1
 41/213 libvirt / virfilecachetest       FAIL  0.44s   exit status 1
 46/213 libvirt / viridentitytest        FAIL  0.44s   exit status 3
 50/213 libvirt / virlockspacetest       FAIL  0.36s   exit status 1
 57/213 libvirt / virrotatingfiletest    FAIL  0.45s   exit status 1
 59/213 libvirt / virstringtest          FAIL  0.44s   exit status 1
 65/213 libvirt / vshtabletest           FAIL  0.44s   exit status 1


Now, some of these are true bugs (either in our code or in wine). I'll
try to post patches. Nevertheless, I can confirm that mocking works with
.dll too. Therefore, I'd like to continue building mocks AND keep this
original patch as is.

Michal

Re: [PATCH] tests: Compile virgdbusmock.c with GIO_COMPILATION enabled
Posted by Andrea Bolognani 1 year, 1 month ago
On Wed, Mar 29, 2023 at 11:08:56AM +0200, Michal Prívozník wrote:
> On 3/28/23 14:54, Andrea Bolognani wrote:
> > Anyway my point is that, at least as far as I can tell, all of the
> > test programs that use virgdbusmock are effectively no-op on Windows,
> > which means that mocking gdbus on Windows should not be necessary and
> > thus not building the mock library on that platform should be viable.
> >
> > Other mocks might work on Windows, I'm not sure. I haven't
> > investigated, and I'm not making blanket statements about the topic.
> > I'm only discussing this specific mock, which is the one currently
> > causing us grief and which looks like we could simply stop building
> > on Windows without losing anything in the process.
>
> Ha, so after many failed attempts I am able to compile and *RUN* our
> test suite under wine. I needed to hack the meson cross file mingw
> ships. Anyway, what I'm seeing is couple of failed tests:
>
>
>  14/213 libvirt / domaincapstest         FAIL  0.84s   exit status 1
>  27/213 libvirt / sockettest             FAIL  0.42s   exit status 1
>  28/213 libvirt / sysinfotest            FAIL  0.44s   exit status 1
>  29/213 libvirt / storagevolxml2xmltest  FAIL  0.46s   exit status 1
>  38/213 libvirt / vircryptotest          FAIL  0.42s   exit status 1
>  41/213 libvirt / virfilecachetest       FAIL  0.44s   exit status 1
>  46/213 libvirt / viridentitytest        FAIL  0.44s   exit status 3
>  50/213 libvirt / virlockspacetest       FAIL  0.36s   exit status 1
>  57/213 libvirt / virrotatingfiletest    FAIL  0.45s   exit status 1
>  59/213 libvirt / virstringtest          FAIL  0.44s   exit status 1
>  65/213 libvirt / vshtabletest           FAIL  0.44s   exit status 1
>
>
> Now, some of these are true bugs (either in our code or in wine). I'll
> try to post patches. Nevertheless, I can confirm that mocking works with
> .dll too.

Excellent stuff!

> Therefore, I'd like to continue building mocks AND keep this
> original patch as is.

Can you please clarify whether the three tests[1] that use
virgdbusmock (networkxml2firewalltest, virsystemdtest, virpolkittest)
run successfully under Wine and, if so, whether they do anything more
interesting than returning EXIT_AM_SKIP? Or can be modified to do so?

It seems to me that they're testing Unix-only features, so we're
never going to be able to meaningfully run them on Windows. And since
they're the only ones using virgdbusmock, going out of our way to
build that mock on Windows only for it to remain unused is pointless.

Again, I'm only talking about virgdbusmock here! All the other mocks
should clearly continue being built on Windows.


[1] I initially included virfirewalltest in the list, but that was
    incorrect as the relevant code is compiled out.
-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] tests: Compile virgdbusmock.c with GIO_COMPILATION enabled
Posted by Michal Prívozník 1 year, 1 month ago
On 3/29/23 11:52, Andrea Bolognani wrote:
> On Wed, Mar 29, 2023 at 11:08:56AM +0200, Michal Prívozník wrote:
>> On 3/28/23 14:54, Andrea Bolognani wrote:
>>> Anyway my point is that, at least as far as I can tell, all of the
>>> test programs that use virgdbusmock are effectively no-op on Windows,
>>> which means that mocking gdbus on Windows should not be necessary and
>>> thus not building the mock library on that platform should be viable.
>>>
>>> Other mocks might work on Windows, I'm not sure. I haven't
>>> investigated, and I'm not making blanket statements about the topic.
>>> I'm only discussing this specific mock, which is the one currently
>>> causing us grief and which looks like we could simply stop building
>>> on Windows without losing anything in the process.
>>
>> Ha, so after many failed attempts I am able to compile and *RUN* our
>> test suite under wine. I needed to hack the meson cross file mingw
>> ships. Anyway, what I'm seeing is couple of failed tests:
>>
>>
>>  14/213 libvirt / domaincapstest         FAIL  0.84s   exit status 1
>>  27/213 libvirt / sockettest             FAIL  0.42s   exit status 1
>>  28/213 libvirt / sysinfotest            FAIL  0.44s   exit status 1
>>  29/213 libvirt / storagevolxml2xmltest  FAIL  0.46s   exit status 1
>>  38/213 libvirt / vircryptotest          FAIL  0.42s   exit status 1
>>  41/213 libvirt / virfilecachetest       FAIL  0.44s   exit status 1
>>  46/213 libvirt / viridentitytest        FAIL  0.44s   exit status 3
>>  50/213 libvirt / virlockspacetest       FAIL  0.36s   exit status 1
>>  57/213 libvirt / virrotatingfiletest    FAIL  0.45s   exit status 1
>>  59/213 libvirt / virstringtest          FAIL  0.44s   exit status 1
>>  65/213 libvirt / vshtabletest           FAIL  0.44s   exit status 1
>>
>>
>> Now, some of these are true bugs (either in our code or in wine). I'll
>> try to post patches. Nevertheless, I can confirm that mocking works with
>> .dll too.
> 
> Excellent stuff!
> 
>> Therefore, I'd like to continue building mocks AND keep this
>> original patch as is.
> 
> Can you please clarify whether the three tests[1] that use
> virgdbusmock (networkxml2firewalltest, virsystemdtest, virpolkittest)

networkxml2firewalltest and virpolkittest aren't even built, well,
weren't in my setup. Which kind of makes sense as neither of the feature
they test can work on Windows.
And virsystemdtest is built but returns AM_SKIP instantly.

> run successfully under Wine and, if so, whether they do anything more
> interesting than returning EXIT_AM_SKIP? Or can be modified to do so?

I don't think it's worth the effort.

> 
> It seems to me that they're testing Unix-only features, so we're
> never going to be able to meaningfully run them on Windows. And since
> they're the only ones using virgdbusmock, going out of our way to
> build that mock on Windows only for it to remain unused is pointless.

Sure, but right now, without my patches (WIP) every test and every mock
is just a compile-only test. We are not running any of the test so they
are kind of pointless too. But if we are okay with just compile testing,
then why not build the mock?

But clearly you care more than I do, so please post a patch and I'm more
than happy to review it.

Michal

Re: [PATCH] tests: Compile virgdbusmock.c with GIO_COMPILATION enabled
Posted by Michal Prívozník 1 year, 1 month ago
On 3/28/23 14:54, Andrea Bolognani wrote:
> On Tue, Mar 28, 2023 at 02:29:10PM +0200, Michal Prívozník wrote:
>> On 3/28/23 12:58, Andrea Bolognani wrote:
>>> It looks like virgdbusmock is used by
>>>
>>>   networkxml2firewalltest
>>>   virfirewalltest
>>>   virsystemdtest
>>>   virpolkittest
>>>
>>> The first three are Linux-only; the last one is ELF-only, which
>>> Windows isn't and I *think* even throwing MinGW into the mix doesn't
>>> change that. In any case, enabling polkit support on Windows is
>>> explicitly blocked at the Meson level, so that test will never be
>>> built on that platform.
>>>
>>> It looks like it might be feasible to sidestep the issue entirely by
>>> simply not builing this specific mock on Windows after all.
>>
>> NB, this is already pushed, so whatever comes out of this discussion
>> will need to revert that commit.
> 
> I'm aware of that. I don't even mind if the currently merged solution
> is what ships with 9.2.0 and a better one, assuming we can come up
> with it, is implemented afterwards.
> 
>> Now - Windows PE - are we really sure that it can't work on Windows? I
>> mean, I only have a vague knowledge on PE, but IIRC then the dynamic
>> linker does symbol resolution that's very similar to ELF. IOW, the first
>> .dll to provide the symbol (i.e. our mock) wins the race.
>>
>> I'm not saying our mock works on Windows as is. Heck, we don't even run
>> !mock tests on Windows. We run no tests there. But if PE was able to do
>> mocking, then not compiling this would be pity.
> 
> I'm not sure what the "PE" you talk about is.

PE stands for Portable Executable:

https://en.wikipedia.org/wiki/Portable_Executable

It's a format (inspired by ELF, well, its predecessor COFF) that Windows
uses.

> 
> Anyway my point is that, at least as far as I can tell, all of the
> test programs that use virgdbusmock are effectively no-op on Windows,
> which means that mocking gdbus on Windows should not be necessary and
> thus not building the mock library on that platform should be viable.

Yeah. Though, there is a windows port of d-bus. But I guess nobody
really uses that.

> 
> Other mocks might work on Windows, I'm not sure. I haven't
> investigated, and I'm not making blanket statements about the topic.
> I'm only discussing this specific mock, which is the one currently
> causing us grief and which looks like we could simply stop building
> on Windows without losing anything in the process.
> 

Question then is whether to build all mocks. I mean, if we claim our
mocking to be ELF specific, then certainly it doesn't make sense to
build on !ELF OSes. But if my assumption is not right and mocking can
indeed work then I guess we should make our mocks at least compile.

Actually, let me see if I can build libvirt with mingw locally and run
test(s) with wine.

Michal

Re: [PATCH] tests: Compile virgdbusmock.c with GIO_COMPILATION enabled
Posted by Martin Kletzander 1 year, 1 month ago
On Mon, Mar 27, 2023 at 02:47:40PM +0200, Michal Privoznik wrote:
>There are couple of g_dbus_*() functions we provide an
>alternative implementation for in our virgdbusmock.c. However,
>these functions are declared in gio/gdbusconnection.h as:
>
>  GIO_AVAILABLE_IN_ALL
>  GDBusConnection  *g_bus_get_sync (GBusType            bus_type,
>                                    GCancellable       *cancellable,
>                                    GError            **error);
>
>where GIO_AVAILABLE_IN_ALL is declared as (in
>/gio/gio-visibility.h):
>
>  #if (defined(_WIN32) || defined(__CYGWIN__)) && !defined(GIO_STATIC_COMPILATION)
>  #  define _GIO_EXPORT __declspec(dllexport)
>  #  define _GIO_IMPORT __declspec(dllimport)
>  #elif __GNUC__ >= 4
>  #  define _GIO_EXPORT __attribute__((visibility("default")))
>  #  define _GIO_IMPORT
>  #else
>  #  define _GIO_EXPORT
>  #  define _GIO_IMPORT
>  #endif
>  #ifdef GIO_COMPILATION
>  #  define _GIO_API _GIO_EXPORT
>  #else
>  #  define _GIO_API _GIO_IMPORT
>  #endif
>
>  #define _GIO_EXTERN _GIO_API extern
>
>  #define GIO_AVAILABLE_IN_ALL _GIO_EXTERN
>
>Now, on mingw the functions we mock are declared with dllimport
>attribute which makes the compiler unhappy:
>
>  ../tests/virgdbusmock.c:25:24: error: 'g_bus_get_sync'
>  redeclared without dllimport attribute: previous dllimport
>  ignored [-Werror=attributes]
>
>The solution is to do what glib does when it compiles the gio
>module: set GIO_COMPILATION macro which in turn annotates the
>function with dllexport attribute.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

and safe for freeze

>---
> tests/meson.build | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/tests/meson.build b/tests/meson.build
>index 0fd3bc62cf..6be806f4ae 100644
>--- a/tests/meson.build
>+++ b/tests/meson.build
>@@ -65,6 +65,10 @@ else
> endif
>
>
>+virgdbusmock_dep = declare_dependency(
>+  compile_args: [ '-DGIO_COMPILATION' ]
>+)
>+
> # mock_libs:
> #   each entry is a dictionary with following items:
> #   * name - mock library name which is also used as default source file name (required)
>@@ -78,7 +82,7 @@ mock_libs = [
>   { 'name': 'virdnsmasqmock' },
>   { 'name': 'virfilecachemock' },
>   { 'name': 'virfirewallmock' },
>-  { 'name': 'virgdbusmock' },
>+  { 'name': 'virgdbusmock', 'deps': [ virgdbusmock_dep] },
>   { 'name': 'virhostcpumock' },
>   { 'name': 'virhostdevmock' },
>   { 'name': 'virnetdaemonmock' },
>-- 
>2.39.2
>