Skip to content
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

Tolerate target cycles when using dependency inference #10393

Merged
merged 3 commits into from Jul 17, 2020

Conversation

Eric-Arellano
Copy link
Contributor

Closes #10059.

It's common in many languages, including Python, to sometimes have import cycles. When using generated subtargets—meaning either dependency inference or explicit file dependencies—Pants will tolerate these cycles. For now, there is no other way to express that a cycle should be tolerated. In the future, we may consider adding a syntax to BUILD files to allow indicating that cycles should be tolerated; it's easier to loosen than it is to tighten.

[ci skip-rust-tests]

# Rust tests will be skipped. Delete if not intended.
[ci skip-rust-tests]
We can now use our conditional `if TYPE_CHECKING` imports. We no longer have the problem that sometimes MyPy would fail to find the import; all dependencies are properly modeled :)

# Rust tests will be skipped. Delete if not intended.
[ci skip-rust-tests]
@coveralls
Copy link

Coverage Status

Coverage remained the same at 0.0% when pulling ca63934 on Eric-Arellano:tolerate-cycles into 45e0ac4 on pantsbuild:master.

@Eric-Arellano Eric-Arellano merged commit d2ecdda into pantsbuild:master Jul 17, 2020
@Eric-Arellano Eric-Arellano deleted the tolerate-cycles branch July 17, 2020 22:54
Eric-Arellano added a commit to Eric-Arellano/pants that referenced this pull request Jul 18, 2020
Eric-Arellano added a commit that referenced this pull request Jul 18, 2020
stuhood added a commit that referenced this pull request Jul 22, 2020
…in file-addresses. (#10409)

### Problem

After more deeply investigating the issues with #10230 (demonstrated in #10393 and its revert), it doesn't seem like the right idea to rely on the engine's cycle detection (which the implementation of #10230 showed should primarily be used for deadlock detection) to expose high level cycle tolerance for the build graph.

### Solution

Move to iterative transitive target expansion (à la #10046), and cycle detect afterward using DFS with a stack of visited entries. 

### Result

Fixes #10059, and closes #10229. A followup will back out portions of #10230 (but not all of it, as there were a number of other improvements mixed in).

[ci skip-rust-tests]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tolerate build graph cycles
3 participants