https://pantsbuild.org/ logo
Join Slack
Powered by
<@U021C96KUGJ> any suggestions on how to organize ...
# development
h
@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
Why yes, 
coursier_fetch.py
is too big and complicated (https://github.com/pantsbuild/pants/pull/14164)!
➕ 1
h
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
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
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
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
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
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
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
?
Open in Slack
PreviousNext
Powered by