From 11742c23567587e60a75a083024af4c4eff792e8 Mon Sep 17 00:00:00 2001 From: Stu Hood Date: Fri, 31 Jul 2020 12:14:51 -0700 Subject: [PATCH] Rely on the init-injection rules to trigger errors for un-depended-on & non-empty __init__.py files, and clarify that you need an owning target regardless of inference. [ci skip-rust] [ci skip-build-wheels] --- .../python/dependency_inference/rules.py | 15 ++++++++------ .../backend/python/rules/missing_init.py | 20 ++++++++++++++----- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/python/pants/backend/python/dependency_inference/rules.py b/src/python/pants/backend/python/dependency_inference/rules.py index 0e047d72235f..f319515b04f9 100644 --- a/src/python/pants/backend/python/dependency_inference/rules.py +++ b/src/python/pants/backend/python/dependency_inference/rules.py @@ -52,10 +52,10 @@ def register_options(cls, register): type=bool, help=( "Infer a target's dependencies on any __init__.py files existing for the packages " - "it is located in (recursively upward in the directory structure). Note that if " - "inference is disabled, empty ancestor __init__.py files will still be included " - "even without an explicit dependency, but ones containing any code (even just " - "comments) will not, and must be brought in via an explicit dependency." + "it is located in (recursively upward in the directory structure). Regardless of " + "whether inference is disabled, empty ancestor __init__.py files will still be " + "included even without an explicit dependency, but ones containing any code (even " + "just comments) will not, and must be brought in via an explicit dependency." ), ) register( @@ -138,9 +138,11 @@ async def infer_python_init_dependencies( ) # And add dependencies on their owners. + # NB: Because the python_sources rules always locate __init__.py files, and will trigger an + # error for files that have content but have not already been included via a dependency, we + # don't need to error for unowned files here. owners = await MultiGet( - Get(Owners, OwnersRequest((f,), OwnersNotFoundBehavior.error)) - for f in extra_init_files.snapshot.files + Get(Owners, OwnersRequest((f,))) for f in extra_init_files.snapshot.files ) return InferredDependencies(itertools.chain.from_iterable(owners)) @@ -164,6 +166,7 @@ async def infer_python_conftest_dependencies( ) # And add dependencies on their owners. + # NB: Because conftest.py files effectively always have content, we require an owning target. owners = await MultiGet( Get(Owners, OwnersRequest((f,), OwnersNotFoundBehavior.error)) for f in extra_conftest_files.snapshot.files diff --git a/src/python/pants/backend/python/rules/missing_init.py b/src/python/pants/backend/python/rules/missing_init.py index bb4625af4689..e08aa072cddb 100644 --- a/src/python/pants/backend/python/rules/missing_init.py +++ b/src/python/pants/backend/python/rules/missing_init.py @@ -3,9 +3,10 @@ from dataclasses import dataclass +from pants.backend.python.dependency_inference.rules import PythonInference from pants.backend.python.rules.ancestor_files import AncestorFiles, AncestorFilesRequest from pants.engine.fs import Digest, DigestContents, Snapshot -from pants.engine.rules import Get, rule +from pants.engine.rules import Get, collect_rules, rule @dataclass(frozen=True) @@ -26,7 +27,9 @@ class MissingNonEmptyInitFiles(Exception): @rule -async def find_missing_empty_init_files(request: MissingInitRequest) -> MissingInit: +async def find_missing_empty_init_files( + request: MissingInitRequest, python_inference: PythonInference, +) -> MissingInit: """Find missing empty __init__.py files. This is a convenience hack, so that repos that aren't using dep inference don't have @@ -46,10 +49,17 @@ async def find_missing_empty_init_files(request: MissingInitRequest) -> MissingI extra_init_files_contents = await Get(DigestContents, Digest, extra_init_files.snapshot.digest) non_empty = [fc.path for fc in extra_init_files_contents if fc.content] if non_empty: + inference_hint = ( + "" + if python_inference.inits + else ( + ", and then add explicit dependencies (or enable `--python-infer-inits` to " + "automatically add the dependencies)" + ) + ) err_msg = ( f"Missing dependencies on non-empty __init__.py files: {','.join(non_empty)}. " - f"To fix, either enable dependency inference or add explicit dependencies on these " - f"files." + f"To fix: ensure that targets own each of these files{inference_hint}." ) # TODO: Note that in the stripped case these paths will be missing their source roots, # which makes this error message slightly less useful to the end user. @@ -60,4 +70,4 @@ async def find_missing_empty_init_files(request: MissingInitRequest) -> MissingI def rules(): - return [find_missing_empty_init_files] + return collect_rules()