Only add resources to the relevant target

Review Request #2103 - Created April 21, 2015 and submitted

Information
Stu Hood
pants
91e0b6b...
Reviewers
pants-reviews
benjyw, jinfeng, nhoward_tw, patricklaw, zundel

PrepareResources was adding a toposorted list of entries to the classpath for ALL targets, rather than the specific resource target that should have been receiving them.

Perhaps because:
1. the toposort implementation preserves the order of the inputs in cases where they don't have dependencies
2. goal run.jvm requires a single target, which would mean that the context would already be ordered for that target
3. goal test.junit is currently mostly-insane in terms of resources (all of everything on the classpath: definite TODO)
...this was resulting in a somewhat-correct (?) ordering of resources at execution time, at least for targets that cared. But if you requested the resources for a particular target using compile_classpath.get_for_target(t), you would get things that you did not depend on.

Fixing that problem exposed the fact that depth-first search (DFS) is not the right way to sort dependencies for the JVM. The JVM needs the highest priority items first on the classpath, and in the case of a depgraph, this means that the nearest items (ie: self, then direct deps, and then their deps, etc) need to be listed first. This calls for breadth-first search (BFS.)

  • Add implementation of bfs for dependency graph traversals and use it UnionProducts
  • Only add resources to the relevant target
  • Move build_graph tests to the appropriate package

https://github.com/pantsbuild/pants/pull/1438

Issues

  • 1
  • 0
  • 1
  • 2
Description From Last Updated
This looks like it will add a target to the result set even if it doesn't match the predicate. Nick Howard (Twitter) Nick Howard (Twitter)
Jin Feng
Eric Ayers
Nick Howard (Twitter)
Stu Hood
Stu Hood
Stu Hood
Stu Hood
Eric Ayers
Stu Hood
Eric Ayers
Stu Hood
Eric Ayers
Stu Hood
Nick Howard (Twitter)
Stu Hood
Review request changed

Status: Closed (submitted)

Loading...