Do not traverse the dependees of regexp excluded targets in ChangedCalculator.

Review Request #3878 — Created May 12, 2016 and updated

patricklaw
pants
3422
pants-reviews
jsirois, kwlzn, mateor, zundel
Suppose that A depends on B.  If B's source is changed and we call
`./pants test-changed --include-dependees=direct`, then both
B and A will be tested.  If we exclude B using `exclude_target_regexp`,
A is still tested even though the only path to it via changed targets
is through B.

In all circumstances where a user excludes a target specifically for
test-changed, they probably intend to also exclude dependees that
would have been brought in by the excluded target.  This patch
changes the behavior of ChangeCalculator to match that
expectation.

CI is green: https://travis-ci.org/pantsbuild/pants/builds/129817416

Added a unit test to exercise this specific corner case.

MA
  1. Ship It!
  2. 
      
PA
Review request changed

Testing Done:

~  

CIs are baking:

  ~

CI is green: https://travis-ci.org/pantsbuild/pants/builds/129817416

-   http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/branch/PR-3422/1/flowGraphTable/
-   https://travis-ci.org/pantsbuild/pants/builds/129817416

   
   

Added a unit test to exercise this specific corner case.

ZU
  1. 
      
  2. FYI @cheister

    I see this is somewhat existing behavior, but I was not aware of this behavior here before.

    First of all I don't understand the premise of this logic. If A depends on B and B is excluded, how do you work at all with this target? Just pretend that B was never defined in the build graph? That is not the way we originally intended --exclude-target-regexp to work.

    In our build we have this set in our pants.ini:

    exclude_target_regexp: [
        ":aux-",
        ...
      ]
    

    --exclude-target-regexp=aux-*

    1) These aux targets are generated (We are at the cusp (oh so close!) of getting rid of our generation logic),

    2) Our expectation is that these targets aren't to be used unless referenced explicitly by another target. What's happening here is that these aux-targets have a bunch of java_libraries with deps computed by pom.xml files that can be referenced from hand-written files if needed - kind of sort of like buildgen, but not really.

    The behavior we expect is for --exclude-target-regexp to influence the command line spec logic, not elsewhere. The way this works that if you use a wildcard, the targets are excluded from the wildcard expansion before being passed into the list of target roots. But if a target transitively pulls in a aux- target, we expect it to be computed.

    So, by our logic, if A depends on B and B was excluded, its just the opposite: B and A are both valid.

  3. 
      
Loading...