[
https://issues.apache.org/jira/browse/MESOS-58?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13170434#comment-13170434
]
[email protected] commented on MESOS-58:
----------------------------------------------------
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > Makefile.am, line 8
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61156#file61156line8>
bq. >
bq. > I know you didn't add these spaces, but feel free to remove them
here and everywhere else in this patch.
Done (in this file).
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > Makefile.am, line 51
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61156#file61156line51>
bq. >
bq. > s/make they're/make sure they're
Done.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > configure.ac, lines 44-59
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61162#file61162line44>
bq. >
bq. > Let's just kill these lines.
Done.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > configure.ac, lines 82-85
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61162#file61162line82>
bq. >
bq. > Kill.
Done.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > configure.ac, line 101
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61162#file61162line101>
bq. >
bq. > Regarding my comment, does autotools give us most of what we want
here?
$target_os already comes from AC_CANONICAL_TARGET.
configure won't set _XOPEN_SOURCE or the funky Solaris flags (which are clearly
wrong on Sparc anyways), and I don't understand why we're not always using
-D_XOPEN_SOURCE=700.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > configure.ac, line 157
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61162#file61162line157>
bq. >
bq. > Kill any extra spaces, here and everywhere else.
Done.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > mesos-slave-dev.in, line 19
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61168#file61168line19>
bq. >
bq. > s/master/slave
Done.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > mesos-slave-dev.in, line 21
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61168#file61168line21>
bq. >
bq. > s/master/slave
Done.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/Makefile.am, line 22
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61169#file61169line22>
bq. >
bq. > What is this for?
I originally tried to use suffix rules for protoc, for which this was
necessary. But I switched to pattern rules (which probably means no BSD make,
not that we care), because suffixes rules couldn't represent the two output
files.
Removed.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/Makefile.am, line 65
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61169#file61169line65>
bq. >
bq. > Can we do this?
No idea.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/Makefile.am, line 78
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61169#file61169line78>
bq. >
bq. > Newline.
Done.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/Makefile.am, line 107
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61169#file61169line107>
bq. >
bq. > The rest of the file indents.
Indented.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/Makefile.am, line 175
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61169#file61169line175>
bq. >
bq. > Why the extra variable? Why not just libmesos_la_LIBADD +=? Line
length?
This is an artifact from when I was considering building the (almost)
self-contained python egg in this Makefile.am.
Replaced with libmesos_la_LIBADD
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/Makefile.am, line 248
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61169#file61169line248>
bq. >
bq. > Wow, I didn't know MKDIR_P existed!!! Or $(@D) ... what does that
mean?
bq. From the GNU make manual. "The directory part of the file name of the
target, with the trailing slash removed". This won't work on non-GNU make, if
you care about that.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/common/process_utils.hpp, line 27
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61171#file61171line27>
bq. >
bq. > s/hardness/harness
Done.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/Makefile.am, line 451
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61169#file61169line451>
bq. >
bq. > Please wrap these lines for the 80 character freaks like me.
Done.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/common/webui_utils.cpp, line 18
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61173#file61173line18>
bq. >
bq. > Brace on newline.
Done.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/java/src/org/apache/mesos/MesosSchedulerDriver.java, line 46
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61199#file61199line46>
bq. >
bq. > Put throw on new line inside braces.
Done.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/java/src/org/apache/mesos/MesosSchedulerDriver.java, lines 47-49
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61199#file61199line47>
bq. >
bq. > ?
Removed.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/master/webui.cpp, line 71
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61205#file61205line71>
bq. >
bq. > This comment still applies, even though you moved the code.
Added to src/common/webui_utils.cpp
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/slave/webui.cpp, line 73
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61208#file61208line73>
bq. >
bq. > Let's call spawnWebui start as well, the namespace says it all:
utils::webui::start(...
Done.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/tests/external/LxcIsolation/run_scheduled_memhog_test.sh, line 16
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61214#file61214line16>
bq. >
bq. > 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!
external_tests.cpp should set the appropriate environment variables. It was
missing MESOS_WEBUI_DIR and MESOS_LAUNCHER_DIR, which I added. I don't think we
have anything which runs the LXC tests frequently.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/tests/external/LxcIsolation/run_scheduled_memhog_test.sh, line 50
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61214#file61214line50>
bq. >
bq. > Put the args on continued newlines.
Done.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/tests/external/LxcIsolation/HoldMoreMemThanRequested.sh, line 18
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61211#file61211line18>
bq. >
bq. > Cool, so are we no longer copying these in the Makefile.am?
Except for the Java framework related scripts.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/tests/external/SampleFrameworks/JavaFramework.sh, line 7
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61220#file61220line7>
bq. >
bq. > Why is this one in BUILD, but some others are in SOURCE, like the
next one is in SOURCE.
The Python framework doesn't need any compilation, so it's run out of the
source directory. (If this generates .pyc files or similar, we should
reconsider this.)
I didn't go to the effort of modifying the Java framework scripts to handle
keeping the scripts in the source directory but finding the Java compiled code
in the build directory.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > src/tests/main.cpp, line 57
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61223#file61223line57>
bq. >
bq. > s/ROOT_DIR/SOURCE_DIR ?? Pretty please?
Done.
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > Makefile.am, line 53
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61156#file61156line53>
bq. >
bq. > Where does this variable come from?
It's in the generated Makefile; thought it was a documented automake variable,
but apparently not. Replaced with a listing of generated non-Makefile files in
src/
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > configure.ac, line 97
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61162#file61162line97>
bq. >
bq. > Can we (do we) test for python before we just invoke it?
Except for --with-webui, we do. (See AM_PATH_PYTHON below.)
bq. On 2011-12-15 01:01:48, Benjamin Hindman wrote:
bq. > mesos-build-env.sh.in, line 1
bq. > <https://reviews.apache.org/r/2981/diff/2/?file=61166#file61166line1>
bq. >
bq. > 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.
Using relative paths only works if you know the current working directory when
the artifacts are run, which you won't get for free.
We could use the executable path to do detect installed versus uninstalled
binaries, but this would require different code for essentially every platform
(unless we want to trust argv[0] + PATH resolution).
http://stackoverflow.com/questions/1023306/finding-current-executables-path-without-proc-self-exe
gives ways of an executable path. We'd need to look at carefully about what we
want to do when the path involves symbolic links or when the executable is in
neither the build directory nor the install location.
Ideally, we could find an elegant way of hooking libtool's wrapper scripts (on
platforms where they exist) to set the relevant environment variables.
- Charles
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2981/#review3611
-----------------------------------------------------------
On 2011-12-15 19:28:39, Charles Reiss wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2981/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-12-15 19:28:39)
bq.
bq.
bq. Review request for mesos, Benjamin Hindman and Andy Konwinski.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. 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).
bq.
bq. Notable changes:
bq. - Builds from source control now require configure, automake, libtool to
be installed; added ./bootstrap to create configure, Makefile.ins, etc.
(autoreconf will _not_ work.)
bq. - Install location are no longer under $(prefix)/mesos; instead, we follow
the configure-set directories mostly, so
bq. * mesos-master, mesos-slave go into $(sbindir) which defaults to
$(prefix)/sbin
bq. * mesos-local, mesos-log, mesos-mesos (framework submission) go into
$(bindir) which defaults to $(prefix)/bin
bq. * mesos-launcher (utility program for LXC isolation), killtree.sh
(utility program) go into $(pkglibexecdir) which defaults to
$(prefix)/libexec/mesos
bq. * mesos.jar goes into $(libdir)/java which defaults to $(prefix)/lib/java
bq. * libmesos.{la,so,dylib} goes into $(libdir) which defaults to
$(prefix)/lib
bq. * webui python scripts go into $(pkgdatadir) which defaults to
$(prefix)/share/mesos
bq. - New configuration options for finding install locations, with defaults
compiled in based on configured install location:
bq. * webui_dir
bq. * launcher_dir
bq. - Created wrapper scripts mesos-master-dev and mesos-slave-dev that set
configuration environment variables suitable for running Mesos out of build
directory
bq. - Tests find location of external files using MESOS_BUILD_DIR and
MESOS_SOURCE_DIR environment variables set by test harness
bq. - libmesos_exec.a, libmesos_sched.a have been eliminated
bq. - configure autodetects Java by default (it can still be disabled)
bq. - Removes support for building without zookeeper?
bq. - 'make test' changes names to 'make check'.
bq.
bq. Notable things not done:
bq. - 'make dist' is broken, primarily because of third_party.
bq. - 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.
bq. - 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.
bq.
bq.
bq. This addresses bug MESOS-58.
bq. https://issues.apache.org/jira/browse/MESOS-58
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. Makefile.am PRE-CREATION
bq. Makefile.in e8dec7a
bq. bootstrap PRE-CREATION
bq. config.guess f32079a
bq. config.sub 6759825
bq. configure fc6bf00
bq. configure.ac f6b6a01
bq. include/mesos/mesos.hpp PRE-CREATION
bq. install-sh 4fbbae7
bq. m4/dummy_m4_contents PRE-CREATION
bq. mesos-build-env.sh.in PRE-CREATION
bq. mesos-master-dev.in PRE-CREATION
bq. mesos-slave-dev.in PRE-CREATION
bq. src/Makefile.am PRE-CREATION
bq. src/Makefile.in 516f128
bq. src/common/process_utils.hpp 8e55d76
bq. src/common/webui_utils.hpp PRE-CREATION
bq. src/common/webui_utils.cpp PRE-CREATION
bq. src/config/config.hpp.in 49f666a
bq. src/detector/detector.cpp ef4aff7
bq. src/examples/cpp_test_executor.cpp 9dd244e
bq. src/examples/cpp_test_framework.cpp ab02805
bq. src/examples/java/Makefile.in 6500ded
bq. src/examples/java/TestExceptionFramework.java ba39757
bq. src/examples/java/TestFramework.java e1b6032
bq. src/examples/java/TestMultipleExecutorsFramework.java a49ecbb
bq. src/examples/java/test-exception-framework PRE-CREATION
bq. src/examples/java/test-executor PRE-CREATION
bq. src/examples/java/test-framework PRE-CREATION
bq. src/examples/java/test-multiple-executors-framework PRE-CREATION
bq. src/examples/java/test_exception_framework be78062
bq. src/examples/java/test_executor d2fbb05
bq. src/examples/java/test_framework 2713b34
bq. src/examples/java/test_multiple_executors_framework 9fad319
bq. src/examples/python/test-executor PRE-CREATION
bq. src/examples/python/test-framework PRE-CREATION
bq. src/examples/python/test_executor 8d6cc64
bq. src/examples/python/test_framework 050ab90
bq. src/examples/python/test_framework.py e6893cf
bq. src/examples/test_executor.cpp PRE-CREATION
bq. src/examples/test_framework.cpp PRE-CREATION
bq. src/java/mesos.pom e5b674d
bq. src/java/mesos.pom.in PRE-CREATION
bq. src/java/src/org/apache/mesos/MesosSchedulerDriver.java 04809a6
bq. src/jvm/jvm.hpp PRE-CREATION
bq. src/jvm/jvm.cpp PRE-CREATION
bq. src/log/log.hpp e50969b
bq. src/log/network.hpp c88822c
bq. src/master/slaves_manager.cpp 3f67aa1
bq. src/master/webui.cpp c5a20d1
bq. src/python/setup.py.in 7c1f6e5
bq. src/slave/lxc_isolation_module.cpp ab0843a
bq. src/slave/webui.cpp bb1c780
bq. src/tests/base_zookeeper_test.hpp e7bb7ed
bq. src/tests/base_zookeeper_test.cpp 202b66b
bq. src/tests/external/LxcIsolation/HoldMoreMemThanRequested.sh 7b87a7b
bq. src/tests/external/LxcIsolation/ScaleUpAndDown.sh 09333e2
bq. src/tests/external/LxcIsolation/TwoSeparateTasks.sh a2e2883
bq. src/tests/external/LxcIsolation/run_scheduled_memhog_test.sh 9ac7cd5
bq. src/tests/external/SampleFrameworks/CFrameworkCmdlineParsing.sh 791cbf1
bq. src/tests/external/SampleFrameworks/CFrameworkInvalidCmdline.sh 3197c81
bq. src/tests/external/SampleFrameworks/CFrameworkInvalidEnv.sh 427b33b
bq. src/tests/external/SampleFrameworks/CppFramework.sh 968d951
bq. src/tests/external/SampleFrameworks/JavaExceptionFramework.sh bc4a4e0
bq. src/tests/external/SampleFrameworks/JavaFramework.sh db88d0b
bq. src/tests/external/SampleFrameworks/PythonFramework.sh 80a7e99
bq. src/tests/external_tests.cpp 4b08f86
bq. src/tests/main.cpp ada489d
bq. src/tests/sample_frameworks_tests.cpp 877fdf1
bq. src/tests/utils.hpp 65ce4df
bq. src/tests/utils.cpp 553ab2d
bq. src/tests/zookeeper_server.hpp cac9f0d
bq. src/tests/zookeeper_server.cpp 3927785
bq. third_party/libprocess/include/process/protobuf.hpp 7636822
bq.
bq. Diff: https://reviews.apache.org/r/2981/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Charles
bq.
bq.
> Migrate build system to Autotools (Automake and Libtool)
> --------------------------------------------------------
>
> Key: MESOS-58
> URL: https://issues.apache.org/jira/browse/MESOS-58
> Project: Mesos
> Issue Type: Improvement
> Components: build
> Reporter: Andy Konwinski
> Assignee: Benjamin Hindman
>
> Ben has been leading the effort to move the build system to Autotools. This
> should lead to better dependency management, cleaner and easier to understand
> human written build input, and a faster build.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
