Hey I’ve been trying to read about creating custom targets in pants. What I’m trying to achieve is create a

python_binary

target that can remove dependencies or transitive dependencies and not packaged them. I’ve got some util targets that depend on

tensorflow

, but don’t use it the code paths in certain binaries. I know we could be disciplined and not have it in the build graph but that could add a lot of targets that aren’t really necessary. I’ve used bazel’s

depset

in before achieving something like this: Is there a good way to get this in pants?

Hey Charlie. With the upcoming Pants 2.0, we added

!

to the

dependencies

field as a way to ignore dependencies from a target. The simplest thing to do would be to write a macro (https://www.pantsbuild.org/v2.0/docs/macros) like

custom_python_binary

and have it always add

'!//:tensorflow'

to the

dependencies

field. —

or transitive dependencies

This is a little trickier. In Pants, creating a custom target type and/or using a macro would only influence the direct dependencies for that target. That is, it would only help to avoid this case:

python_binary(
  dependencies=["bad_dep"]
)

Not this case:

python_library(
  name="util",
  dependencies=["bad_dep"],
)

python_binary(
  dependencies=[":util"],
)

-- But, you could use the Rules API to essentially fork the builtin

binary

implementation. You could do something like check “If the target is a

custom_python_binary

, then no matter what exclude tensorflow; otherwise, use dependencies like normal.” You’d use the builtin rules for everything else like

test

, and swap in your rules that you forked. It sounds like correctly handling transitive deps is important too, right?

Yes It’s important for transitive too. How are transitive deps resolved? I was hoping I could extend the

python_binary

implementation with another field

deps_to_remove

or something.

Another approach is to have two targets referring to the same util code. One with tensorflow, and one without:

python_library(
  name="utils_without_tf",
  dependencies=["foo", "bar"]
)

python_library(
  name="utils",
  dependencies=[":utils_without_tf", "//:tensorflow"]
)

But this means that the BUILD file authors must now go out of their way to depend on the correct target. And generally Pants is less ergonomic when there’s >1 target owning a file. For example, dependency inference wouldn’t be able to infer a dependency anymore because it doesn’t know which of the two targets you want to use.

That would be great.

We’re hoping to have alpha0 tomorrow or Friday. There’s a pretty bad performance regression in the time to create the rule graph during the past 3 releases. We’re very close to landing a fix for.

I think it could definitely be useful for other people. I’ll open an issue

Thanks! One question that would be helpful for us to scope it: Would you want this to be expressed in the BUILD file, through the

dependencies

field? Or it’s okay to have more ceremony like having to define a new target type? If we want it to live in BUILD files, one possible syntax is to use

!!ignore_me_transitively

..? vs.

!ignore_me_directly

.

If the precedent is set

!

can ignore things it could be in the

BUILD

files. My concern is that could be too magical and a user wouldn’t understand what is going on. If there is a easy code snip-it you could put on the website it could be helpful to teach people about more advanced api. I don’t have a strong opinion.

If we add an entry point in the Target API, that would be pretty easy for a codebase admin to do. The code would essentially be:

class CustomPythonBinaryDependencies(Dependencies):
   exclude_transitively = ["bad_target"]

class CustomPythonBinary(Target):
  alias = "custom_python_binary"
  core_fields = (*(FrozenOrderedSet(PythonBinary.core_fields) - {Dependencies}), CustomPythonBinaryDependencies)

Then, anytime the BUILD file uses

custom_python_binary

, those hardcoded excluded targets would be ignored. Other than that, it behaves identically to a normal

python_binary

target. But that’s really really special-cased, and not at all obvious where that would fit into the docs. It also might not be generic enough. It requires you to put a list of hardcoded addresses in a target definition, rather than being able to tweak in a decentralized way in a BUILD file.

I can see this being useful for other users, so we'd definitely welcome the contribution. There are complications though. E.g., if we go the

!!

route, does that mean "don't bring in this dep through this target"? Or "don't bring in this dep through any target'? Maybe the

!!

has to be on

python_binary

or other root targets, and it means "strip this dep from the transitive closure when computed on (but not through) this target"?

Agreed we’d need to answer that question. Expecting it to be on a “root” target—i.e. a target with no dependees—is tricky to implement, though. There is nothing enforcing that no one depends on a

python_tests

or

python_binary

target, outside of convention. The best thing I could think of to try our best to enforce that

!!

is only used on roots is to have plugin authors express if it’s supported in the Target API. When defining a Dependencies field for their target type, they could set a class property `supports_transitive_excludes = True`:

class PythonBinaryDependencies(Dependencies):
     supports_transitive_excludes = True

If this is set to the default of

False

, then we’ll error if any values in the

dependencies

field start with

!!

.

Sounds great. Commented that on the issue

Great! I’m hoping to get it into the alpha0 release we’re planning for tomorrow. So far no hiccups in implementing it, beyond getting pulled to another priority

That’s awesome!

All ready 🙂 https://github.com/pantsbuild/pants/pull/10663