Extract `python-eval` and `pythonstyle` to plugin.

Review Request #3114 — Created Nov. 13, 2015 and submitted

jsirois
pants
jsirois/pycheck/plugin
2554
pants-reviews
kwlzn, molsen, patricklaw, stuhood
This moves these two tasks to a python checks plugin in contrib and adds
registration in the 'compile' goal to enable easy install and activation
of these extra checks using the `plugins` mechanism; ie a pure
`pants.ini` activation.

To enable use of individual checks but not all, a `--skip` is added to
`python-eval`.

 contrib/python/README.md                                                                                                                            |  4 ++++
 contrib/python/src/python/pants/__init__.py                                                                                                         |  1 +
 contrib/python/src/python/pants/contrib/__init__.py                                                                                                 |  1 +
 contrib/python/src/python/pants/contrib/python/__init__.py                                                                                          |  1 +
 contrib/python/src/python/pants/contrib/python/checks/BUILD                                                                                         | 14 ++++++++++++++
 {tests/python/pants_test/backend/python/tasks/checkstyle => contrib/python/src/python/pants/contrib/python/checks}/__init__.py                      |  0
 {pants-plugins/src/python/internal_backend/optional => contrib/python/src/python/pants/contrib/python/checks}/register.py                           |  9 +++------
 contrib/python/src/python/pants/contrib/python/checks/tasks/BUILD                                                                                   | 22 ++++++++++++++++++++++
 {src/python/pants/backend/python/tasks/checkstyle => contrib/python/src/python/pants/contrib/python/checks/tasks}/__init__.py                       |  0
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/BUILD                                   |  8 ++++----
 {tests/python/pants_test/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/__init__.py                      |  0
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/checker.py                              |  7 ++++---
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/class_factoring.py                      |  2 +-
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/class_factoring_subsystem.py            |  4 ++--
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/common.py                               |  0
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/except_statements.py                    |  3 ++-
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/except_statements_subsystem.py          |  4 ++--
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/file_excluder.py                        |  0
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/future_compatibility.py                 |  2 +-
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/future_compatibility_subsystem.py       |  4 ++--
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/import_order.py                         |  2 +-
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/import_order_subsystem.py               |  4 ++--
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/indentation.py                          |  2 +-
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/indentation_subsystem.py                |  4 ++--
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/missing_contextmanager.py               |  2 +-
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/missing_contextmanager_subsystem.py     |  4 ++--
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/new_style_classes.py                    |  2 +-
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/new_style_classes_subsystem.py          |  4 ++--
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/newlines.py                             |  2 +-
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/newlines_subsystem.py                   |  4 ++--
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/pep8.py                                 |  2 +-
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/pep8_subsystem.py                       |  5 +++--
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/plugin_subsystem_base.py                |  0
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/print_statements.py                     |  2 +-
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/print_statements_subsystem.py           |  4 ++--
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/pyflakes.py                             |  2 +-
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/pyflakes_subsystem.py                   |  4 ++--
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/register_plugins.py                     | 28 +++++++++++++++-------------
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/trailing_whitespace.py                  |  2 +-
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/trailing_whitespace_subsystem.py        |  4 ++--
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/variable_names.py                       |  2 +-
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/checkstyle/variable_names_subsystem.py             |  4 ++--
 {src/python/pants/backend/python => contrib/python/src/python/pants/contrib/python/checks}/tasks/python_eval.py                                     |  5 +++++
 contrib/python/src/python/pants/contrib/python/checks/tasks/templates/python_eval/eval.py.mustache                                                  | 45 +++++++++++++++++++++++++++++++++++++++++++++
 contrib/python/tests/python/pants_test/__init__.py                                                                                                  |  1 +
 contrib/python/tests/python/pants_test/contrib/__init__.py                                                                                          |  1 +
 contrib/python/tests/python/pants_test/contrib/python/__init__.py                                                                                   |  1 +
 {tests/python/pants_test/backend/python/tasks/checkstyle => contrib/python/tests/python/pants_test/contrib/python/checks}/__init__.py               |  0
 contrib/python/tests/python/pants_test/contrib/python/checks/tasks/BUILD                                                                            | 11 +++++++++++
 {tests/python/pants_test/backend/python/tasks/checkstyle => contrib/python/tests/python/pants_test/contrib/python/checks/tasks}/__init__.py         |  0
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/BUILD                     | 19 ++++++-------------
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/__init__.py               |  0
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/plugin_test_base.py       |  3 ++-
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/test_class_factoring.py   |  8 +++++---
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/test_common.py            |  5 +++--
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/test_except_statements.py |  6 ++++--
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/test_file_excluder.py     |  3 ++-
 .../python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/test_future_compatibility.py |  8 +++++---
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/test_import_order.py      |  8 +++++---
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/test_indentation.py       |  6 ++++--
 .../pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/test_missing_contextmanager.py      |  9 ++++++---
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/test_new_style_classes.py |  6 ++++--
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/test_newlines.py          |  6 ++++--
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/test_noqa.py              |  8 ++++----
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/test_print_statements.py  |  6 ++++--
 .../python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/test_trailing_whitespace.py  |  6 ++++--
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/checkstyle/test_variable_names.py    | 17 ++++++++++-------
 {tests/python/pants_test/backend/python => contrib/python/tests/python/pants_test/contrib/python/checks}/tasks/test_python_eval.py                  |  9 ++++++++-
 contrib/release_packages.sh                                                                                                                         | 15 +++++++++++++++
 pants-plugins/src/python/internal_backend/optional/BUILD                                                                                            |  2 --
 pants-plugins/src/python/internal_backend/optional/register.py                                                                                      |  4 ----
 pants.ini                                                                                                                                           |  2 ++
 src/python/pants/backend/python/tasks/BUILD                                                                                                         |  1 -
 73 files changed, 271 insertions(+), 125 deletions(-)

Locally ran ./pants test contrib/python/:: and
./build-support/bin/release.sh -n successfully.

CI went green here:
https://travis-ci.org/pantsbuild/pants/builds/90969899

JS
JS
JS
  1. OK - Ready for real now.
  2. 
      
JS
JS
JS
PA
  1. 
      
  2. StringIO has been backported to the standard py3k location (io) for python27, we don't need to use six for it if we don't want to. I'd prefer to stick with stdlib where we're guaranteed it exists.

    1. We traded nice to knows.  Fixed up.
    2. Reverting that since:

          return tokenize.generate_tokens(io.StringIO(blob).readline)
      Exception message: initial_value must be unicode or None, not str
      

      And the file already uses six for other things.

    3. Ah yeah, this should probably be BytesIO, or this whole module should be more careful about reifying bytes -> code points at the barriers. But not a blocker for me.

  3. Nice, I wish I'd known about this in other projects.

  4. 
      
JS
JS
JS
JS
  1. I'm going to submit based on Patrick's review in ~30 minutes to prepare for the `0.0.58` release.  More eyes are welcome though before then.
  2. 
      
KW
  1. lgtm afaict!

  2. 
      
JS
  1. I just tested this out in the Medium/mono repo using locally published sdists and the new plugin does the trick, allows eliminating their custom plugin enabling python-eval and pythonstyle.
    1. And ditto for Aurora.  There I turned off `python-eval` successfully with a pants.ini tweak (they have some python code with side-effects upon import).
  2. 
      
MO
  1. LGTM, thanks for cleaning up some of the twitter.common stuff while you were working on this.

  2. 
      
JS
Review request changed

Status: Closed (submitted)

Change Summary:

Thanks folks - submitted @ https://github.com/pantsbuild/pants/commit/51c48026691b08d12e6b003ebd3aecaee219b1b2
Loading...