-
-
Notifications
You must be signed in to change notification settings - Fork 645
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
Split Python targets into atom vs. generator and use source: str
field
#13231
Split Python targets into atom vs. generator and use source: str
field
#13231
Conversation
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
source: str
fieldsource: str
field
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
source: str
fieldsource: str
field
Each commit is distinct. -- I'm still thinking of a good title to put in the changelog to capture the change to dependency inference. Maybe
Fixing https://docs.google.com/document/d/1WWQM-X6kHoSCKwItqf61NiKFWNSlpnTC5QNu3ul9RDk/edit# will be important I think, as |
Also, the dependency inference speedup is an excellent example of why I felt so compelled to prioritize this change. As the author of that dep inference implementation, I had no idea we were running the import parser twice! Probably I could have profiled better, but also it's hard to reason about |
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This actually speeds up dependency inference! We no longer run
import_parser.py
twice on the same files.
Whoops.
Fixing https://docs.google.com/document/d/1WWQM-X6kHoSCKwItqf61NiKFWNSlpnTC5QNu3ul9RDk/edit# will be important I think, as ./pants dependencies :: no longer includes the generated targets given that we don't "expand" the input roots
Yea, fixing Specs
matching is a blocker for 2.8.0
.
Confirmed that depending on a |
Now our Python code operates on
python_source
andpython_test
targets. This causes some changes.Dependency inference
Dependency inference no longer runs on
python_sources
andpython_tests
, onlypython_source
andpython_test
. That applies to import analysis, conftest.py, and__init__.py
.This actually speeds up dependency inference! We no longer run
import_parser.py
twice on the same files.Before:
After:
It means that
./pants dependencies src/py:lib
now only has explicitly declared dependencies. You have to use--transitive
to get the results of dep inference.Thanks to this change, we can simplify
import_parser.py
to assume there is only one file.Lockfile generation
When determining the interpreter constraints to use, we now look at generated targets, not target generators. We only run over the
python_source
andpython_test
targets that actually will be used.This makes us future-proof to an
overrides
field addingskip_tool
to only some of the files. I think it also fixes our dependency resolution for Pylint looking at direct deps?FieldSet.opt_out
no longer worries about file targetsPytest and MyPy used to skip running on non-file targets. That can be removed now. This unblocks allowing you to explicitly define a
python_source
/python_test
target.