From 6cbb607c43febcc93dcf4b83635358b5141c3c00 Mon Sep 17 00:00:00 2001 From: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Date: Mon, 20 Jul 2020 11:40:34 -0700 Subject: [PATCH] Fix validation of dependencies ignores breaking with generated subtargets (#10407) ### Problem We eagerly validate that every `!` is used in the `dependencies` field as a matter of good UX, because it's confusing how all the different pieces fit together. However, imagine this: you have a target that owns 5 files. You set on that target `!./bad.py`. However, only one of those 5 files actually uses `bad.py`. This means that the 4 other generated subtargets would now all complain that the ignore `!./bad.py` is unused. [ci skip-rust-tests] --- src/python/pants/engine/internals/graph.py | 6 ++++- .../pants/engine/internals/graph_test.py | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/python/pants/engine/internals/graph.py b/src/python/pants/engine/internals/graph.py index 1b3cdb9dd75..17178130f6a 100644 --- a/src/python/pants/engine/internals/graph.py +++ b/src/python/pants/engine/internals/graph.py @@ -769,7 +769,11 @@ async def resolve_dependencies( *used_ignored_addresses, *used_ignored_file_deps, } - if unused_ignores: + # If there are unused ignores and this is not a generated subtarget, we eagerly error so that + # the user isn't falsely led to believe the ignore is working. We do not do this for generated + # subtargets because we cannot guarantee that the ignore specified in the original owning + # target would be used for all generated subtargets. + if unused_ignores and not request.field.address.generated_base_target_name: raise UnusedDependencyIgnoresException( request.field.address, unused_ignores=unused_ignores, result=result ) diff --git a/src/python/pants/engine/internals/graph_test.py b/src/python/pants/engine/internals/graph_test.py index aa8ae965f94..58e63510d65 100644 --- a/src/python/pants/engine/internals/graph_test.py +++ b/src/python/pants/engine/internals/graph_test.py @@ -1198,3 +1198,27 @@ def test_dependency_inference(self) -> None: Address.parse("//:inferred_and_provided2"), ], ) + + self.assert_dependencies_resolved( + requested_address=Address( + "demo", target_name="f1.st", generated_base_target_name="demo" + ), + enable_dep_inference=True, + expected=[ + Address.parse("//:inferred1"), + Address("", target_name="inferred2.st", generated_base_target_name="inferred2"), + Address.parse("//:inferred_and_provided1"), + Address.parse("//:inferred_and_provided2"), + ], + ) + + self.assert_dependencies_resolved( + requested_address=Address( + "demo", target_name="f2.st", generated_base_target_name="demo" + ), + enable_dep_inference=True, + expected=[ + Address.parse("//:inferred_and_provided1"), + Address.parse("//:inferred_and_provided2"), + ], + )