Fix python_requirements to tie PRLs to req files.

Review Request #2397 — Created June 19, 2015 and discarded

jsirois
pants
jsirois/issues/1583
1583, 1713
pants-reviews
benjyw, chrischen, dturner-tw
Previously the python_requirements would expand a python requirements
file to a list of PythonRequirementLibrary instances that were each
divorced from their python requirements file source.  This change adds a
private PythonRequirmentLibrary constructor parameter (avoids
documentation in the BUILD dictionary) that python_requirements uses to
associate the requirements file as a sources file that each
PythonRequirmentLibrary generated from it owns.  This makes goals like
`filemap`, `filedeps` and `changed` work as expected.

 src/python/pants/backend/python/python_requirements.py                                                                 |  1 +
 src/python/pants/backend/python/targets/python_requirement_library.py                                                  | 18 +++++++++++++-----
 tests/python/pants_test/backend/python/BUILD                                                                           | 13 +------------
 tests/python/pants_test/backend/python/targets/BUILD                                                                   | 20 ++++++++++++++++++++
 tests/python/pants_test/backend/python/targets/__init__.py                                                             |  0
 tests/python/pants_test/backend/python/{test_python_requirement_list.py => targets/test_python_requirement_library.py} |  2 +-
 6 files changed, 36 insertions(+), 18 deletions(-)
CI went green here:
  https://travis-ci.org/pantsbuild/pants/builds/67575915
BE
  1. 
      
  2. So every requirement will have the same requirements.txt file as a source. That seems like a step backwards, since we'd prefer a world where targets don't overlap, and here we're requiring it.

    Not for this change, but maybe python_requirements should be a full target, not a macro?

    1. I'm not sure it is a step backwards.  Its true we don't want multiple targets to one source when those sources are acted on by tasks - but this is different, the source files are acted on by the BuildFileParser and generate targets.
      For this same reason python_requirements can't be a target afaict - since it expands one to many.  A macro seems exactly appropriate.
  3. 
      
JS
CH
  1. John, thanks for fixing this!

  2. 
      
JS
  1. I'm going to take a second swing at this that avoids treating requirements files as source payloads.  The concept of a target's provenance already weakly exists (is_synthetic, is_original, derived_from, ...) and tightened up a bit, that information could also be used to track (potential) changes in a target definition.  I won't close this RB, but I'll link to the new one and then after folks take a look decide what to do with this RB.
  2. 
      
JS
Review request changed

Status: Discarded

Loading...