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.

Sign up for GitHub

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

Jump to bottom

Use dependency inference internally #10283

Merged
merged 14 commits into from Jul 8, 2020
Merged

Use dependency inference internally #10283

merged 14 commits into from Jul 8, 2020

Conversation

Eric-Arellano
Copy link
Contributor

We also add a new python_integration_tests macro to cut down on boilerplate even further.

@Eric-Arellano
Use dep inference
5a7d2ca 
# Rust tests will be skipped. Delete if not intended.
[ci skip-rust-tests]
@Eric-Arellano
Add python_integration_tests macro
3b57fa2 
# Rust tests will be skipped. Delete if not intended.
[ci skip-rust-tests]
@Eric-Arellano
In CI, only warn on files_not_found_behavior
19f7f48 
[ci skip-rust-tests]
@Eric-Arellano
Stop erroring on files_not_found_behavior in general
191e1d4 
# Rust tests will be skipped. Delete if not intended.
[ci skip-rust-tests]
@Eric-Arellano
Fix bad 3rdparty module mapping
0a4ce05 
# Rust tests will be skipped. Delete if not intended.
[ci skip-rust-tests]
@Eric-Arellano
Fix some testutil files having two owning targets
3423306 
This was unintentional. It resulted in dep inference not working due to ambiguity.

# Rust tests will be skipped. Delete if not intended.
[ci skip-rust-tests]
@Eric-Arellano
Fix engine tests leaving off resource dependencies
c73fa45 
# Rust tests will be skipped. Delete if not intended.
[ci skip-rust-tests]
@Eric-Arellano
Fix subproject_integration_test.py not being declared as an integrati…
695f6a2 
…on test

# Rust tests will be skipped. Delete if not intended.
[ci skip-rust-tests]
@Eric-Arellano
Fix integration tests
52e8ab2 
We take the approach of forcing the target author to explicitly say whether it uses PantsRun. This is to avoid having worse caching on rules-based ITs which do not actually depend on PantsRunIntegrationTest.

# Rust tests will be skipped. Delete if not intended.
[ci skip-rust-tests]
@Eric-Arellano
Fix some more ITs
faf20cc 
# Rust tests will be skipped. Delete if not intended.
[ci skip-rust-tests]
@Eric-Arellano Eric-Arellano changed the title WIP: Use dependency inference internally Use dependency inference internally Jul 7, 2020
@Eric-Arellano Eric-Arellano marked this pull request as ready for review July 7, 2020 20:07
stuhood
stuhood approved these changes Jul 7, 2020
View reviewed changes
Copy link
Sponsor Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow. That was effective beyond my absolute best hopes. Really, really awesome. Thanks Eric!

benjyw
benjyw approved these changes Jul 7, 2020
View reviewed changes
Copy link
Sponsor Contributor

@benjyw benjyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wowowow!

How did you figure out which deps could be removed? Manually?

@Eric-Arellano
Copy link
Contributor Author

How did you figure out which deps could be removed? Manually?

Mostly trial and error. I started by deleting everything, although paid attention to targets that I knew have resources involved, such as testutil:int-test. From there, I added things back based on feedback from CI. The git history is accurate, if you want to see each step :)

I'm really happy with the result. Now when you read BUILD files, you get all the pertinent metadata. For example, the dependencies field is now illuminating because it highlights things that can't be inferred. I liked Stu's point yesterday about there being less value in repeating information.

Also, the perf hit is much lower than I expected!

@Eric-Arellano
Merge branch 'master' of github.com:pantsbuild/pants into dep-inference
7d58475 
[ci skip-rust-tests]
@Eric-Arellano
Fix reversion test
1496711 
# Rust tests will be skipped. Delete if not intended.
[ci skip-rust-tests]
@Eric-Arellano
Hack around issue with release so that we can land this PR
c0f0318 
John has a fix incoming.

# Rust tests will be skipped. Delete if not intended.
[ci skip-rust-tests]
@Eric-Arellano
Merge branch 'master' of github.com:pantsbuild/pants into dep-inference
8453e91 
# Conflicts:
#	src/python/pants/engine/internals/BUILD
#	src/python/pants/engine/internals/examples/BUILD
#	src/python/pants/engine/legacy/BUILD
#	src/python/pants/init/BUILD
@Eric-Arellano Eric-Arellano merged commit d60cde2 into pantsbuild:master Jul 8, 2020
@Eric-Arellano Eric-Arellano deleted the dep-inference branch July 8, 2020 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuhood stuhood stuhood approved these changes

@benjyw benjyw benjyw approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Eric-Arellano @stuhood @benjyw