The Win32 platform can not do link time overrides in the same way
that we can on POSIX / ELF based platforms, so we cannot build
the virfilewrapper.c code reliably. Just stub it out on Win32
so it is a no-op. Tests that use this file are already written
to skip on Win32.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
Pushed as a mingw build fix
tests/virfilewrapper.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/tests/virfilewrapper.c b/tests/virfilewrapper.c
index 5ea70b3..bf2fa69 100644
--- a/tests/virfilewrapper.c
+++ b/tests/virfilewrapper.c
@@ -18,15 +18,17 @@
#include <config.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <fcntl.h>
+#ifndef WIN32
-#include "viralloc.h"
-#include "virfile.h"
-#include "virfilewrapper.h"
-#include "virmock.h"
-#include "virstring.h"
+# include <stdio.h>
+# include <stdlib.h>
+# include <fcntl.h>
+
+# include "viralloc.h"
+# include "virfile.h"
+# include "virfilewrapper.h"
+# include "virmock.h"
+# include "virstring.h"
/* Mapping for prefix overrides */
@@ -139,7 +141,7 @@ virFileWrapperOverridePrefix(const char *path)
}
-#define PATH_OVERRIDE(newpath, path) \
+# define PATH_OVERRIDE(newpath, path) \
do { \
init_syms(); \
\
@@ -177,7 +179,7 @@ int access(const char *path, int mode)
return ret;
}
-#ifdef HAVE___LXSTAT
+# ifdef HAVE___LXSTAT
int __lxstat(int ver, const char *path, struct stat *sb)
{
int ret = -1;
@@ -191,7 +193,7 @@ int __lxstat(int ver, const char *path, struct stat *sb)
return ret;
}
-#endif /* HAVE___LXSTAT */
+# endif /* HAVE___LXSTAT */
int lstat(const char *path, struct stat *sb)
{
@@ -207,7 +209,7 @@ int lstat(const char *path, struct stat *sb)
return ret;
}
-#ifdef HAVE___XSTAT
+# ifdef HAVE___XSTAT
int __xstat(int ver, const char *path, struct stat *sb)
{
int ret = -1;
@@ -221,7 +223,7 @@ int __xstat(int ver, const char *path, struct stat *sb)
return ret;
}
-#endif /* HAVE___XSTAT */
+# endif /* HAVE___XSTAT */
int stat(const char *path, struct stat *sb)
{
@@ -278,3 +280,4 @@ DIR *opendir(const char *path)
return ret;
}
+#endif
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, May 11, 2017 at 11:46:01AM +0100, Daniel P. Berrange wrote: >The Win32 platform can not do link time overrides in the same way >that we can on POSIX / ELF based platforms, so we cannot build >the virfilewrapper.c code reliably. Just stub it out on Win32 >so it is a no-op. Tests that use this file are already written >to skip on Win32. > I understood we wanted to see that some tests were skipped on a particular platform. But we already have so many tests build conditionally, why don't we just exclude all of them in a Makefile? That way we wouldn't need to make whole file inside a condition. I can walk through them and clean it up, I just want to know if it makes sense. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, May 11, 2017 at 03:13:03PM +0200, Martin Kletzander wrote: > On Thu, May 11, 2017 at 11:46:01AM +0100, Daniel P. Berrange wrote: > > The Win32 platform can not do link time overrides in the same way > > that we can on POSIX / ELF based platforms, so we cannot build > > the virfilewrapper.c code reliably. Just stub it out on Win32 > > so it is a no-op. Tests that use this file are already written > > to skip on Win32. > > > > I understood we wanted to see that some tests were skipped on a > particular platform. But we already have so many tests build > conditionally, why don't we just exclude all of them in a Makefile? > That way we wouldn't need to make whole file inside a condition. I can > walk through them and clean it up, I just want to know if it makes > sense. The benefit of compiling the tests, but having them exit status 77 is that it means automake test harness then clearly reports which tests are being skipped on each platform. If we do conditionals in the makefile, them skipped tests are invisible - they just disappear entirely from output. So I prefer that we use the exit status 77 Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, May 11, 2017 at 02:16:11PM +0100, Daniel P. Berrange wrote: >On Thu, May 11, 2017 at 03:13:03PM +0200, Martin Kletzander wrote: >> On Thu, May 11, 2017 at 11:46:01AM +0100, Daniel P. Berrange wrote: >> > The Win32 platform can not do link time overrides in the same way >> > that we can on POSIX / ELF based platforms, so we cannot build >> > the virfilewrapper.c code reliably. Just stub it out on Win32 >> > so it is a no-op. Tests that use this file are already written >> > to skip on Win32. >> > >> >> I understood we wanted to see that some tests were skipped on a >> particular platform. But we already have so many tests build >> conditionally, why don't we just exclude all of them in a Makefile? >> That way we wouldn't need to make whole file inside a condition. I can >> walk through them and clean it up, I just want to know if it makes >> sense. > >The benefit of compiling the tests, but having them exit status 77 >is that it means automake test harness then clearly reports which >tests are being skipped on each platform. If we do conditionals >in the makefile, them skipped tests are invisible - they just >disappear entirely from output. So I prefer that we use the exit >status 77 > Yes. As I said, I understand that. But we already have bunch of test_programs appended only conditionally. And I felt like mixing the approach is not really intuitive for readers. >Regards, >Daniel >-- >|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| >|: https://libvirt.org -o- https://fstop138.berrange.com :| >|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, May 11, 2017 at 03:40:37PM +0200, Martin Kletzander wrote: > On Thu, May 11, 2017 at 02:16:11PM +0100, Daniel P. Berrange wrote: > > On Thu, May 11, 2017 at 03:13:03PM +0200, Martin Kletzander wrote: > > > On Thu, May 11, 2017 at 11:46:01AM +0100, Daniel P. Berrange wrote: > > > > The Win32 platform can not do link time overrides in the same way > > > > that we can on POSIX / ELF based platforms, so we cannot build > > > > the virfilewrapper.c code reliably. Just stub it out on Win32 > > > > so it is a no-op. Tests that use this file are already written > > > > to skip on Win32. > > > > > > > > > > I understood we wanted to see that some tests were skipped on a > > > particular platform. But we already have so many tests build > > > conditionally, why don't we just exclude all of them in a Makefile? > > > That way we wouldn't need to make whole file inside a condition. I can > > > walk through them and clean it up, I just want to know if it makes > > > sense. > > > > The benefit of compiling the tests, but having them exit status 77 > > is that it means automake test harness then clearly reports which > > tests are being skipped on each platform. If we do conditionals > > in the makefile, them skipped tests are invisible - they just > > disappear entirely from output. So I prefer that we use the exit > > status 77 > > > > Yes. As I said, I understand that. But we already have bunch of > test_programs appended only conditionally. And I felt like mixing the > approach is not really intuitive for readers. My vote would be to get rid of the makefile conditionals because they often lead to bugs in 'make dist' missing out files. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.