Nick Howard (Twitter) got a fish trophy!

[engine] Fix running changed with v2 flag; Replace context address_mapper when using engine; Fix excludes filespecs in engine globs

Review Request #4114 - Created July 23, 2016 and submitted

Nick Howard (Twitter)
benjyw, ity, jsirois, patricklaw, stuhood

Changed was failing to work because when the change calculator attempted to look up targets, it would use the existing address mapper. That address mapper would parse the BUILD files for itself, and then pass a BuildFileAddress object to the LegacyBuildGraph. That would blow up because the engine doesn't know how to compute from one of those.

This change introduces a LegacyAddressMapper that uses the engine. This ensures that the interaction between the engine backed build graph and the address mapper will have matching types. To do this, I extracted a common interface class and reimplemented the methods on AddressMapper that are used from other classes.

While working on this, I ran into a bug with exclude handling in the engine's glob implementation. The globs for the excludes weren't being translated into filespecs correctly, which lead to things blowing up. I could break that out as a separate changeset if this is too big.

This patch includes a fix for it and some regression tests.

Note: This introduces a validation that filespecs are prefixed with the relpath of the fileset they are a part of. This is something that changed assumed to be true, but wasn't in all cases. It may cause using absolute paths in BUILD files as part of glob declarations to fail.

Wrote an integration test for changed, got it passing. Added some regression tests around excludes and got them passing. CI away on linked PR.


  • 0
  • 6
  • 1
  • 7
Description From Last Updated
Stu Hood
Nick Howard (Twitter)
Stu Hood
Nick Howard (Twitter)
Stu Hood
Nick Howard (Twitter)
Yujie Chen
Yujie Chen
Nick Howard (Twitter)
Nick Howard (Twitter)
Review request changed

Status: Closed (submitted)

Change Summary:

Submitted as