Re: Review Request 35234: libprocess: consistent handling of --enable options
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/#review141479 --- Closing this review due to inactivity. Please see our [guidelines](https://github.com/apache/mesos/blob/master/docs/reopening-reviews.md) for reopening reviews. - Joris Van Remoortere On Aug. 26, 2015, 8:09 p.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35234/ > --- > > (Updated Aug. 26, 2015, 8:09 p.m.) > > > Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. > Clair. > > > Bugs: MESOS-2537 > https://issues.apache.org/jira/browse/MESOS-2537 > > > Repository: mesos > > > Description > --- > > Let both --enable-$OPTION and --disable-$OPTION work consistently. > Add bundled package options consistent with Mesos, so that options > passed down from Mesos work correctly. > > > Diffs > - > > 3rdparty/libprocess/3rdparty/Makefile.am > eb34251d24b1e5d1540151b59cf1062ca85aeb03 > 3rdparty/libprocess/configure.ac 7c2bcffe5c7be1f7d90e6df470d20a00245bfbff > > Diff: https://reviews.apache.org/r/35234/diff/ > > > Testing > --- > > Make and make check on CentOS 7 and OS X. There's definitely combinations > that have not been tested! > > Note that this removes some login around using gmock. AFAICT the unbundled > gmock doesn't work in the general case. I have a bunch of crashes where the > build would pick up gtest headers from the system and gmock from libprocess > 3rdparty. My conclusion is that the only safe path is to use the bundled > gmock. There's no real path through the build to use decoupled gmock and > gtest, it seems to be assumed that gmock will provide gtest. > > > Thanks, > > James Peach > >
Re: Review Request 35234: libprocess: consistent handling of --enable options
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/#review96594 --- Patch looks great! Reviews applied: [33752, 35084, 35234] All tests passed. - Mesos ReviewBot On Aug. 26, 2015, 8:09 p.m., James Peach wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/ --- (Updated Aug. 26, 2015, 8:09 p.m.) Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair. Bugs: MESOS-2537 https://issues.apache.org/jira/browse/MESOS-2537 Repository: mesos Description --- Let both --enable-$OPTION and --disable-$OPTION work consistently. Add bundled package options consistent with Mesos, so that options passed down from Mesos work correctly. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am eb34251d24b1e5d1540151b59cf1062ca85aeb03 3rdparty/libprocess/configure.ac 7c2bcffe5c7be1f7d90e6df470d20a00245bfbff Diff: https://reviews.apache.org/r/35234/diff/ Testing --- Make and make check on CentOS 7 and OS X. There's definitely combinations that have not been tested! Note that this removes some login around using gmock. AFAICT the unbundled gmock doesn't work in the general case. I have a bunch of crashes where the build would pick up gtest headers from the system and gmock from libprocess 3rdparty. My conclusion is that the only safe path is to use the bundled gmock. There's no real path through the build to use decoupled gmock and gtest, it seems to be assumed that gmock will provide gtest. Thanks, James Peach
Re: Review Request 35234: libprocess: consistent handling of --enable options
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/ --- (Updated Aug. 19, 2015, 8:09 p.m.) Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair. Changes --- Rebased changes onto current master. Bugs: MESOS-2537 https://issues.apache.org/jira/browse/MESOS-2537 Repository: mesos Description --- Let both --enable-$OPTION and --disable-$OPTION work consistently. Add bundled package options consistent with Mesos, so that options passed down from Mesos work correctly. Diffs (updated) - 3rdparty/libprocess/3rdparty/Makefile.am 790bb463e7bab43ad0111010d6db7befd160948e 3rdparty/libprocess/configure.ac 7c2bcffe5c7be1f7d90e6df470d20a00245bfbff Diff: https://reviews.apache.org/r/35234/diff/ Testing --- Make and make check on CentOS 7 and OS X. There's definitely combinations that have not been tested! Note that this removes some login around using gmock. AFAICT the unbundled gmock doesn't work in the general case. I have a bunch of crashes where the build would pick up gtest headers from the system and gmock from libprocess 3rdparty. My conclusion is that the only safe path is to use the bundled gmock. There's no real path through the build to use decoupled gmock and gtest, it seems to be assumed that gmock will provide gtest. Thanks, James Peach
Re: Review Request 35234: libprocess: consistent handling of --enable options
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/#review95915 --- Bad patch! Reviews applied: [33752, 35084] Failed command: ./support/apply-review.sh -n -r 35084 Error: 2015-08-20 05:16:46 URL:https://reviews.apache.org/r/35084/diff/raw/ [17310/17310] - 35084.patch [1] error: patch failed: configure.ac:444 error: configure.ac: patch does not apply Failed to apply patch - Mesos ReviewBot On Aug. 19, 2015, 8:09 p.m., James Peach wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/ --- (Updated Aug. 19, 2015, 8:09 p.m.) Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair. Bugs: MESOS-2537 https://issues.apache.org/jira/browse/MESOS-2537 Repository: mesos Description --- Let both --enable-$OPTION and --disable-$OPTION work consistently. Add bundled package options consistent with Mesos, so that options passed down from Mesos work correctly. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am 790bb463e7bab43ad0111010d6db7befd160948e 3rdparty/libprocess/configure.ac 7c2bcffe5c7be1f7d90e6df470d20a00245bfbff Diff: https://reviews.apache.org/r/35234/diff/ Testing --- Make and make check on CentOS 7 and OS X. There's definitely combinations that have not been tested! Note that this removes some login around using gmock. AFAICT the unbundled gmock doesn't work in the general case. I have a bunch of crashes where the build would pick up gtest headers from the system and gmock from libprocess 3rdparty. My conclusion is that the only safe path is to use the bundled gmock. There's no real path through the build to use decoupled gmock and gtest, it seems to be assumed that gmock will provide gtest. Thanks, James Peach
Re: Review Request 35234: libprocess: consistent handling of --enable options
On June 9, 2015, 12:26 a.m., Till Toenshoff wrote: 3rdparty/libprocess/configure.ac, line 31 https://reviews.apache.org/r/35234/diff/1/?file=980998#file980998line31 Can we switch to `#` prefixed comments here instead? James Peach wrote: Originally I used #-comments, but since autoconf completely elides the macro definition, this results in the header comments hanging in the middle of nowhere in the resulting configure script, which just looks weird. If you really want me to change it I will though :) Sounds good to me - Cody --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/#review87101 --- On June 9, 2015, 12:04 a.m., James Peach wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/ --- (Updated June 9, 2015, 12:04 a.m.) Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair. Bugs: MESOS-2537 https://issues.apache.org/jira/browse/MESOS-2537 Repository: mesos Description --- Let both --enable-$OPTION and --disable-$OPTION work consistently. Add bundled package options consistent with Mesos, so that options passed down from Mesos work correctly. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am 519e38c2c22904e75f36b936142a915a8f615b21 3rdparty/libprocess/configure.ac 710490b2a7c71f35434494e87e2d132f78ef370a Diff: https://reviews.apache.org/r/35234/diff/ Testing --- Make and make check on CentOS 7 and OS X. There's definitely combinations that have not been tested! Note that this removes some login around using gmock. AFAICT the unbundled gmock doesn't work in the general case. I have a bunch of crashes where the build would pick up gtest headers from the system and gmock from libprocess 3rdparty. My conclusion is that the only safe path is to use the bundled gmock. There's no real path through the build to use decoupled gmock and gtest, it seems to be assumed that gmock will provide gtest. Thanks, James Peach
Re: Review Request 35234: libprocess: consistent handling of --enable options
On June 9, 2015, 12:26 a.m., Till Toenshoff wrote: 3rdparty/libprocess/configure.ac, line 31 https://reviews.apache.org/r/35234/diff/1/?file=980998#file980998line31 Can we switch to `#` prefixed comments here instead? Originally I used #-comments, but since autoconf completely elides the macro definition, this results in the header comments hanging in the middle of nowhere in the resulting configure script, which just looks weird. If you really want me to change it I will though :) - James --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/#review87101 --- On June 9, 2015, 12:04 a.m., James Peach wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/ --- (Updated June 9, 2015, 12:04 a.m.) Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair. Bugs: MESOS-2537 https://issues.apache.org/jira/browse/MESOS-2537 Repository: mesos Description --- Let both --enable-$OPTION and --disable-$OPTION work consistently. Add bundled package options consistent with Mesos, so that options passed down from Mesos work correctly. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am 519e38c2c22904e75f36b936142a915a8f615b21 3rdparty/libprocess/configure.ac 710490b2a7c71f35434494e87e2d132f78ef370a Diff: https://reviews.apache.org/r/35234/diff/ Testing --- Make and make check on CentOS 7 and OS X. There's definitely combinations that have not been tested! Note that this removes some login around using gmock. AFAICT the unbundled gmock doesn't work in the general case. I have a bunch of crashes where the build would pick up gtest headers from the system and gmock from libprocess 3rdparty. My conclusion is that the only safe path is to use the bundled gmock. There's no real path through the build to use decoupled gmock and gtest, it seems to be assumed that gmock will provide gtest. Thanks, James Peach
Re: Review Request 35234: libprocess: consistent handling of --enable options
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/#review87118 --- Patch looks great! Reviews applied: [33752, 35084, 35234] All tests passed. - Mesos ReviewBot On June 9, 2015, 12:04 a.m., James Peach wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/ --- (Updated June 9, 2015, 12:04 a.m.) Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair. Bugs: MESOS-2537 https://issues.apache.org/jira/browse/MESOS-2537 Repository: mesos Description --- Let both --enable-$OPTION and --disable-$OPTION work consistently. Add bundled package options consistent with Mesos, so that options passed down from Mesos work correctly. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am 519e38c2c22904e75f36b936142a915a8f615b21 3rdparty/libprocess/configure.ac 710490b2a7c71f35434494e87e2d132f78ef370a Diff: https://reviews.apache.org/r/35234/diff/ Testing --- Make and make check on CentOS 7 and OS X. There's definitely combinations that have not been tested! Note that this removes some login around using gmock. AFAICT the unbundled gmock doesn't work in the general case. I have a bunch of crashes where the build would pick up gtest headers from the system and gmock from libprocess 3rdparty. My conclusion is that the only safe path is to use the bundled gmock. There's no real path through the build to use decoupled gmock and gtest, it seems to be assumed that gmock will provide gtest. Thanks, James Peach
Re: Review Request 35234: libprocess: consistent handling of --enable options
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/#review87101 --- Great updates - quick style review which applies to the dependending one as well. 3rdparty/libprocess/configure.ac https://reviews.apache.org/r/35234/#comment139405 Can we switch to `#` prefixed comments here instead? 3rdparty/libprocess/configure.ac https://reviews.apache.org/r/35234/#comment139412 s/packaged/package/g 3rdparty/libprocess/configure.ac https://reviews.apache.org/r/35234/#comment139408 Please end comments with a punctuation. - Till Toenshoff On June 9, 2015, 12:04 a.m., James Peach wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35234/ --- (Updated June 9, 2015, 12:04 a.m.) Review request for mesos, Benjamin Hindman, Cody Maloney, and Timothy St. Clair. Bugs: MESOS-2537 https://issues.apache.org/jira/browse/MESOS-2537 Repository: mesos Description --- Let both --enable-$OPTION and --disable-$OPTION work consistently. Add bundled package options consistent with Mesos, so that options passed down from Mesos work correctly. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am 519e38c2c22904e75f36b936142a915a8f615b21 3rdparty/libprocess/configure.ac 710490b2a7c71f35434494e87e2d132f78ef370a Diff: https://reviews.apache.org/r/35234/diff/ Testing --- Make and make check on CentOS 7 and OS X. There's definitely combinations that have not been tested! Note that this removes some login around using gmock. AFAICT the unbundled gmock doesn't work in the general case. I have a bunch of crashes where the build would pick up gtest headers from the system and gmock from libprocess 3rdparty. My conclusion is that the only safe path is to use the bundled gmock. There's no real path through the build to use decoupled gmock and gtest, it seems to be assumed that gmock will provide gtest. Thanks, James Peach
