----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2981/#review3611 -----------------------------------------------------------
Makefile.am <https://reviews.apache.org/r/2981/#comment8049> I know you didn't add these spaces, but feel free to remove them here and everywhere else in this patch. Makefile.am <https://reviews.apache.org/r/2981/#comment8048> s/make they're/make sure they're Makefile.am <https://reviews.apache.org/r/2981/#comment8063> Where does this variable come from? configure.ac <https://reviews.apache.org/r/2981/#comment8051> Let's just kill these lines. configure.ac <https://reviews.apache.org/r/2981/#comment8052> Kill. configure.ac <https://reviews.apache.org/r/2981/#comment8808> Can we (do we) test for python before we just invoke it? configure.ac <https://reviews.apache.org/r/2981/#comment8809> Regarding my comment, does autotools give us most of what we want here? configure.ac <https://reviews.apache.org/r/2981/#comment8810> Kill any extra spaces, here and everywhere else. mesos-build-env.sh.in <https://reviews.apache.org/r/2981/#comment8811> I'd like to revisit how bad it would be to just use relative paths and have the build procedure copy artifacts to their necessary spot so that they work given the relative directories after building. I'm concerned about the added mental overhead of getting people used to using these extra scripts as well as exporting environmental variables (can cause frustrating issues) as well as getting this all to work with a debugger. It would be great to enumerate the issues getting the existing mesos-master, mesos-slave, etc to run to determine if we can come up with any (clever) solutions. mesos-slave-dev.in <https://reviews.apache.org/r/2981/#comment8812> s/master/slave mesos-slave-dev.in <https://reviews.apache.org/r/2981/#comment8813> s/master/slave src/Makefile.am <https://reviews.apache.org/r/2981/#comment8814> What is this for? src/Makefile.am <https://reviews.apache.org/r/2981/#comment8815> Can we do this? src/Makefile.am <https://reviews.apache.org/r/2981/#comment8816> Newline. src/Makefile.am <https://reviews.apache.org/r/2981/#comment8817> The rest of the file indents. src/Makefile.am <https://reviews.apache.org/r/2981/#comment8818> Why the extra variable? Why not just libmesos_la_LIBADD +=? Line length? src/Makefile.am <https://reviews.apache.org/r/2981/#comment8819> Wow, I didn't know MKDIR_P existed!!! Or $(@D) ... what does that mean? src/Makefile.am <https://reviews.apache.org/r/2981/#comment8820> Please wrap these lines for the 80 character freaks like me. src/common/process_utils.hpp <https://reviews.apache.org/r/2981/#comment8821> s/hardness/harness src/common/webui_utils.cpp <https://reviews.apache.org/r/2981/#comment8822> Brace on newline. src/java/src/org/apache/mesos/MesosSchedulerDriver.java <https://reviews.apache.org/r/2981/#comment8824> Put throw on new line inside braces. src/java/src/org/apache/mesos/MesosSchedulerDriver.java <https://reviews.apache.org/r/2981/#comment8823> ? src/master/webui.cpp <https://reviews.apache.org/r/2981/#comment8825> This comment still applies, even though you moved the code. src/slave/webui.cpp <https://reviews.apache.org/r/2981/#comment8826> Let's call spawnWebui start as well, the namespace says it all: utils::webui::start(... src/tests/base_zookeeper_test.cpp <https://reviews.apache.org/r/2981/#comment8827> Yeah, I like the idea of a source dir and a build dir ... src/tests/external/LxcIsolation/HoldMoreMemThanRequested.sh <https://reviews.apache.org/r/2981/#comment8829> Cool, so are we no longer copying these in the Makefile.am? src/tests/external/LxcIsolation/run_scheduled_memhog_test.sh <https://reviews.apache.org/r/2981/#comment8830> I would have expected first running the mesos-build-env.sh script ... so it is not needed? Or is it expected that it will be run before this? If it's not needed, great! src/tests/external/LxcIsolation/run_scheduled_memhog_test.sh <https://reviews.apache.org/r/2981/#comment8831> Put the args on continued newlines. src/tests/external/SampleFrameworks/CppFramework.sh <https://reviews.apache.org/r/2981/#comment8832> It might be nice to stick a test in each of these files that uses one of the environment variables that is expected and print if they don't exist ... and that these files are expected to get run from the the test binary which sets up all these envs. src/tests/external/SampleFrameworks/JavaFramework.sh <https://reviews.apache.org/r/2981/#comment8833> Why is this one in BUILD, but some others are in SOURCE, like the next one is in SOURCE. src/tests/main.cpp <https://reviews.apache.org/r/2981/#comment8828> s/ROOT_DIR/SOURCE_DIR ?? Pretty please? - Benjamin On 2011-11-30 19:15:42, Charles Reiss wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2981/ > ----------------------------------------------------------- > > (Updated 2011-11-30 19:15:42) > > > Review request for mesos, Benjamin Hindman and Andy Konwinski. > > > Summary > ------- > > This very large patch changes the build system to use automake, libtool. This > patch attempts to be feature-complete with the existing build system > (exception: Solaris support). > > Notable changes: > - Builds from source control now require configure, automake, libtool to be > installed; added ./bootstrap to create configure, Makefile.ins, etc. > (autoreconf will _not_ work.) > - Install location are no longer under $(prefix)/mesos; instead, we follow > the configure-set directories mostly, so > * mesos-master, mesos-slave go into $(sbindir) which defaults to > $(prefix)/sbin > * mesos-local, mesos-log, mesos-mesos (framework submission) go into > $(bindir) which defaults to $(prefix)/bin > * mesos-launcher (utility program for LXC isolation), killtree.sh (utility > program) go into $(pkglibexecdir) which defaults to $(prefix)/libexec/mesos > * mesos.jar goes into $(libdir)/java which defaults to $(prefix)/lib/java > * libmesos.{la,so,dylib} goes into $(libdir) which defaults to $(prefix)/lib > * webui python scripts go into $(pkgdatadir) which defaults to > $(prefix)/share/mesos > - New configuration options for finding install locations, with defaults > compiled in based on configured install location: > * webui_dir > * launcher_dir > - Created wrapper scripts mesos-master-dev and mesos-slave-dev that set > configuration environment variables suitable for running Mesos out of build > directory > - Tests find location of external files using MESOS_BUILD_DIR and > MESOS_SOURCE_DIR environment variables set by test harness > - libmesos_exec.a, libmesos_sched.a have been eliminated > - configure autodetects Java by default (it can still be disabled) > - Removes support for building without zookeeper? > - 'make test' changes names to 'make check'. > > Notable things not done: > - 'make dist' is broken, primarily because of third_party. > - We make third_party libraries part of libmesos.la; to do this, we link > (PIC) .a's and non-convenience static .la's into libmesos.la. This prevents > us from building a non-shared libmesos and may not be portable to some > platforms. This is not a change from what the existing build system does. > - the root directory and src have separate Makefile.am's; this means that > dependencies in src/ won't cause config.status to regenerate files created a > configure time from .in files. > > > This addresses bug MESOS-58. > https://issues.apache.org/jira/browse/MESOS-58 > > > Diffs > ----- > > Makefile.am PRE-CREATION > Makefile.in e8dec7a > bootstrap PRE-CREATION > config.guess f32079a > config.sub 6759825 > configure fc6bf00 > configure.ac f6b6a01 > include/mesos/mesos.hpp PRE-CREATION > install-sh 4fbbae7 > m4/dummy_m4_contents PRE-CREATION > mesos-build-env.sh.in PRE-CREATION > mesos-master-dev.in PRE-CREATION > mesos-slave-dev.in PRE-CREATION > src/Makefile.am PRE-CREATION > src/Makefile.in 516f128 > src/common/process_utils.hpp 8e55d76 > src/common/webui_utils.hpp PRE-CREATION > src/common/webui_utils.cpp PRE-CREATION > src/config/config.hpp.in 49f666a > src/detector/detector.cpp ef4aff7 > src/examples/cpp_test_executor.cpp 9dd244e > src/examples/cpp_test_framework.cpp ab02805 > src/examples/java/Makefile.in 6500ded > src/examples/java/TestExceptionFramework.java ba39757 > src/examples/java/TestFramework.java e1b6032 > src/examples/java/TestMultipleExecutorsFramework.java a49ecbb > src/examples/java/test-exception-framework PRE-CREATION > src/examples/java/test-executor PRE-CREATION > src/examples/java/test-framework PRE-CREATION > src/examples/java/test-multiple-executors-framework PRE-CREATION > src/examples/java/test_exception_framework be78062 > src/examples/java/test_executor d2fbb05 > src/examples/java/test_framework 2713b34 > src/examples/java/test_multiple_executors_framework 9fad319 > src/examples/python/test-executor PRE-CREATION > src/examples/python/test-framework PRE-CREATION > src/examples/python/test_executor 8d6cc64 > src/examples/python/test_framework 050ab90 > src/examples/python/test_framework.py e6893cf > src/examples/test_executor.cpp PRE-CREATION > src/examples/test_framework.cpp PRE-CREATION > src/java/mesos.pom e5b674d > src/java/mesos.pom.in PRE-CREATION > src/java/src/org/apache/mesos/MesosSchedulerDriver.java 04809a6 > src/jvm/jvm.hpp PRE-CREATION > src/jvm/jvm.cpp PRE-CREATION > src/log/log.hpp e50969b > src/log/network.hpp c88822c > src/master/slaves_manager.cpp 3f67aa1 > src/master/webui.cpp c5a20d1 > src/python/setup.py.in 7c1f6e5 > src/slave/lxc_isolation_module.cpp ab0843a > src/slave/webui.cpp bb1c780 > src/tests/base_zookeeper_test.hpp e7bb7ed > src/tests/base_zookeeper_test.cpp 202b66b > src/tests/external/LxcIsolation/HoldMoreMemThanRequested.sh 7b87a7b > src/tests/external/LxcIsolation/ScaleUpAndDown.sh 09333e2 > src/tests/external/LxcIsolation/TwoSeparateTasks.sh a2e2883 > src/tests/external/LxcIsolation/run_scheduled_memhog_test.sh 9ac7cd5 > src/tests/external/SampleFrameworks/CFrameworkCmdlineParsing.sh 791cbf1 > src/tests/external/SampleFrameworks/CFrameworkInvalidCmdline.sh 3197c81 > src/tests/external/SampleFrameworks/CFrameworkInvalidEnv.sh 427b33b > src/tests/external/SampleFrameworks/CppFramework.sh 968d951 > src/tests/external/SampleFrameworks/JavaExceptionFramework.sh bc4a4e0 > src/tests/external/SampleFrameworks/JavaFramework.sh db88d0b > src/tests/external/SampleFrameworks/PythonFramework.sh 80a7e99 > src/tests/external_tests.cpp 4b08f86 > src/tests/main.cpp ada489d > src/tests/sample_frameworks_tests.cpp 877fdf1 > src/tests/utils.hpp 65ce4df > src/tests/utils.cpp 553ab2d > src/tests/zookeeper_server.hpp cac9f0d > src/tests/zookeeper_server.cpp 3927785 > third_party/libprocess/include/process/protobuf.hpp 7636822 > > Diff: https://reviews.apache.org/r/2981/diff > > > Testing > ------- > > > Thanks, > > Charles > >
