https://pantsbuild.org/ logo
#general
Title
# general
p

proud-dentist-22844

05/31/2021, 3:44 PM
In a directory
A
with subdirectories
B
,
C
,
D
(and many others), I want to create a
target()
that encompasses the
:metadata
targets in all subdirectories (so
./B:metadata
,
./C:metadata
,
./D:metadata
and every other subdirectory). Can I use globs to create dependencies? I've tried several glob variations, none of which seem to do what I'm looking for:
**:metadata
./**:metadata
*:metadata
./*:metadata
...
Hmm. I did
./dummy*:metadata
and got this error:
Copy code
DifferingFamiliesError: Expected AddressMaps to share the same parent directory 'st2tests/st2tests/fixtures/packs/dummy*', but received: 'st2tests/st2tests/fixtures/packs/dummy_pack_1/BUILD'
https://github.com/st2sandbox/st2/tree/pants/st2tests/st2tests/fixtures/packs every subdirectory with a pack.yaml files has a :metadata target. These fixtures are used for a lot of tests in a lot of places, so I'm trying to make a super target to simplify depending on all of them for the tests.
h

hundreds-father-404

05/31/2021, 4:04 PM
You cannot use globs for dependencies. It sounds like you want the generic
target
target https://www.pantsbuild.org/docs/reference-target
p

proud-dentist-22844

05/31/2021, 4:19 PM
Yeah. I'm trying to use the generic
target()
, but do I really have to register every subdirectory individually in the BUILD file?
Is there a way to say "depend on all targets in sub directories"?
e

enough-analyst-54434

05/31/2021, 4:33 PM
There is not by design. Historically this was to prevent all the bad outcomes of depending on too much, which globs tend to evolve towards even if those globs are actually correct when 1st written down. Clearly you have some motivation here though that maybe upends this thinking. What's the scenario?
For example, do the metadata targets cover a disjoint set of file types from other targets? If so can the target in A suffice with recursive globs to replace the targets in B, C and D with one comprehensive target?
Recursive sources globs that is.
p

proud-dentist-22844

05/31/2021, 4:57 PM
The
:metadata
targets get added by a
tailor
rule I wrote. The tests that use the fixtures in this directory expect to have access to all of the fixtures (they are all similar w/ slight variations for the various tests). When someone adds a new subdirectory,
./pants tailor
can easily take care of adding the new BUILD file, but I don't want people to forget to register it in the parent directory's super target.
e

enough-analyst-54434

05/31/2021, 5:09 PM
Ok, but why is the latter needed?
p

proud-dentist-22844

05/31/2021, 5:33 PM
I have a module,
st2tests
whose sole purpose is to provide files fixtures for tests in other modules. I don't have a good way to say "this test file needs these fixture files", so I made top-level
python_library
target in
st2tests
depend on most of the
files
targets within that module so that if you impor
st2tests.fixtureloader
, pants knows to include all the files as well. There are quite a few fixtures in there, so I'm trying to figure out how to set up the targets for the various fixtures so that they get picked up. I can't just do
sources=['**/*']
because there are python targets in there as well.
I'm trying to minimize the amount of work people will have to do to maintain the BUILD files or people will curse pants and say they prefer the old Makefiles which I dislike.
e

enough-analyst-54434

05/31/2021, 5:47 PM
And the fixtures are allowed to have arbitrary names? If the weren't you could just have a single top level target with
sources=['**/<fixture file naming pattern>']
p

proud-dentist-22844

05/31/2021, 5:53 PM
here are the contents of st2tests.
I'm trying to think through how I might replace all the existing targets with one target like that.
e

enough-analyst-54434

05/31/2021, 6:01 PM
Ok. So stepping back to confirm: You're choosing to create a single top level target for user (test writer) convenience knowing that will make test writers more happy than having that single too-big target will make them mad when a test fixture they don't use but do depend on via the umbrella changes and forces their test to re-run when it shouldn't?
p

proud-dentist-22844

05/31/2021, 6:13 PM
My problem is that the tests are lumped in one big directory
How can I distinguish which tests need which fixtures if I only have one BUILD file to work with? https://github.com/st2sandbox/st2/tree/pants/st2common/tests/unit
I would love to reorganize the tests into more logically discrete directories, but that will have to wait for some future change after pants is adopted (assuming I convince people to adopt it).
e

enough-analyst-54434

05/31/2021, 6:18 PM
So in that example, in reality, there is a direct dependency on just dummy_pack_1, it's just that Pants isn't smart enough to know that. Is that right?
p

proud-dentist-22844

05/31/2021, 6:18 PM
right
So, then I would have to build some kind of inference thing to parse all the test files and figure out which fixtures are actually used... Sounds like every possible solution involves some kind of pain.
e

enough-analyst-54434

05/31/2021, 6:21 PM
So, one answer: create an infra function or class and then, beside each pack emit a python file that minimally inherits that infra. Then the code you pointed me at imports that type and asks it to load. The new file per-pack should be reducible to one infra import line and one code line. With that, normal dep inference would solve all of this.
p

proud-dentist-22844

05/31/2021, 6:21 PM
I'm not sure I understand - where would that infra function/class go?
e

enough-analyst-54434

05/31/2021, 6:22 PM
Anywhere.
p

proud-dentist-22844

05/31/2021, 6:24 PM
"new file per pack" => some kind of codegen that puts a python file in each pack dir and then I import that in the file that uses the fixture?
e

enough-analyst-54434

05/31/2021, 6:26 PM
Sure, could be codegen. But if it's 3 lines you could just start with writing the boilerplate to prove it out.
Presumably people or a tool add packs? They could just be told, add this file too.
And pre-existing packs, well, script up a thing to backfill them all.
Then when you have time to implement codegen, you can just kill the files and the instruction to add them post-facto with ~no disruption and no time wasted now.
This sort of solution is much closer to "how would I do this without pants" - and that's always the best answer when its feasible. Then Pants can make things like codegenning the boilerplate better.
p

proud-dentist-22844

05/31/2021, 6:30 PM
yeah. people add the packs when writing tests that need them.
e

enough-analyst-54434

05/31/2021, 6:31 PM
Ok, so presumably this boilerplate thing then import is not too onerous on test writers?
p

proud-dentist-22844

05/31/2021, 6:31 PM
Yeah. It has to work without pants so that I can run tests with nosetest and with pants+pytest at the same time to prove that I haven't broken things.
e

enough-analyst-54434

05/31/2021, 6:32 PM
The big benefit to this style will be the ~correct dependencies and not too large ones. That will save trees and CI time.
👍 1
p

proud-dentist-22844

05/31/2021, 6:34 PM
ok. so the goal is to have the tests import something such that pants can infer the dependency. It would probably not go in the tests directory, so it would go somewhere in fixtures. hmm. I think I have a mental map of one way to do that. I'll try it with that test I showed you.
1
e

enough-analyst-54434

05/31/2021, 6:34 PM
Exactly and great.
p

proud-dentist-22844

05/31/2021, 7:14 PM
I added
st2tests/st2tests/fixtures/packs/dummy_pack_1/fixture.py
then I can import the vars I need from there, however, pants isn't inferring that dependency according to
./pants dependencies st2common/tests/unit/test_aliasesregistrar.py
Would overlapping source roots make it difficult for pants to infer dependencies?
pack.yaml
is a source root marker file, and there is a
st2tests/st2tests/fixtures/packs/dummy_pack_1/pack.yaml
file, so could that be messing this up?
yeah. dropping the marker from pants.toml allows it to infer the dep.
Hmm
🤯
e

enough-analyst-54434

05/31/2021, 7:25 PM
Yeah - source_roots == PYTHONPATH, classpath, etc entries, so they have the effect of chopping off the path prefix wrt imports in whatever language.
Did you mean to be using them as source root markers? No one imports yaml fwict. Or was that an attempt to get Resources targets working? Those use source roots since most (all?) languages that have a resource concept also load those files relative to the load path (PYTHONPATH, classpath, etc.)..
And, yeah, nested source roots will almost certainly cause problems if the nested one contains files meant to be used from the outer one as in your case.
I'd treat nested source roots as unsupported.
p

proud-dentist-22844

05/31/2021, 7:49 PM
Some of the packs need to be source roots themselves. I'll have to revisit how to deal with that later - for now I dropped the marker setting.
Well, implementing that required a lot of extra files, but it seems to work: https://github.com/st2sandbox/st2/commit/26ea0fdb25cba44b719aef00c232981eed50bbd0
Just a few more packs to go...
e

enough-analyst-54434

05/31/2021, 10:50 PM
Cool. You should be able to eliminate the
os
import, the
__all__
and reduce the symbols to something like
NAME, PATH = fixtures loader.here()
by using something like https://github.com/pantsbuild/pants/blob/24991b67600e17cbbf2d16591f5c3eb47fe0388e/src/python/pants/engine/rules.py#L352-L363
p

proud-dentist-22844

06/01/2021, 12:49 AM
Ooh nice. I added a function to minimize the boilerplate. I went ahead and passed
__file__
as an arg.
Thanks for your help figuring out how to add the pack fixture dependencies cleanly. I was struggling to figure that out for awhile.
❤️ 1
h

happy-kitchen-89482

06/01/2021, 9:54 AM
This should help you with the python_tests target customization: https://github.com/pantsbuild/pants/pull/12156
👏 1
💯 1