Skip to content

Commit

Permalink
A Param should never be directly used to satisfy a Get (pantsbuild#9890)
Browse files Browse the repository at this point in the history
### Problem

As described in pantsbuild#9878, we observed a case where a `Param` was being used to direct satisfy a `Get`. But this runs afoul of an invariant that has been in place for a while now that the "provided" `Param` (ie the "input" to the `Get`) must be consumed by the subgraph below the `Get`.

The reasoning behind this invariant is that when a user uses a `Get`, the intent is always for the input `Param` to be used in that computation. And that holds in this case.

### Solution

Extract `RuleGraph` building into a separate module, and then add two tests of cyclic behavior: one that was already passing, and another that described the situation in pantsbuild#9878, and which was failing. Finally, enforce the invariant for `Params` (as it was already being applied to `WithDeps` entries: ie, `@rules`) to fix the test.

### Result

Fixes pantsbuild#9878.
  • Loading branch information
stuhood authored May 29, 2020
1 parent ba18e8e commit 676877b
Show file tree
Hide file tree
Showing 5 changed files with 698 additions and 601 deletions.
8 changes: 5 additions & 3 deletions src/python/pants/engine/internals/scheduler_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,10 @@ class UnionX:

@rule
async def error_msg_test_rule(union_wrapper: UnionWrapper) -> UnionX:
union_x = await Get[UnionX](UnionWithNonMemberErrorMsg, union_wrapper.inner)
return union_x
# NB: We install a UnionRule to make UnionWrapper a member of this union, but then we pass the
# inner value, which is _not_ registered.
_ = await Get[A](UnionWithNonMemberErrorMsg, union_wrapper.inner)
raise AssertionError("The statement above this one should have failed!")


class TypeCheckFailWrapper:
Expand Down Expand Up @@ -176,7 +178,7 @@ def rules(cls):
transitive_coroutine_rule,
RootRule(UnionWrapper),
UnionRule(UnionBase, UnionA),
UnionRule(UnionWithNonMemberErrorMsg, UnionX),
UnionRule(UnionWithNonMemberErrorMsg, UnionWrapper),
RootRule(UnionA),
select_union_a,
UnionRule(union_base=UnionBase, union_member=UnionB),
Expand Down
2 changes: 1 addition & 1 deletion src/rust/engine/engine_cffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ use hashing::{Digest, EMPTY_DIGEST};
use log::{self, error, warn, Log};
use logging::logger::LOGGER;
use logging::{Destination, Logger, PythonLogLevel};
use rule_graph::RuleGraph;
use rule_graph::{self, RuleGraph};
use std::any::Any;
use std::borrow::Borrow;
use std::convert::TryInto;
Expand Down
Loading

0 comments on commit 676877b

Please sign in to comment.