Move BuildFileAliases validation to BuildFileAliases.

Review Request #2790 - Created Sept. 10, 2015 and submitted

Information
John Sirois
pants
jsirois/issues/2124
2124, 2160
2796
974a179...
Reviewers
pants-reviews
davidt, gmalmquist, kwlzn, nhoward_tw, patricklaw
Previously, alias mappings were validated by BuildConfiguration.  Doing
the validation in BuilfFileAliases ensures only valid objects are
constructed and never float around in an invalid state and it
centralizes the mostly internal detail that "targets" contains a mix of
types, allowing the mix to be split and presented via seperate
properties.

This adds TargetMacro.Factory coverage to BuildConfigurationTest while
moving its validation tests over to BuilfFileAliasesTest.

The change also forces existing users of BuilfFileAliases.targets to
deal with BuilfFileAliases.target_types and
BuilfFileAliases.target_macro_factories.  To help those users a few
utility methods are added to BuildFileAliases to unify target types
accessed via these two properties.

 contrib/cpp/src/python/pants/contrib/cpp/register.py                                |   2 +-
 contrib/go/src/python/pants/contrib/go/register.py                                  |   2 +-
 contrib/node/src/python/pants/contrib/node/register.py                              |   2 +-
 contrib/scrooge/tests/python/pants_test/contrib/scrooge/tasks/test_scrooge_gen.py   |   2 +-
 contrib/scrooge/tests/python/pants_test/contrib/scrooge/tasks/test_thrift_linter.py |   2 +-
 contrib/spindle/src/python/pants/contrib/spindle/register.py                        |   2 +-
 contrib/spindle/tests/python/pants_test/contrib/spindle/tasks/test_spindle_gen.py   |   2 +-
 examples/src/java/org/pantsbuild/example/page.md                                    |   2 +-
 pants-plugins/src/python/internal_backend/repositories/register.py                  |   2 +-
 pants-plugins/src/python/internal_backend/utilities/register.py                     |   2 +-
 src/python/pants/backend/android/register.py                                        |   2 +-
 src/python/pants/backend/authentication/register.py                                 |   2 +-
 src/python/pants/backend/codegen/register.py                                        |   2 +-
 src/python/pants/backend/core/register.py                                           |   2 +-
 src/python/pants/backend/core/tasks/BUILD                                           |   5 +-
 src/python/pants/backend/core/tasks/dependees.py                                    |  56 ++++++++------
 src/python/pants/backend/core/tasks/filter.py                                       |  22 ++----
 src/python/pants/backend/core/tasks/reflect.py                                      |   9 ++-
 src/python/pants/backend/jvm/register.py                                            |   2 +-
 src/python/pants/backend/maven_layout/register.py                                   |   2 +-
 src/python/pants/backend/project_info/tasks/BUILD                                   |   5 +-
 src/python/pants/backend/project_info/tasks/export.py                               |  16 ++--
 src/python/pants/backend/python/register.py                                         |   2 +-
 src/python/pants/base/BUILD                                                         |   3 +
 src/python/pants/base/build_configuration.py                                        |  55 +++-----------
 src/python/pants/base/build_file_aliases.py                                         | 198 ++++++++++++++++++++++++++++++++++++++++++++++----
 src/python/pants/docs/howto_plugin.md                                               |   2 +-
 tests/python/pants_test/android/tasks/test_unpack_libraries.py                      |   2 +-
 tests/python/pants_test/backend/codegen/targets/test_java_antlr_library.py          |   2 +-
 tests/python/pants_test/backend/codegen/targets/test_java_protobuf_library.py       |   6 +-
 tests/python/pants_test/backend/codegen/tasks/test_antlr_gen.py                     |   2 +-
 tests/python/pants_test/backend/codegen/tasks/test_simple_codegen_task.py           |   2 +-
 tests/python/pants_test/backend/core/test_prep.py                                   |   2 +-
 tests/python/pants_test/backend/core/test_wrapped_globs.py                          |   2 +-
 tests/python/pants_test/backend/jvm/targets/test_jar_library.py                     |   4 +-
 tests/python/pants_test/backend/jvm/tasks/test_ivy_imports.py                       |   2 +-
 tests/python/pants_test/backend/jvm/tasks/test_junit_run.py                         |   2 +-
 tests/python/pants_test/backend/jvm/tasks/test_unpack_jars.py                       |   2 +-
 tests/python/pants_test/backend/python/tasks/test_python_repl.py                    |   2 +-
 tests/python/pants_test/backend/python/test_python_requirement_list.py              |   2 +-
 tests/python/pants_test/base/BUILD                                                  |   4 +-
 tests/python/pants_test/base/test_build_configuration.py                            |  80 ++++++++++++++------
 tests/python/pants_test/base/test_build_file_aliases.py                             |  70 +++++++++++-------
 tests/python/pants_test/base/test_build_file_parser.py                              |   6 +-
 tests/python/pants_test/base/test_cmd_line_spec_parser.py                           |   2 +-
 tests/python/pants_test/base/test_extension_loader.py                               |  21 +++---
 tests/python/pants_test/base/test_payload.py                                        |   2 +-
 tests/python/pants_test/base/test_source_mapper.py                                  |   2 +-
 tests/python/pants_test/base_test.py                                                |   2 +-
 tests/python/pants_test/jvm/jvm_tool_task_test_base.py                              |   2 +-
 tests/python/pants_test/targets/test_java_agent.py                                  |   2 +-
 tests/python/pants_test/targets/test_scala_library.py                               |   2 +-
 tests/python/pants_test/targets/test_wiki_page.py                                   |   2 +-
 tests/python/pants_test/tasks/test_check_published_deps.py                          |   2 +-
 tests/python/pants_test/tasks/test_dependees.py                                     |   2 +-
 tests/python/pants_test/tasks/test_filemap.py                                       |   2 +-
 tests/python/pants_test/tasks/test_filter.py                                        |   8 +-
 tests/python/pants_test/tasks/test_jar_create.py                                    |   2 +-
 tests/python/pants_test/tasks/test_jar_publish.py                                   |   2 +-
 tests/python/pants_test/tasks/test_jar_task.py                                      |   2 +-
 tests/python/pants_test/tasks/test_listtargets.py                                   |   2 +-
 tests/python/pants_test/tasks/test_minimal_cover.py                                 |   2 +-
 tests/python/pants_test/tasks/test_sorttargets.py                                   |   2 +-
 tests/python/pants_test/tasks/test_what_changed.py                                  |   2 +-
 tests/python/pants_test/test_maven_layout.py                                        |   2 +-
 65 files changed, 434 insertions(+), 230 deletions(-)
CI went green here:
  https://travis-ci.org/pantsbuild/pants/builds/79719489

Issues

  • 0
  • 2
  • 0
  • 2
Description From Last Updated
John Sirois
John Sirois
John Sirois
Kris Wilson
John Sirois
John Sirois
John Sirois
John Sirois
Review request changed

Status: Closed (submitted)

Nick Howard (Twitter)

   
  1. Just missed you by 5 minutes, I'll circle back with an RB to address all your points.
  2. Fixed as noted below over here: https://rbcommons.com/s/twitter/r/2796/

I think that the error here is still that the passed type argument is invalid. Since an invalid target type won't have any targets that match, the message is correct, but I feel like it's misleading.

How about normalizing these to Not a target type: {}?

  1. That's better wording, changed.

Same comment wrt error. Maybe we should extract this into a function?

  1. Extracted a mixin base to centralize.

nit: !r the alias for consistency.

  1. Fixed

nit: typo s/alias o sev/alias to sev/

  1. Fixed

Could you add whitespace here to make it easier to see where the exercise vs verify stanzas start/end?

  1. Fixed
Loading...