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

hundreds-father-404

01/20/2022, 6:20 PM
@ancient-vegetable-10556 any suggestions on how to organize JVM lockfile code better? 🧵
As discussed in DM, I'm changing it so that to run
coursier fetch
you use
Get(CoursierResolvedLockfile, GenerateJVMLockfile)
rather than
Get(CoursierResolvedLockfile, ArtifactRequirements)
. That causes a cycle between
jvm/goals/lockfile.py
and
jvm/resolve/coursier_fetch.py
. I'm thinking it might make sense to move that coursier code into
goals/lockfile.py
. We do still have a lot of other code for how to build a
ClasspathEntry
for user code. That seems to make sense not living in
goals/lockfile.py
, maybe staying where it is
a

ancient-vegetable-10556

01/20/2022, 6:23 PM
Why yes,
coursier_fetch.py
is too big and complicated (https://github.com/pantsbuild/pants/pull/14164)!
1
h

hundreds-father-404

01/20/2022, 6:27 PM
Definitely agreed. Maybe what we should do is: 1. move the definition of
CoursierResolvedLockfile
(which is huge) into that
common.py
file 2. move the rule to go from
GenerateJvmLockfile -> CoursierResolvedLockfile
into
goals/lockfile.py
. Note that no one needs to import that rule 3. maybe rename
coursier_fetch.py
to somehow express it's about user code &
ClasspathEntry
. It's not actually doing any fetching. It's the user-code version of
jvm_tool_base.py
a

ancient-vegetable-10556

01/20/2022, 6:29 PM
Let me refamiliarise myself with the code
So point 3 is not quire right —
coursier_resolve_lockfile
in
coursier_fetch.py
is the piece of code used to generate both user and tool lockfiles;
coursier_fetch_one_coord
actually fetches the remote artifact when consuming the lockfile. Both use
coursier fetch
under the hood (though the script that says
fetch
is in
coursier_setup.py
)
note that
JvmToolBase
doesn’t call coursier itself. That is delegated to
coursier_fetch
h

hundreds-father-404

01/20/2022, 6:41 PM
To clarify, step 2 would be moving the rule
coursier_resolve_lockfile
into
jvm/goals/lockfile.py
. This will be necessary for us to change the rule to be
GenerateJVMLockfile -> CoursierResolvedLockfile
rather than
ArtifactRequirements -> CoursierResolvedLockfile
, because we define
GenerateJVMLockfile
in
lockfile.py
so would have an import cycle
a

ancient-vegetable-10556

01/20/2022, 6:42 PM
OK, that makes sense
🙌 1
I think more generally, we should try and split out the concepts of fetching artifacts vs resolving the lockfile.
CoursierResolvedLockfile
is – generally speaking – more generic than is justifiable (compared with how Pants likes to use types, anyway)
1
The resolve classpath entry union certainly helps avoid using it directly in a lot of places, but it’s clearly still confusing
h

hundreds-father-404

01/20/2022, 6:45 PM
Agreed. Still thinking about how to do that....don't hate me for this, possibly wrapper types hehe
CoursierResolvedLockfile
works really well for what it claims to do: an in-memory representation of a lockfile, regardless of how you got it. Where it gets confusing is that there are multiple ways to get that type via the engine
a

ancient-vegetable-10556

01/20/2022, 6:45 PM
Wrapper types are fine, given that we don’t really have a way to flag usage any other way
👍 1
we lean heavily into using Python types, and wrappers are the tool we get out of that
1
(hammer/nail etc)
h

hundreds-father-404

01/20/2022, 7:25 PM
Alright I think this refactor works nicely. It leaves
coursier_fetch.py
with only code related to user requirements, whereas
goals/lockfile.py
is highly generic code for resolving/generating a lockfile with Coursier. Like how
python/goals/lockfile.py
has that but with Poetry Only part I'm stuck on is what to rename
coursier_fetch.py
. Maybe
jvm/resolves/user_resolves.py
?