On Fri, Apr 17, 2015 at 09:41:19PM -0700, Ben Pfaff wrote: > > On Sat, Apr 18, 2015 at 01:28:21AM +0800, Kevin Lo wrote: > > On Fri, Apr 17, 2015 at 09:17:46AM -0700, Ben Pfaff wrote: > > > > > > On Fri, Apr 17, 2015 at 02:45:32PM +0800, Kevin Lo wrote: > > > > On Wed, Apr 15, 2015 at 05:50:42PM -0700, Ben Pfaff wrote: > > > > > > > > > > On Wed, Apr 15, 2015 at 07:52:24AM -0700, Gurucharan Shetty wrote: > > > > > > > > > > > > > > I am also concerned that it this will break the Windows build. > > > > > > > Currently Windows uses some Windows-only code in > > > > > > > m4/openvswitch.m4, > > > > > > > which requires Win32 builders to specify --with-pthread=<dir> on > > > > > > > the > > > > > > > configure command line. Ideally, we would want Windows builds to > > > > > > > work > > > > > > > the same as other builds. Maybe that would mean that Windows > > > > > > > builders > > > > > > > should specify the PTHREAD_* variables on the configure command > > > > > > > line, > > > > > > > instead of --with-pthread, or that the OVS pthread-win32 support > > > > > > > should > > > > > > > move from OVS_CHECK_WIN32 to somewhere around the new invocation > > > > > > > of > > > > > > > AX_PTHREAD. Either way, I think that this will require some > > > > > > > change to > > > > > > > what this patch does (and possibly an update to > > > > > > > INSTALL.Windows.md) > > > > > > > before it can go in. I'm CCing Guru, who knows the Windows > > > > > > > build, to > > > > > > > get his opinion. > > > > > > > > > > > > commit 94887cf4caa74bfb5 added the support for pthreads check for > > > > > > Windows. > > > > > > As one can see in that commit, we have very specific Windows related > > > > > > includes, ldflags, libs that we need added. This is mainly because > > > > > > pthreads on Windows is downloadable and installed in any directory. > > > > > > I > > > > > > do not know what is a good portable solution here if we need to make > > > > > > Windows related changes again there. > > > > > > > > > > > > One concern that I have in changing '--with-pthread' for Windows > > > > > > means > > > > > > that the auto builds at different places will also need changing. > > > > > > Unless the benefits look good, I would like to avoid making that > > > > > > change. > > > > > > > > > > I think that this is pretty close to working already, actually. The > > > > > proposed AX_PTHREAD allows previously set shell variables > > > > > PTHREAD_CFLAGS > > > > > and PTHREAD_LIBS to control how pthread compiling and linking should > > > > > work. I think that OVS_CHECK_WIN32 could simply set these variables: > > > > > PTHREAD_CFLAGS to the -I option and PTHREAD_LIBS to the -L and -l > > > > > options. > > > > > > > > > > We'd want to ensure that OVS_CHECK_WIN32 gets called before > > > > > AX_PTHREAD. > > > > > I think we can add AC_BEFORE([$0], [AX_PTHREAD]) to OVS_CHECK_WIN32 to > > > > > ensure that. > > > > > > > > I know some of you are concerned about it will break build on Windows. > > > > The diff below fixes pthread linking on FreeBSD. > > > > Tested on Linux, NetBSD, and FreeBSD. > > > > > > Does the following even simpler change also fix the problem on BSD? > > > > Indeed. I must have more coffee :( Sorry for the noise. > > Will you submit this diff with an updated commit message, then? I'm > happy that we arrived at such a straightforward solution!
Done. http://openvswitch.org/pipermail/dev/2015-April/054242.html Thanks. > > > diff --git a/configure.ac b/configure.ac > > > index 8af5ef0..b100c80 100644 > > > --- a/configure.ac > > > +++ b/configure.ac > > > @@ -81,7 +81,7 @@ AC_SUBST([LT_AGE]) > > > AC_SEARCH_LIBS([pow], [m]) > > > AC_SEARCH_LIBS([clock_gettime], [rt]) > > > AC_SEARCH_LIBS([timer_create], [rt]) > > > -AC_SEARCH_LIBS([pthread_sigmask], [pthread]) > > > +AC_SEARCH_LIBS([pthread_create], [pthread]) > > > AC_FUNC_STRERROR_R > > > > > > OVS_CHECK_ESX _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
