[PATCH] tests: Compile virgdbusmock.c with GIO_COMPILATION enabled
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
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
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
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
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
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
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
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
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
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
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
participants (4)
-
Andrea Bolognani -
Martin Kletzander -
Michal Privoznik -
Michal Prívozník