https://pantsbuild.org/ logo
Join Slack
Powered by
obviously i'd like to preserve the ability to have...
# general
w
obviously i'd like to preserve the ability to have an implicit name for a target, as it really does help encourage 111
w
hard to say. we still have many instances of a BUILD file containing a target with the default name, and then other targets with names specified. (could be because of historical reasons)
w
Copy code
# not eligible to receive the default name
x = thing()
# eligible
thang()
@happy-kitchen-89482, @average-vr-56795... thread
w
w
@wide-energy-11069: that's a legal usecase, but not really the one we're trying to make easier. the thing we're trying to make easier is: https://github.com/pantsbuild/pants/blob/5813a2d3637db9ebf7537bad09114b3c14ec1641/src/java/org/pantsbuild/junit/annotations/BUILD
w
ok that’s fair.
although for the legal case there, one may argue it is making things a bit harder to read. so I’m still neutral about optional names. whereas on the other side, if target requires a name, then there would be a lot of net positives.
w
@wide-energy-11069: it makes the encouraged case shorter and easier to read... that's the argument.
@wide-energy-11069: your example is the kind of thing we discourage... 1) multiple targets in a directory, 2) has a "catchall" target.
the latter is never necessary
a
I can definitely argue that it's a nice complement to the whole //foo/bar:bar == //foo/bar thing
(But I also find it a little jarring myself :))
w
is it mostly relevant when people are creating a target? if so, based on how many people twitter has and how many targets we have, i think creating targets probably doesn’t happen too often
w
that's the whole point of 111 though. you should create many small targets
packages, targets, directories, namespaces: all good things.
h
I will quite strongly oppose any idea that increases the amount of boilerplate in build files…
What is the problem here though?
All targets have names, they are just implicit in some cases.
I’m reasonably surprised that a bazel tool is capable of parsing, let alone manipulating, our build files. Is that the case?
w
buildozer does a pretty good job actually, except not being able to figure out the target with an implicit name
h
That still seems fragile
And possibly limits our own BUILD language
The decision “our BUILD files have to be parseable by some bazel tool” is a MAJOR one, requiring discussion, not something we should slip in out of convenience in the implementation of a relatively minor feature…
w
i wouldn’t say it is required, because internally twitter can leverage this tool once we have all our targets name specified. it is more for the oss community to benefit from this
I understand where the fragility concern comes from, but in practice we haven’t found any issue yet
also it’s meant to be a refactoring tool, so potentially some manual tweaking is expected
w
@wide-energy-11069: "once we have all our targets name specified"... we should not do this.
again, we want twitter to head in the direction of 111, and this would be a step back there.
@happy-kitchen-89482: the concerns are higher in this thread, and overflowed a bit in #C046T6T9U. but basically: there is ambiguity to the parse if you don't have a symbol table
h
What does buildozer do exactly?
w
i think there are reasonable heuristics that we could follow in a patch upstream. they're not completely bulletproof, but @average-vr-56795 pointed out that the current heuristic ("anything with a 
name
kwarg is a named rule") isn't either
h
We already know how to parse our own BUILD files. Where’s the advantage in using buildozer?
w
build file rewriting/refactoring
say you want to rename a target, then all its depeedee’s dependencies need to be changed https://github.com/pantsbuild/pants/issues/4845
h
OK, that logic doesn’t seem hard to replicate. We already know how to query our build graph. I don’t think we know how to write out targets, but that wouldn’t be hard, I suspect.
I just smell danger in trying to use a tool that wasn’t designed for Pants BUILD files and that can break at any time.
a
FWIW buildozer is also in use at Facebook for Buck's BUILD files
And the Bazel folks are pretty excited about the idea of having some commonality and being able to work on ecosystem tooling together
(I agree that it's definitely a topic for larger discussion)
Definitely, the problem of automatically modifying BUILD files is one that feels trivial, but gets annoying complex when you want to do things like preserve comments. If we can share (and contribute to) common tooling, there's value to be had there. For sure that shouldn't be at the expense of other things that we think are valuable, but it's worth spending a little time finding the right trade-off 🙂
w
if it's true that they're excited about commonality, then it seems like we could lean on that to get them to accept a (reasonable) patch to support implicit names
a
We can certainly try 🙂
w
Going back to the issue @witty-crayon-22786, it’ll still be a problem that the tool wouldn’t be able to tell which one is a function and which is a target, right?
without an explicit symbol tables that is
w
@wide-energy-11069: no... i gave an example heuristic in general, and in here.
only top-level symbols with no assignment would be eligible
(er... only in here.)
Copy code
# not eligible to receive the default name
x = thing()
# eligible
thang()
w
oic
a
So the heuristic being: If there is exactly one top-level function call with no name, treat it as if it has the name of $(basename $(dirname $file)), else error
?
(When trying to address that particular target, and simply ignore if you're not trying to modify that target)
w
yes
w
put up a draft to file against buildozer, let me know if i summarized things correctly. https://docs.google.com/document/d/15lw3GArcGsm5CAi2lGKNB_AUTgaOJPCRP2TcTvS1Cos/edit
w
looks good... one comment to link more directly to the 111 section
w
ty
h
Keep in mind that the ultimate goal here is no BUILD files at all, at least in the common case. I really don’t want to step backwards there…
w
eh... i think where we left that was that it isn't possible to not specify the language type at the very least. but yes... just
Copy code
$ cat BUILD
scala_library()
hopefully
w
2 Views
Open in Slack
PreviousNext
Powered by