Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fail for deleted-but-depended-on targets in changed (#5636)
### Problem #5579 broke detection of deleted targets. ### Solution As described in #382 (mega classic!), it might be possible to more deeply integrate change detection into the v2 engine itself to compute a delta on the graph. But for now we defer a deeper solution and simply ensure that we fail for deleted targets by transitively expanding targets. Adds a test to cover the behaviour. ### Result Due to fully hydrating targets, this represents a linear performance regression from #5579: the runtime of `--changed-include-dependees=transitive` for a small set of roots used to be slightly lower than the runtime for `./pants list ::`, because the operation that occurred "on the entire graph" was computing `HydratedStructs`, rather than computing `TransitiveHydratedTargets`. The impact for exactly that step is constant, and fairly high: ``` # before: DEBUG] Root Select(Collection(dependencies=(DescendantAddresses(directory=u''),)), =Collection.of(Struct)) completed. DEBUG] computed 1 nodes in 1.709688 seconds. there are 8858 total nodes. # after: DEBUG] Root Select(Collection(dependencies=(DescendantAddresses(directory=u''),)), =TransitiveHydratedTargets) completed. DEBUG] computed 1 nodes in 2.989497 seconds. there are 15916 total nodes. ``` ... but the impact on overall runtime is dependent on the count of targets that are transitively affected, because for all affected targets, we're going to need to compute transitive roots anyway. So for the example change from #5579 which affects 567 targets: ``` time ./pants --changed-diffspec=22ca0604b1c6ce8de019214b821e922aac66b026^..22ca060 --changed-include-dependees=transitive list | wc -l # before: real 0m4.877s user 0m4.081s sys 0m1.068s # after real 0m5.294s user 0m4.487s sys 0m1.142s ``` For a change impacting only 14 targets the difference is slightly more pronounced: ``` $ time ./pants --changed-diffspec=f35e1e6fb1cdf45fcb5080cfe567bdbae8060125^..f35e1e6 --changed-include-dependees=transitive list | wc -l # before: real 0m4.279s user 0m3.376s sys 0m1.011s # after: real 0m4.954s user 0m4.284s sys 0m1.120s ```
- Loading branch information