https://pantsbuild.org/ logo
#development
Title
# development
h

hundreds-father-404

07/16/2020, 9:26 PM
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”?
w

witty-crayon-22786

07/16/2020, 10:37 PM
i think of it as some mix of both of those, yea. accounting for unknown-unknowns.
👍 1
if we wanted to keep it in our back pocket until someone complained/had-a-concrete-need-for-it, that would be an option too.
h

hundreds-father-404

07/16/2020, 10:40 PM
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
h

happy-kitchen-89482

07/16/2020, 10:52 PM
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?
h

hundreds-father-404

07/16/2020, 10:52 PM
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
So, right now, it’s only false positives based on import statements
w

witty-crayon-22786

07/16/2020, 10:55 PM
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.
like maybe you have a conditional import, and you just never want it to be available in a test, but at runtime/deploytime you mix something into the PEX…?
it’s a stretch.
h

hundreds-father-404

07/16/2020, 10:57 PM
That could happen in terms of things like conditional imports, indeed. That makes sense.
Copy code
if PROD:
   import really_expensive_thing
w

witty-crayon-22786

07/16/2020, 10:57 PM
ah, yea, true.
basically, this particular syntax seems to be pretty low cost, and i feel like we’re prepared for eventualities by having it.
(unlike a lot of syntax, heh)
h

hundreds-father-404

07/16/2020, 10:59 PM
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
w

witty-crayon-22786

07/16/2020, 10:59 PM
you could load it via
PEX_EXTRA_SYS_PATH
, for example.
👍 1
h

hundreds-father-404

07/16/2020, 11:01 PM
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
👍 1