https://pantsbuild.org/ logo
#development
Title
# development
f

fast-nail-55400

12/06/2021, 10:41 PM
thread re jvm tool lockfile path in pants source (https://github.com/pantsbuild/pants/pull/13811#discussion_r763288383)
what is the preferred path for such lockfiles? cc @witty-crayon-22786 @hundreds-father-404 @ancient-vegetable-10556
the comment suggests:
3rdparty/(jvm|tool)/${name}.lockfile
so maybe
3rdparty/jvm/tool/${TOOL}.lockfile
?
so that we can still use other
3rdparty/jvm
subpaths as we see fit
w

witty-crayon-22786

12/06/2021, 10:44 PM
the
(jvm|tool)
was sortof meant to suggest either-or… i think the question there is whether there is any benefit to namespacing lockfiles by platform… there may not be
in which case
3rdparty/tool/black.lockfile
is fine too. there will be subsystem naming helping to enforce encourage no collisions here
h

hundreds-father-404

12/06/2021, 10:45 PM
I suggest that you use default tool lockfiles right next to the tool itself. For Black, we have
src/.../lint/black/lockfile.txt
The
3rdparty/
pattern is only for when we use a custom lockfile different than the default. Which iiuc should never happen in pantsbuild/pants - no reason to not use the default given how little JVM code we have
w

witty-crayon-22786

12/06/2021, 10:46 PM
@hundreds-father-404: this is for the default value of the option: i.e., where we suggest to put the lockfile if you’ve set a custom version
f

fast-nail-55400

12/06/2021, 10:46 PM
I prefer Eric’s suggestion, which fits with the
resource
target for the lockfile being next to the same code it is for.
@witty-crayon-22786: the default option value will now just be
<default>
w

witty-crayon-22786

12/06/2021, 10:47 PM
er
h

hundreds-father-404

12/06/2021, 10:47 PM
Oh? Why would we be setting the default value for where the lockfile of a custom version should be? In Python, you use either
"<default>"
or you choose whatever arbitrary path you want I don't want to be imposing a default location on folks
f

fast-nail-55400

12/06/2021, 10:47 PM
and the existing generate-lockfiles goal will skip lockfiles where the path is
<default>
w

witty-crayon-22786

12/06/2021, 10:47 PM
but
The 
3rdparty/
 pattern is only for when we use a custom lockfile different than the default.
is exactly the relevant case here. this option comes into play when an end user needs to actually generate a lockfile
f

fast-nail-55400

12/06/2021, 10:48 PM
right, then they need to set the lockfile location explicitly
1
h

hundreds-father-404

12/06/2021, 10:48 PM
to whichever arbitrary path they want
w

witty-crayon-22786

12/06/2021, 10:48 PM
why though?
f

fast-nail-55400

12/06/2021, 10:48 PM
because how else should Pants know to prefer the resource over the explicit path?
coke 1
h

hundreds-father-404

12/06/2021, 10:48 PM
I don't see how else it would work? We need a way to say you want to use the default, which is currently the magic string
"<default>"
. Otherwise, we have to assume it's a custom lockfile
w

witty-crayon-22786

12/06/2021, 10:49 PM
because the specified version doesn’t match the builtin version
h

hundreds-father-404

12/06/2021, 10:49 PM
If that happens, then we should eagerly error and say you need to explicitly set the option to whatever location you want. Or, use the default version
f

fast-nail-55400

12/06/2021, 10:49 PM
we don’t match a specified version to the version used to generate the default lockfile
w

witty-crayon-22786

12/06/2021, 10:49 PM
as it stands, you have to set two things whenever you change the version: you change the version, and then we say “ah ah ah! you forgot to choose a lockfile location.”
h

hundreds-father-404

12/06/2021, 10:49 PM
This is what Chris's validation code does for Python. We validate that it's safe to use the default, else generate an awesome error message
you have to set two things whenever you change the version: you change the version, and then we say “ah ah ah! you forgot to choose a lockfile location.”
Yep! That's exactly what we want imo! You are opting out of the defaults
w

witty-crayon-22786

12/06/2021, 10:50 PM
…twice
f

fast-nail-55400

12/06/2021, 10:50 PM
I am fixing up
JvmToolBase
currently. Just going to have the paths be the existing resource file locations. If we want to make further changes, they can be a separate PR.
w

witty-crayon-22786

12/06/2021, 10:50 PM
i don’t see why you need to opt out twice (with two different options)
h

hundreds-father-404

12/06/2021, 10:51 PM
Because you are changing different things with those two options...one is the path for your lockfile, one is the version to use the tool. They're coupled, but not the same thing
I don't think we should be assuming a default lockfile path to write to
w

witty-crayon-22786

12/06/2021, 10:51 PM
i do. ~every other tool does.
cargo, poetry, go, etc
npm, yarn, etc
h

hundreds-father-404

12/06/2021, 10:52 PM
Those also aren't optimized for monorepos and multilingual repos. And they have a single lockfile (ack on Cargo and workspaces)
☝️ 1
a

ancient-vegetable-10556

12/06/2021, 10:53 PM
I think it’s really important for Pants’ usecase that each tool have dependencies that don’t have to change in lockstep with each other
w

witty-crayon-22786

12/06/2021, 10:53 PM
sure, and people will have to name lockfiles if they create more than one. but i don’t think that’s a good argument for needing to choose filenames for all of them
h

hundreds-father-404

12/06/2021, 10:53 PM
Setting up a custom lockfile path is a one-time thing done by a codebase admin. For example, Asher set up Toolchain to never use the defaults. Now that that's done, he never needs to worry about it Minimizing boilerplate here seems like the wrong goal fwict. More important is being explicit rather than trying to guess where you want the default
w

witty-crayon-22786

12/06/2021, 10:54 PM
Asher set up Toolchain to never use the defaults. Now that that’s done, he never needs to worry about it
hm, no: only half of them are. i made changes to more of them, and some tools still remain as defaults
a

ancient-vegetable-10556

12/06/2021, 10:54 PM
TO be clear, we’re talking about tool lockfiles here, right?
w

witty-crayon-22786

12/06/2021, 10:54 PM
yes, tool lockfiles. user lockfiles will need names, although i think that we should add a default location for the first lockfile for each language as well
h

hundreds-father-404

12/06/2021, 10:54 PM
I don't want people to think that they're forced to use a particular setup or reorganize their code. That's already something we are trying really hard to counteract in our docs If you're incrementally adopting Pants and don't use a
3rdparty/
convention, I don't want people to misunderstand in thinking they need to because that's what Pants defaulted to
w

witty-crayon-22786

12/06/2021, 10:55 PM
i am fairly confident that anyone who is disgruntled about the (default) location of the lockfile will be able to find out that you can change it.
h

hundreds-father-404

12/06/2021, 10:56 PM
All in favor of the
help
message saying something like "A good convention is to put this lockfile in a folder named `3rdparty/jvm`". But making it the default behavior makes it seem like we are forcing you into it imo, and you might not realize it's even an option because why would you bother with it?
w

witty-crayon-22786

12/06/2021, 10:58 PM
i think that having a default is preferable. the alternative is boilerplate in
pants.toml
, and boilerplate in the help
people who don’t care where that file is located (which i expect will be 98% of them) shouldn’t ever need to read the help for that option.
(not to mention that we can delete a lot of handling code for “you forgot to set option X”)
h

hundreds-father-404

12/06/2021, 11:00 PM
Oh hm, I was thinking about how this default will actually work. We can't automatically generate the lockfile as a side-effect yet, you have to run the relevant goal What would that look like with a default path? We eagerly error when consuming the lockfile and tell you to run the goal like normal, then when you run it it uses the default path?
w

witty-crayon-22786

12/06/2021, 11:00 PM
correct
h

hundreds-father-404

12/06/2021, 11:01 PM
(not to mention that we can delete a lot of handling code for “you forgot to set option X”)
Almost all the code is checking whether the lockfile is valid or not. It's not checking if the option is set. I don't think that argument is very compelling tbh
w

witty-crayon-22786

12/06/2021, 11:04 PM
DEFAULT_TOOL_LOCKFILE
is mentioned 19 times in 5 files (and hardcoded in 2 more)
f

fast-nail-55400

12/06/2021, 11:05 PM
https://github.com/pantsbuild/pants/pull/13822 fixes the immediate brokenness
h

hundreds-father-404

12/06/2021, 11:07 PM
Thanks, Tom. Sounds like we all agree with using
<default>
as a magic string
w

witty-crayon-22786

12/06/2021, 11:08 PM
i agree that we should land https://github.com/pantsbuild/pants/pull/13822 to fix a bug / make things consistent. but i still think that we should add a default for all of these in the medium term, rather than using the
<default>
magic string
f

fast-nail-55400

12/06/2021, 11:08 PM
it maintains the status quo to use a magic string. but those options could have been
str | None
and used
None
as a default
the python options are actually tri-states since they also can use the second magic string
<none>
👍 1
I chose to require a lockfile for JVM tools and left out support for
<none>
1
w

witty-crayon-22786

12/06/2021, 11:10 PM
yea, although the
<none>
case is actually necessary i think
since we don’t have a facility for setting an option to the python literal
None
f

fast-nail-55400

12/06/2021, 11:11 PM
it is easy to add. instead of requesting a classpath using a lockfile, the artifact requirements can be used, as we were doing prior to introducing tool lockfiles there
also
<none>
!= None (i.e.,
<default>
)
not specifying the option means use the default embedded lockfile;
<none>
means don’t use a lockfile whatsoever and re-resolve each time
👍 1
w

witty-crayon-22786

12/06/2021, 11:24 PM
one last thought on this for now though: another reason why i think that we should set a default is because of user resolves: https://github.com/pantsbuild/pants/issues/12742
if we don’t have a default for user resolves, every repo will be forced to set one at creation time (which is more obviously unnecessary boilerplate than tools are)
since we will need that convention, it’s another reason for tools having one too
h

hundreds-father-404

12/06/2021, 11:27 PM
yeah that is true on user lockfiles. A few months ago I accepted that as acceptable for initial one-time setup I don't want to die on this hill tho, I'm not blocking with having a default lockfile path
Oh @witty-crayon-22786, very related is https://github.com/pantsbuild/pants/issues/12742. I think that using
"default"
as the name of the default resolve is a non-starter when we merge Python and Java to use the same resolve universe We could use default names like
py-default
and
jvm-default
, although I continue to lean a bit more in favor of approach #2 that you have to give an explicit name
altho I'm willing to rethink that preference. I do recognize that naming things is really hard, so asking you to come up with a name for a lockfile when you just want to try out Pants is an additional barrier to adoption..
f

fast-nail-55400

12/06/2021, 11:32 PM
@hundreds-father-404: re https://github.com/pantsbuild/pants/pull/13777#discussion_r763489820,
Want me to handle merging the two goals? I’m now working on lockfiles as my main project so the context switch isn’t bad for me
yes. I don’t have the context across both JVM and Python lockfiles to figure out what we want here and I don’t have a particular viewpoint, other than having an easy way to make JVM lockfiles which is what I built thus far.
👍 1
and those who have particular thoughts here can figure out the agreed-upon way forward 🙂
w

witty-crayon-22786

12/06/2021, 11:34 PM
I do recognize that naming things is really hard, so asking you to come up with a name for a lockfile when you just want to try out Pants is an additional barrier to adoption.
yea. and i think that even if the default name is more awkward, we should not ask users to do this. as mentioned before, i think that 98% of users will be fine with a default, and never think twice about changing it (as evidenced by lockfiles in ~all of the other tools)
👍 1
h

hundreds-father-404

12/06/2021, 11:36 PM
Okay. I'll incorporate into my lockfile plan looking into what a default path would look like Still scoping the best way to approach this project, whether I start with: 1) unifying everything into
generate-lockfiles
2) hooking up
compatible_resolves
3) switching to pex for lockfile generation 4) Stu's proposal for disambiguating dep inference
Coming full circle...I continue to think it's a good idea for tool lockfiles to behave the same as now, where you either use the magic string
<default>
or you set a custom path. That has worked really well for Python it seems But for user lockfiles, I agree that we should set a default like
{"jvm": "3rdparty/jvm/artifacts.lockfile"}
. That's necessary for Pants to work out-of-the-box -- What should that default path be? I don't know what's idiomatic in JVM
w

witty-crayon-22786

12/16/2021, 11:28 PM
yea, that makes sense.
there is no idiom for lockfiles on the JVM
range resolves are rare, which is one of the primary motivating factors (obviously there are others, but that’s one of the big ones in other ecosystems)
👍 1
h

hundreds-father-404

12/16/2021, 11:31 PM
k. I like the
3rdparty/jvm
part, but any opinion
artifacts.lockfile
part? In Python, we're using
.txt
-- also why are we doing
.lockfile
instead of
.json
?
w

witty-crayon-22786

12/16/2021, 11:31 PM
so i think that something like
3rdparty/jvm/artifacts.lockfile
should be fine.
re the lockfile suffix: i don’t know. it could be
json
, but since it’s not actually meant for hand-editting, the need for syntax highlighting is reduced a bit
👍 1
h

hundreds-father-404

12/16/2021, 11:34 PM
how about
{jvm: 3rdparty/jvm/lockfile.json, py: 3rdparty/py/lockfile.txt}
?
w

witty-crayon-22786

12/16/2021, 11:34 PM
sure.
a

ancient-vegetable-10556

12/16/2021, 11:34 PM
I tend to disagree — the JSON here is actually encoding for a file format, it’s not arbitrary JSON
h

hundreds-father-404

12/16/2021, 11:35 PM
Meaning, you are +1 or -1 for ending in
.json
?
a

ancient-vegetable-10556

12/16/2021, 11:35 PM
I am -1 for ending in
.json
if it’s not arbitrary JSON
h

hundreds-father-404

12/16/2021, 11:37 PM
Oh hm, Rust and Yarn both end in
.lock
. Sg But
3rdparty/jvm/lockfile.lock
is redudant. Any suggestions?
artifacts.lock
seems fine other than upsetting non-American English speakers 🙈
a

ancient-vegetable-10556

12/16/2021, 11:38 PM
We might want to go with
.pantslock
or
.jvmlock
artifacts.lock
is not correct, as it also contains dependencies of artifacts 😄
👍 1
w

witty-crayon-22786

12/16/2021, 11:38 PM
i think that putting
default
in the name might make sense. so
default.lock
maybe. because users using multiple resolves will need to choose other names for the rest
1
a

ancient-vegetable-10556

12/16/2021, 11:38 PM
(which are artifacts, but not ones that are declared or accessible to anything)
h

hundreds-father-404

12/16/2021, 11:39 PM
k, then should the resolve be called
jvm-default
or just
jvm
?
I guess if it's the default, you're not writing it out very frequently. So the extra characters isn't that offensive. Meaning I think
jvm-default
👍 1