Fix --changed-dependees to work when v1 is disabled
#10235
Conversation
[ci skip-rust-tests]
# Rust tests will be skipped. Delete if not intended. [ci skip-rust-tests]
| (changed_addresses,) = session.product_request(ChangedAddresses, [changed_request]) | ||
| logger.debug("changed addresses: %s", changed_addresses.addresses) | ||
| (changed_addresses,) = session.product_request( | ||
| ChangedAddresses, [Params(changed_request, options_bootstrapper)] |
There was a problem hiding this comment.
FYI we must pass options_bootstrapper because dep inference requires it. We now use dep inference when considering --changed-dependees.
| map_addresses_to_dependees, | ||
| find_dependees, |
There was a problem hiding this comment.
So this is a little weird. We need the dependees.py rules to be always in the build graph because they are now always used for resolving addresses. Without this, most unit tests fail due to an incomplete rule graph.
I wasn't sure where to register the rules, among these 3 options:
- Move the dependees logic out of
backend/project_info/dependees.py, e.g. intograph.py. Always activate from there. - Export the rules from
changed.py, because we already always register these rules. - Directly import these two rules from
engine_initializer.py.
I didn't go with #1 because I couldn't think of a good place for those two rules. It makes sense to me to have the logic for dependees in dependees.py. So, I went with #2.
There was a problem hiding this comment.
Seems fine to me, and we can always change this later. Nice thing about the engine is that you don't import rules in order to use them, so moving rule code around doesn't require deprecation!
| map_addresses_to_dependees, | ||
| find_dependees, |
There was a problem hiding this comment.
Seems fine to me, and we can always change this later. Nice thing about the engine is that you don't import rules in order to use them, so moving rule code around doesn't require deprecation!
While we were using a rule to calculate the dependees, it was using a v1 code path based on TargetAdaptor. This means that
--changed-dependeescaused a crash when v1 is disabled.To land this, we refactor
dependeesinto two helper rules so that any rule can nowawait Get(Dependees, DependeesRequest).[ci skip-rust-tests]