From reviews-return-51176-apmail-mesos-reviews-archive=mesos.apache.org@mesos.apache.org Mon Nov 28 22:02:49 2016 Return-Path: X-Original-To: apmail-mesos-reviews-archive@minotaur.apache.org Delivered-To: apmail-mesos-reviews-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 093A91965C for ; Mon, 28 Nov 2016 22:02:49 +0000 (UTC) Received: (qmail 50360 invoked by uid 500); 28 Nov 2016 22:02:49 -0000 Delivered-To: apmail-mesos-reviews-archive@mesos.apache.org Received: (qmail 50335 invoked by uid 500); 28 Nov 2016 22:02:48 -0000 Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@mesos.apache.org Delivered-To: mailing list reviews@mesos.apache.org Received: (qmail 50313 invoked by uid 99); 28 Nov 2016 22:02:48 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 28 Nov 2016 22:02:48 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 9E9A92F6B62; Mon, 28 Nov 2016 22:02:46 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============5654961237894503522==" MIME-Version: 1.0 Subject: Re: Review Request 53759: CMake: Change libprocess to a shared library. From: Joseph Wu To: Joris Van Remoortere , Alex Clemmer Cc: Joseph Wu , mesos Date: Mon, 28 Nov 2016 22:02:46 -0000 Message-ID: <20161128220246.5576.58125@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: Joseph Wu X-ReviewGroup: mesos X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/53759/ X-Sender: Joseph Wu References: <20161117170914.20484.77056@reviews.apache.org> In-Reply-To: <20161117170914.20484.77056@reviews.apache.org> Reply-To: Joseph Wu X-ReviewRequest-Repository: mesos --===============5654961237894503522== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On Nov. 17, 2016, 9:09 a.m., Alex Clemmer wrote: > > 3rdparty/libprocess/src/CMakeLists.txt, line 93 > > > > > > Seems like this should be using `MESOS_DEFAULT_LIBRARY_LINKAGE`? > > Joseph Wu wrote: > Nope, Mesos variables shouldn't be used in stout/libprocess. > > Alex Clemmer wrote: > Right. I suppose my real question is: should this be a variable of some sort? It seems odd we'd use one elsewhere, but not for Stout. For the 3rdparty libraries, libprocess is the only one where we explicitly control the linking strategy. (Stout is header-only and bundled dependencies have their own build systems.) I suppose the HTTP parser library also counts, but this one is also built as an external project, meaning that CMake variables will not be inherited. In the automake build, there is an option to build libprocess as a static library, but that build path is untested (and completely broken :). We can consider adding an equivalent variable in future, but for now, it is safer not to. - Joseph ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53759/#review156214 ----------------------------------------------------------- On Nov. 14, 2016, 7:09 p.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53759/ > ----------------------------------------------------------- > > (Updated Nov. 14, 2016, 7:09 p.m.) > > > Review request for mesos, Alex Clemmer and Joris Van Remoortere. > > > Bugs: MESOS-5792 > https://issues.apache.org/jira/browse/MESOS-5792 > > > Repository: mesos > > > Description > ------- > > In order to load modules that are themselves based on libprocess, > we must link libprocess as a shared library. Since modules are > only supported on non-Windows platforms, this changes the default > linking mode to SHARED on non-Windows. > > This review replaces: https://reviews.apache.org/r/49924/ > > > Diffs > ----- > > 3rdparty/libprocess/src/CMakeLists.txt d1547ef6a8762385f653d3824307727e4d0a7e71 > > Diff: https://reviews.apache.org/r/53759/diff/ > > > Testing > ------- > > cmake .. > make > > > Thanks, > > Joseph Wu > > --===============5654961237894503522==--