Posted https://reviews.apache.org/r/36972/ for feedback.
On Thu, Jul 30, 2015 at 5:17 PM, Bill Farner <[email protected]> wrote: > +1 > > -=Bill > > On Thu, Jul 30, 2015 at 4:24 PM, Kevin Sweeney > <[email protected] > > wrote: > > > Somewhat complementary work - this change would make the generated > packages > > equivalent to the ultimate goal of replacing these top-level BUILD files > > directly with setup.py files. Users could begin testing deployment with > the > > pants-generated sdists and seamlessly drop in setup.py-generated > > replacements. Users wishing to deploy via PEX could use the pex tool on > the > > generated sdists rather than the pants pex plugin. > > > > On Thu, Jul 30, 2015 at 4:13 PM, Bill Farner <[email protected]> wrote: > > > > > We've talked in the past about switching build tools. Just to keep > that > > in > > > context - how would you weigh this effort against a tool change? > > > > > > -=Bill > > > > > > On Thu, Jul 30, 2015 at 4:00 PM, Kevin Sweeney <[email protected]> > > wrote: > > > > > > > I propose a simplification of the Python BUILD layout as follows and > a > > > set > > > > of new conventions, as follows: > > > > > > > > 1) 1 BUILD per 3rd level directory. These are currently > > > > > > > > ``` > > > > % find src/main/python -maxdepth 3 -mindepth 3 -type d |while read > > > dirname; > > > > do echo $dirname | sed 's@src > > > /main/python/\(.*\)/\(.*\)/\(.*\).*@\1.\2.\3@ > > > > '; > > > > done > > > > apache.aurora.client > > > > apache.aurora.common > > > > apache.aurora.tools > > > > apache.aurora.admin > > > > apache.aurora.executor > > > > apache.aurora.config > > > > apache.thermos.monitoring > > > > apache.thermos.common > > > > apache.thermos.cli > > > > apache.thermos.testing > > > > apache.thermos.core > > > > apache.thermos.runner > > > > apache.thermos.observer > > > > apache.thermos.config > > > > ``` > > > > > > > > 2) Each BUILD file exports 1 python_library that provides a setup_py > > > > containing each python_binary in the BUILD file, named the same as > the > > > > directory it's in so that it can be referenced without a ':' > character. > > > The > > > > sources field in the python_binary will always be rglobs('*.py'). > > > > > > > > 3) Other BUILD files may only depend on this single public > > python_library > > > > target. Any other target is considered a private implementation > detail > > > and > > > > should be prefixed with an _. > > > > > > > > 4) python_binary targets are always named the same as the exported > > > console > > > > script. > > > > > > > > 5) python_binary targets must have identical dependencies to the > > > > python_library exported by the package and must use entry_point. > > > > > > > > The advantage of this change is that a PEX file generated by pants > will > > > > contain exactly the same files that will be available on the > PYTHONPATH > > > in > > > > the case of pip-installation of the corresponding library target. > This > > > will > > > > help our migration off pants in the future. > > > > > > > > Annotated example: apache.thermos.runner (renamed thermos/bin -> > > > > thermos/runner) > > > > > > > > ``` > > > > % find src/main/python/apache/thermos/runner > > > > src/main/python/apache/thermos/runner > > > > src/main/python/apache/thermos/runner/__init__.py > > > > src/main/python/apache/thermos/runner/thermos_runner.py > > > > src/main/python/apache/thermos/runner/BUILD > > > > % cat src/main/python/apache/thermos/runner/BUILD > > > > # License boilerplate omitted > > > > import os > > > > > > > > > > > > # Private target so that a setup_py can exist without a circular > > > > dependency. Only targets within this file should depend on > > > > # this. > > > > python_library( > > > > name = '_runner', > > > > # The target covers every python file under this directory and > > > > subdirectories. > > > > sources = rglobs('*.py'), > > > > dependencies = [ > > > > '3rdparty/python:twitter.common.app', > > > > '3rdparty/python:twitter.common.log', > > > > # Source dependencies are always referenced without a ':'. > > > > 'src/main/python/apache/thermos/common', > > > > 'src/main/python/apache/thermos/config', > > > > 'src/main/python/apache/thermos/core', > > > > ], > > > > ) > > > > > > > > # Binary target for thermos_runner.pex. Nothing should depend on > this - > > > > it's only used as an argument to ./pants binary. > > > > python_binary( > > > > name = 'thermos_runner', > > > > # Use entry_point, not source so the files used here are the same > > ones > > > > tests see. > > > > entry_point = 'apache.thermos.bin.thermos_runner', > > > > dependencies = [ > > > > # Notice that we depend only on the single private target from > this > > > > BUILD file here. > > > > ':_runner', > > > > ], > > > > ) > > > > > > > > # The public library that everyone importing the runner symbols uses. > > > > # The test targets and any other dependent source code should depend > on > > > > this. > > > > python_library( > > > > name = 'runner', > > > > dependencies = [ > > > > # Again, notice that we depend only on the single private target > > from > > > > this BUILD file here. > > > > ':_runner', > > > > ], > > > > # We always provide a setup_py. This will cause any dependee > > libraries > > > to > > > > automatically reference this library > > > > # in their requirements.txt rather than copy the source files into > > > their > > > > sdist. > > > > provides = setup_py( > > > > # Conventionally named and versioned. > > > > name = 'apache.thermos.runner', > > > > version = open(os.path.join(get_buildroot(), > > > > '.auroraversion')).read().strip().upper(), > > > > ).with_binaries({ > > > > # Every binary in this file should also be repeated here. > > > > # Always use the dict-form of .with_binaries so that commands > with > > > > dashes in their names are supported. > > > > # The console script name is always the same as the PEX with .pex > > > > stripped. > > > > 'thermos_runner': ':thermos_runner', > > > > }), > > > > ) > > > > ``` > > > > > > > > Let me know what you think, if y'all agree I'll prepare a patch > > shortly. > > > > > > > > > > > > > > > -- > > Kevin Sweeney > > @kts > > >
