Bug 969231 - [PATCH] FTBFS: mingw-libvirt fails to build when linking against mingw-winpthreads
Summary: [PATCH] FTBFS: mingw-libvirt fails to build when linking against mingw-winpth...
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Fedora
Classification: Fedora
Component: mingw-libvirt
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Daniel Berrangé
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-05-31 00:44 UTC by Erik van Pienbroek
Modified: 2013-08-29 22:15 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-08-03 19:40:04 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
Proposed patch (329 bytes, patch)
2013-05-31 00:44 UTC, Erik van Pienbroek
no flags Details | Diff

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.


Note You need to log in before you can comment on or make changes to this bug.