https://pantsbuild.org/ logo
Powered by
w

witty-crayon-22786

10/09/2017, 5:02 PM
obviously i'd like to preserve the ability to have an implicit name for a target, as it really does help encourage 111
w

wide-energy-11069

10/09/2017, 5:10 PM
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

witty-crayon-22786

10/09/2017, 5:11 PM
Copy code
# not eligible to receive the default name
x = thing()
# eligible
thang()
@happy-kitchen-89482, @average-vr-56795... thread
w

wide-energy-11069

10/09/2017, 5:16 PM
w

witty-crayon-22786

10/09/2017, 5:20 PM
@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

wide-energy-11069

10/09/2017, 5:23 PM
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

witty-crayon-22786

10/09/2017, 5:31 PM
@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

average-vr-56795

10/09/2017, 5:33 PM
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

wide-energy-11069

10/09/2017, 5:38 PM
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

witty-crayon-22786

10/09/2017, 5:39 PM
that's the whole point of 111 though. you should create many small targets
packages, targets, directories, namespaces: all good things.
h

happy-kitchen-89482

10/09/2017, 7:23 PM
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

wide-energy-11069

10/09/2017, 7:27 PM
buildozer does a pretty good job actually, except not being able to figure out the target with an implicit name
h

happy-kitchen-89482

10/09/2017, 7:29 PM
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

wide-energy-11069

10/09/2017, 7:32 PM
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

witty-crayon-22786

10/09/2017, 7:36 PM
@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 #general. but basically: there is ambiguity to the parse if you don't have a symbol table
h

happy-kitchen-89482

10/09/2017, 7:39 PM
What does buildozer do exactly?
w

witty-crayon-22786

10/09/2017, 7:39 PM
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

happy-kitchen-89482

10/09/2017, 7:44 PM
We already know how to parse our own BUILD files. Where’s the advantage in using buildozer?
w

wide-energy-11069

10/09/2017, 7:44 PM
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

happy-kitchen-89482

10/09/2017, 8:00 PM
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

average-vr-56795

10/09/2017, 8:03 PM
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

witty-crayon-22786

10/09/2017, 8:42 PM
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

average-vr-56795

10/09/2017, 8:44 PM
We can certainly try 🙂
w

wide-energy-11069

10/09/2017, 8:46 PM
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

witty-crayon-22786

10/09/2017, 8:48 PM
@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

wide-energy-11069

10/09/2017, 8:49 PM
oic
a

average-vr-56795

10/09/2017, 8:51 PM
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

witty-crayon-22786

10/09/2017, 8:58 PM
yes
w

wide-energy-11069

10/09/2017, 9:10 PM
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

witty-crayon-22786

10/09/2017, 9:12 PM
looks good... one comment to link more directly to the 111 section
w

wide-energy-11069

10/09/2017, 9:16 PM
ty
h

happy-kitchen-89482

10/09/2017, 10:19 PM
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

witty-crayon-22786

10/09/2017, 10:21 PM
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

wide-energy-11069

10/10/2017, 3:35 AM
2 Views
Powered by