-
-
Notifications
You must be signed in to change notification settings - Fork 637
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider porting graph-inspection tests to rust #4027
Comments
stuhood
pushed a commit
that referenced
this issue
Sep 20, 2018
### Problem As described in #5788: `@rules` need a way to rely on values that are provided at request time, but which do not necessarily participate in the signatures of all `@rules` between the root and the consuming `@rule`. This is related to the existing concept of "variants", but requires an implementation that does not introduce variants into `Node` identities in subgraphs where they are not required. Additionally, as described a while back in #4304, it should be possible to generate concrete subgraphs by removing ambiguity from the `RuleGraph`... but ambiguity is currently a "feature" required for composability of `@rule`s that do not know about one another. ### Solution This change merges `Variants` and "subjects" into `Params`, and statically determines which `Params` are required in each subgraph. In order to handle cases where multiple providers of an `@rule` type dependency are available with different required input `Params`, the change "monomorphizes" (duplicates) `RuleGraph` entries per used parameter set. This allows us to remove runtime `Noop`s, because every `RuleGraph` entry (and thus `Node`) has exactly one `@rule` provider for each of its declared dependencies. ### Result Lays groundwork for #4020 and #5788. Fixes #4304 by monomorphizing `RuleGraph` entries and removing `Noop`. Fixes #4027 by... deleting that code. This change does not yet expose any sort of UX for providing more than one `Param` in a `Get` or root request, but it was already way too large, so I've opened #6478 for followup.
stuhood
added a commit
to twitter/pants
that referenced
this issue
Jan 10, 2019
stuhood
added a commit
to twitter/pants
that referenced
this issue
Jan 10, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There are a series of graph-introspection tests in:
tests/python/pants_test/engine/test_scheduler.py
tests/python/pants_test/engine/test_fs.py
tests/python/pants_test/engine/test_graph.py
that are half integration test, and half unit test (in that they inspect the internals of the execution graph to validate how a result was achieved). All tests that inspect graph internals have been skipped.
The text was updated successfully, but these errors were encountered: