Switch all internal code to use the new source roots mechanism.

Review Request #2987 — Created Oct. 17, 2015 and submitted

benjyw
pants
6ad7191...
pants-reviews
jsirois, stuhood

In particular, have tests use standard source root names that
match the default patterns, so we don't need to explicitly register
source roots in tests.

Required a couple of hacks to set up the SourceRootConfig
subsystem in a handful of tests that don't use BaseTest.context().
We really need to find a more straightforward way to set up tests.
Hopefully the new engine will remove the need for so much implicit
global state.

The changes are mostly mundane, but there a few files to pay particular
attention to:

- source_root.py
- context.py
- base_test.py
- go_buildgen.py (and other go-related files)
- maven_layout.py

CI passes (after many trial-and-error iterations): https://travis-ci.org/pantsbuild/pants/builds/85874775

CI still passes after code review iteration: https://travis-ci.org/pantsbuild/pants/builds/86257544.

  • 0
  • 0
  • 1
  • 1
  • 2
Description From Last Updated
BE
BE
ST
  1. Awesome! Thanks so much Benjy. Folks will love removing this boilerplate.

  2. src/python/pants/build_graph/target.py (Diff revision 1)
     
     
     
     
     
     
     
     
     

    IIRC, the previous behaviour was to automatically create a source root at the location of the target... might make it easier to track down errors if this threw a good error message instead:

    "Target {t} is not located under a configured source root pattern: {configured_patterns}"

    1. Good idea. Done. This may cause some breakages, but that's probably for the best, as the previous behavior made no sense.

      Will run another CI to ensure that this at least doesn't break pantsbuild.

    2. It's certain to break us in a few places, but then again, so would the None behaviour.

  3. src/python/pants/source/source_root.py (Diff revision 1)
     
     

    Would be good to make as many of these options append=True as possible... that way people wouldn't have to redefine the default roots in order to add their own.

    I think that would require append=True support for dict?

    1. I think append=True would turn the type into a list of dicts, and we can then merge them. The same is true for lists, actually.

      I'll deal with this in a followup change?

    2. +1

    3. Actually, on further reflection I'm not sure we want it to be "append", because that would leave us with no way to remove a default pattern.

      In practice almost all shops will have a single relevant pattern, or maybe two, so I don't think this is onerous. They wouldn't need to repeat all the defaults.

  4. 
      
BE
BE
BE
BE
  1. Thanks Stu! Submitted as 570690f2974a698afee90026f4275da375cb5d4e.

  2. 
      
BE
Review request changed

Status: Closed (submitted)

Change Summary:

570690f2974a698afee90026f4275da375cb5d4e

Loading...