----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24264/#review49824 -----------------------------------------------------------
Ship it! Cool. Mostly just a request for more comments so the next person that comes along understands what's happening can can easily amend or debug. Also, see my comment re: bundling pip and wheel. Maybe for a follow up on this review, but I wonder if we check for pip/wheel at configure time and only build wheels and install if we have wheel. At some point we'll want to unbundle these dependencies too. At the very least, assuming this makes sense, capturing this as a TODO and even a JIRA ticket would be great! mpi/mpiexec-mesos.in <https://reviews.apache.org/r/24264/#comment87188> Can we throw a comment above this block to explain what it is that you're doing? It's starting to get more and more complicated. src/Makefile.am <https://reviews.apache.org/r/24264/#comment87191> Can you please leave a comment that explains how you go from a concrete egg (e.g., python/dist/mesos.interface-0.20.0-py2.7.egg) to the directory that you need to 'cd' into in order to run setup.py? That will help folks understand the magic in 'cd python/`echo $@ | awk -F"mesos[.]?" '{print $$2}' | cut -d- -f1`'. Along with that comment, can you highlight that we build both eggs and wheels because we need to run the tests against the eggs (or in the event someone wants an egg) but we use wheels for installation because they can be uninstalled. src/Makefile.am <https://reviews.apache.org/r/24264/#comment87192> Just my ignorance here, but do you need to specify --dist-dir= twice? Once for bdist_egg and once for bdist_wheel? src/Makefile.am <https://reviews.apache.org/r/24264/#comment87190> Can we add a brief comment here explaining that rather than assuming 'pip' is installed we use the extracted 'pip' in 3rdparty, thus no call to 'pip' directly. Also, is there any reason not to require pip? I know Tim St Clair will ultimately want to "unbundle" this as he's doing for the rest of the 3rdparty dependencies. - Benjamin Hindman On Aug. 6, 2014, 10:35 p.m., Thomas Rampelberg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24264/ > ----------------------------------------------------------- > > (Updated Aug. 6, 2014, 10:35 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Bugs: MESOS-899 > https://issues.apache.org/jira/browse/MESOS-899 > > > Repository: mesos-git > > > Description > ------- > > Because eggs are impossible to uninstall, pip + wheel has been added as a > packaging format. The wheels are generated in addition to eggs and then > subsequently installed/uninstalled. > > Note that the building of eggs has not been removed. This is because wheels > cannot be run in place like eggs are via. modification of PYTHONPATH. The eggs > are needed for all the test scripts to run correctly without actually > installing anything locally. > > > Diffs > ----- > > 3rdparty/Makefile.am 70b45fe8b846a2a3fda599c0b5b7cfa5eb7e78e0 > 3rdparty/pip-1.5.6.tar.gz PRE-CREATION > 3rdparty/versions.am cd7c1cf087dbcfd385ac33145a562764e426c5e5 > 3rdparty/wheel-0.24.0.tar.gz PRE-CREATION > configure.ac 8fb0a3a794db4d3671243d06ff45232eae53c27b > mpi/mpiexec-mesos.in 8812ee28c1f845bc3de40ffbf9e9d18033e450f2 > src/Makefile.am 39af0365e429b8d08addadb09ee18080a19625f8 > src/examples/python/test-containerizer.in > f71828db98a90f455c88d90cb4e3320b7e8c9e9e > src/examples/python/test-executor.in > b22e7a7dc0c26f805eb63c7139066ce7dc830636 > src/examples/python/test-framework.in > 64fb1ddc1a0e5772c12d7497dfc1cf6ca2a7dceb > > Diff: https://reviews.apache.org/r/24264/diff/ > > > Testing > ------- > > `make distcheck -j6` > > > Thanks, > > Thomas Rampelberg > >