Bug 969231

Summary: [PATCH] FTBFS: mingw-libvirt fails to build when linking against mingw-winpthreads
Product: [Fedora] Fedora Reporter: Erik van Pienbroek <erik-fedora>
Component: mingw-libvirtAssignee: Daniel Berrangé <berrange>
Status: CLOSED UPSTREAM QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: berrange, eblake, erik-fedora, marcandre.lureau, rjones
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-08-03 19:40:04 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Proposed patch none

Description Erik van Pienbroek 2013-05-31 00:44:52 UTC
Created attachment 755122 [details]
Proposed patch

In rawhide we're currently preparing the introduction of winpthreads as part of our mingw cross compiler toolchain: http://lists.fedoraproject.org/pipermail/fedora-mingw/2013-April/006628.html . During an initial test mass rebuild the package mingw-libvirt failed to build: http://lists.fedoraproject.org/pipermail/fedora-mingw/2013-May/006740.html

The build failed with this error:
  CCLD   libvirt.la
./.libs/libvirt_driver_remote.a(libvirt_net_rpc_client_la-virnetclient.o): In function `virNetClientIOEventLoop':
/builddir/build/BUILD/libvirt-0.10.2/build_win32/src/../../src/rpc/virnetclient.c:1521: undefined reference to `pthread_sigmask'
/builddir/build/BUILD/libvirt-0.10.2/build_win32/src/../../src/rpc/virnetclient.c:1528: undefined reference to `pthread_sigmask'
/builddir/build/BUILD/libvirt-0.10.2/build_win32/src/../../src/rpc/virnetclient.c:1528: undefined reference to `pthread_sigmask'
./.libs/libvirt_driver_remote.a(libvirt_net_rpc_client_la-virnetclient.o): In function `virNetClientSetTLSSession':
/builddir/build/BUILD/libvirt-0.10.2/build_win32/src/../../src/rpc/virnetclient.c:795: undefined reference to `pthread_sigmask'
/builddir/build/BUILD/libvirt-0.10.2/build_win32/src/../../src/rpc/virnetclient.c:802: undefined reference to `pthread_sigmask'
./.libs/libvirt_driver_remote.a(libvirt_net_rpc_client_la-virnetclient.o):/builddir/build/BUILD/libvirt-0.10.2/build_win32/src/../../src/rpc/virnetclient.c:819: more undefined references to `pthread_sigmask' follow

I've been able to workaround it with the attached patch. Does this look like a good change to you and could you include it in upstream libvirt?

A successful koji scratch build with this patch was done at http://koji.fedoraproject.org/koji/taskinfo?taskID=5443695

Comment 1 Daniel Berrangé 2013-06-03 11:24:28 UTC
Hmm, libvirt is supposed to be using Native Win32 threads for all its threading support.  For the pthread_sigmask function, we were relying on gnulib to provide it. That said I'm curious as to how adding a #include will fix a linker problem. I would have expected a compiler error rather than linker error for a missing include file.

In any case the definition of pthread_sigmask  is supposed to be provided by signal.h rather than pthread.h according to POSIX.

http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_sigmask.html

and virnetclient.c already includes signal.h So if the provided patch really does fix the problem, then this looks like it might be highlighting a bug in winpthreads exposing the function in the wrong header file.

Comment 2 Erik van Pienbroek 2013-06-03 22:32:08 UTC
(In reply to Daniel Berrange from comment #1)
> That said I'm curious as to how adding a #include will
> fix a linker problem. I would have expected a compiler error rather than
> linker error for a missing include file.

Apparently winpthreads doesn't have a real implementation of pthread_sigmask, but by including pthread.h it will get defined to a no-op:

$ grep pthread_sigmask /usr/i686-w64-mingw32/sys-root/mingw/include/pthread.h 
#define pthread_sigmask(H, S1, S2) 0

So by including pthread.h the mentioned linker error can be avoided

> In any case the definition of pthread_sigmask  is supposed to be provided by
> signal.h rather than pthread.h according to POSIX.
> 
> http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_sigmask.html

Thanks for the reference documentation. I'll pass this information on to the upstream winpthreads developers so they can move the declaration to signal.h

Comment 3 Erik van Pienbroek 2013-08-03 19:40:04 UTC
Closing bug, this is an upstream winpthreads issue which must be resolved there

Comment 4 Eric Blake 2013-08-29 22:15:54 UTC
I may end up reopening this, if I can come up with a workaround so that libvirt will build now without waiting on a winpthreads fix.