I am surprised to see what ` pants dependencies` does when d Pants #general

I am surprised to see what `./pants dependencies` ...

fresh-cat-90827

12/05/2022, 10:48 PM

I am surprised to see what

./pants dependencies

does when dependency inference is disabled and enabled 🧵

✅ 1

fresh-cat-90827

12/05/2022, 10:48 PM

on https://github.com/pantsbuild/example-python

Copy code

cp helloworld/translator/translator_test.py helloworld/translator/translator_another_test.py

Copy code

$ ./pants dependencies helloworld/translator/translator_test.py                                 

//:reqs#pytest
helloworld/translator/translator.py:lib

$ PANTS_PYTHON_INFER_IMPORTS=False ./pants dependencies helloworld/translator/translator_test.py

helloworld/translator/translator_another_test.py:tests

Why does Pants report

translator_another_test.py

to be a dependency of

helloworld/translator/translator_test.py

? It's clearly not:

Copy code

$ ./pants paths --to=helloworld/translator/translator_test.py --from=helloworld/translator/translator_another_test.py
[]

It must have something to do with the fact that they are part of the same target, but I can't find the reasoning behind this behavior when dependency inference is disabled.

curved-television-6568

12/05/2022, 10:51 PM

https://github.com/pantsbuild/pants/blob/20c8e11460ef9e7d9bfce12504f27729db7cc929/src/python/pants/backend/python/target_types_rules.py#L82

curved-television-6568

12/05/2022, 10:52 PM

that is, with dep inference off, there’s an implicit assumption that all files in the same module have a dependency on each other.

fresh-cat-90827

12/05/2022, 10:52 PM

wow

fresh-cat-90827

12/05/2022, 10:53 PM

I found https://github.com/pantsbuild/pants/pull/12906

👍 1

fresh-cat-90827

12/05/2022, 10:55 PM

okay this may make sense for

python_sources

, but for

python_tests

? 😕

curved-television-6568

12/05/2022, 10:56 PM

True, as tests are executed one by one, probably doesn’t make sense there.. cc @hundreds-father-404 thoughts?

hundreds-father-404

12/05/2022, 10:58 PM

hm yeah probably doesn't make sense for tests

hundreds-father-404

12/05/2022, 10:58 PM

but generally it'd be better to have an option to disable this. it should be legal to have your own dep inference implementation and ignore our builtin one

💯 1

hundreds-father-404

12/05/2022, 10:59 PM

this behavior destroys effective caching, even tho it's a good default if you have no dep inference at all

hundreds-father-404

12/05/2022, 10:59 PM

but generally it'd be better to have an option to disable this.

advanced option on python-infer. In fact I thought we added it, but couldn't find it. Someone wanted it 2 months ago

fresh-cat-90827

12/05/2022, 11:06 PM

amazing, thank you for giving it a thought. Shall I file a GH issue with the motivation and command examples to illustrate the problem?

Copy code

$ ./pants --changed-since=HEAD --changed-dependees=direct \
    --filter-target-type=python_test list

The use case is that if there have been changes to a test module, this command would list all Python test modules in the same

python_tests

target which is unnecessary

👍 1

fresh-cat-90827

12/05/2022, 11:38 PM

https://github.com/pantsbuild/pants/issues/17717

happy-kitchen-89482

12/05/2022, 11:45 PM

that is, with dep inference off, there’s an implicit assumption that all files in the same module have a dependency on each other.

Is it “all files in the same module” or “all files in the same target generator”?

hundreds-father-404

12/05/2022, 11:46 PM

same target generator

curved-television-6568

12/06/2022, 12:25 AM

☝️ yep, I was inaccurate in that statement

4 Views

Open in Slack

Previous Next