Now I think the dust is starting to settle on the ...
# development
c
Now I think the dust is starting to settle on the new visibility rules implementation. Looking at the BUILD file changes in this PR for example: https://github.com/pantsbuild/pants/pull/17630/files 🙂
🙂 1
image.png
Big thanks to @happy-kitchen-89482 for his patience and persistence on reviewing all of my visibility rules related PRs (~11 of them over a short period of time!)
p
Yeah way to go @happy-kitchen-89482! I wish every open source community read as friendly and responsive as pants.
1
❤️ 1
👖 1
🙂 1
Where is the best place to see a summary of the visibility syntax changes? I got a little lost in all the PRs.
c
I’m not surprised, it’s been a blur of changes
I’ll start writing the docs within the next day or so
p
So, in your example rule spec what is
"target_a"
? Is that like
python_sources
? Or is that an address to a target?
And in your new syntax,
()
is for tags and
[]
is for filepaths? Where did that syntax come from? Is there anything else like it in pants?
In your example BUILD file, how does
[fastapi]
compare to
//:reqs#fastsapi
? Something still feels disconnected.
Maybe I just need to wait and read the docs.
c
it’s valid questions, and not set in stone in any way (as you can see from my GH discussion, there’s not been very much feedback yet 😛 )
yes,
()
are for tags and
[]
for paths.
[fastapi]
is a “path glob”, if I used the full thing, it would’ve been
[3rdparty/python#fastapi]
there’s 4 different ways to “anchor” the globs:
//glob
anchors to project root
/glob
anchors to path of BUILD file where the glob is declared
that is if you have
src/BUILD
with a glob that reads
/foo
it will match the same as
//src/foo
3:
./glob
anchors to the rule where it is used. This make come into effect if I have a glob in
src/BUILD
that reads
./foo
but then I have a target in
src/a/BUILD
but with no rules in it, so the rules from
src/BUILD
applies (are inherited), so when target
src/a:baz
evaluates the
./foo
glob it will match
//src/a/foo
and finally 4:
glob
(i.e. no leading slash or period) is not anchored (I refer to it as floating, think
re.search
vs
re.match
for the anchored versions) and matches anywhere and up to the end. That is what I used in the
[fastapi]
it’s the final part of the name.
So, the parens for tags and sq brackets for path on the target selector part of the rule is very arbitrary. I just wanted some syntactic sugar for
{"path": "foo", "tags":["bar"], "type":"resources"}
and that became
resoures(bar)[foo]
I’m not aware of anything else in Pants that would apply here… using the address syntax too much could potentially become confusing I think, as it’s not addresses.
You can use that dict based syntax if you prefer! 😉
👍 1
perhaps paths will be more common than target types, so it would make sense in that case perhaps to swap it around a bit, so by default a bare value is a path rather than target type. 🤷 It’s just that we started with target types only so that was what was already there.
p
got it. Yeah, my first inclination is to use the dict format. But I want to make sure I understand the other syntax too.
👍 1
So, would
[fastapi]
match both
//3rdparty/python:reqs#fastapi
and
//3rdparty/python:reqs#fastapi-plugins
?
c
no, just the former, as the globs have implicit
$
at the end, so must match to the end
[fastapi*]
would, otoh
p
So, then should it be
[#fastapi]
instead to make sure it doesn't match something like
//3rdparty/python:reqs#aioauth-fastapi
?
The four ways to anchor the globs are much clearer. Is this only for the selector? Or could I use that in the rules too?
c
you may. I had that at first..
the anchoring are for the rules as well
(it started there 😉 )
So, then should it be
[#fastapi]
instead to make sure it doesn’t match something like
//3rdparty/python:reqs#aioauth-fastapi
?
as its only for what we have in our own requirments file and not the whole world, I’m not that concerned with that conflict (and actually in this case would see it as the right thing to cover both)
p
That feels pretty good.
//glob
,
/glob
,
./glob
,
glob
. And it looks like you made it handle
*
and `**`: so
*
only handles up to one level of a path, but
**
can match recursively, right?
💯 1
c
yes, additionally, the
foo/**
also matches
foo
so the trailing slash is optional when followed by recursive match all
👍 1
p
So,
[fastapi]
in regex form is basically:
^.*fastapi$
- Could i make that more explicit and do
[*fastapi]
if I wanted to make it clear that I'm trying to match more than one? Or is the wildcard only allowed at the end?
(hopefully my barrage of questions is helpful when it comes to writing the docs. If not, well, it's at least helpful for my mental model 😛 )
c
oh, I absolutely love the questions, it helps smoke out if there’s any bad choices and otherwise help clarify the concepts. and yes, it’s spurred me into writing the docs already, so got a first few paragraphs down already 😉
💯 1
And yes, you may use wildcards whereever, so
*fastapi
is perfectly fine, just to make it clear
you may look at some of the tests to see what crazy stuff you can do 😉 https://github.com/pantsbuild/pants/blob/main/src/python/pants/backend/visibility/glob_test.py#L18 like use
foo/bar/../baz
and stuff..
the .. in the middle may not be as useful, but at the start it can be crucial in making rules work well for sibling directories for instance..
that last link has an interesting case, as the rule is declared in a parent directory, but applies correctly to the sibling origin and dependency directories further down 🙂
which are parent directories to the files with the dependency link
p
Wow. The sibling directory handling is surprising but nice.
/../bar
. huh. Go figure.
c
haha, yeah, it can get hairy…
p
yeah, but siblings are hairy no matter what selector language you're using.
1
c
ok, I’m going to call it a night, but pushed what I have so far. Nothing particularly interesting yet, trying to be thorough https://github.com/pantsbuild/pants/pull/17632/files
p
Awesome stuff @curved-television-6568. Thank you! sleep well.
🙏 1
1
b
Really neat to see where this is going. Looking forward to reading the documentation of it. Thank you @curved-television-6568!!
🙏 1