https://pantsbuild.org/ logo
Join Slack
Powered by
In a directory `A` with subdirectories `B`, `C`, `...
# general
p
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
You cannot use globs for dependencies. It sounds like you want the generic 
target
target https://www.pantsbuild.org/docs/reference-target
p
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
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
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
Ok, but why is the latter needed?
p
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
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
here are the contents of st2tests.
st2tests_contents.txt
I'm trying to think through how I might replace all the existing targets with one target like that.
e
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
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
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
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
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
I'm not sure I understand - where would that infra function/class go?
e
Anywhere.
p
"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
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
yeah. people add the packs when writing tests that need them.
e
Ok, so presumably this boilerplate thing then import is not too onerous on test writers?
p
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
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
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
Exactly and great.
p
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
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
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
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
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
This should help you with the python_tests target customization: https://github.com/pantsbuild/pants/pull/12156
👏 1
💯 1
4 Views
Open in Slack
PreviousNext
Powered by