Are we certain that we want to add dependency ignores via

!

? (https://github.com/pantsbuild/pants/pull/10385). I think the motivating cases are: 1) Our dep inference logic has a flaw. This is an escape hatch for dep inference causing false positives. 2) Even if it’s not a false positive, a user for some reason doesn’t like having a dependency included, e.g. because it causes a cycle. #2 is what I’m a little concerned about. Things like MyPy won’t work if you leave off a dependency. But I suppose this falls into “This is an advanced escape hatch 99% of users won’t need; trust the user when they say they don’t want something”?

i think of it as some mix of both of those, yea. accounting for unknown-unknowns.

cc @happy-kitchen-89482 on if we want to really add this feature. It’s silly to do an ignore for an explicit dep like

[':foo', '!//:foo']

. This is really only useful for dep inference. Right now, we haven’t known of dep inference causing any false positives because it is overly conservative. So this is really for the second reason, but that second reason is better solved via cycle tolerance

Yeah, it's only for dep inference, I think we do need an escape hatch if we start introducing heuristics (like inferring deps from Django settings files) but possibly not for deps that are only inferred from imports?

And we decided to not infer from strings for 2.0. Too many false positives. That will need more design, like a possible global option like Toolchain had with buildgen

i feel like there are false positives where you might not have something until prod, and we shouldn’t infer the local thing? not sure though.

That could happen in terms of things like conditional imports, indeed. That makes sense.

if PROD:
   import really_expensive_thing

ah, yea, true.

I think that is an actual pattern that people use. But, how would that actually work? You wouldn’t dynamically rewrite your BUILD file in CI to soemtimes include it and sometimes not

you could load it via

PEX_EXTRA_SYS_PATH

, for example.

Okay, that makes sense. I agree that this is cheap enough of a syntax to justify having the escape hatch. And I agree with your review to make it even stricter than the current implementation; we error if your ignore is unused