https://pantsbuild.org/ logo
h

hundreds-father-404

08/11/2021, 6:16 PM
Oof. Before Poetry 1.2, it does not include
setuptools
and
wheel
in lockfiles. We need those included. But they're currently at 1.2.0a2, and it took 3.5 months for 1.1 to go from a2 to stable release Some options:
1. Use unstable release of Poetry in production.. 2. Fork Poetry to make a one-line change that fixes this? I think #2 makes most sense, but I don't know how we handle users wanting to change to different versions of Poetry. Maybe we set the default to a Git fork, and then override the help message to caution that changing the version will require using 1.2+ or a forked version?
w

witty-crayon-22786

08/11/2021, 6:30 PM
i think that given that our use of poetry is 1) internal, 2) for a brand new feature, using an unstable release is ok.
a

ancient-vegetable-10556

08/11/2021, 6:31 PM
Yeah, it’s for a new and expeirmental feature. If Poetry releasing a stable release is the only blocker on this becoming non-experimental, that seems fine to me.
h

hundreds-father-404

08/11/2021, 6:33 PM
I at first was thinking that it's okay to use their alpha release, but it seems very unstable unfortunately 😕 For example, their alpha fails to generate a lockfile for our default
pytest
due to
Invalid PEP 440 version: '3.5.'
, even though 1.1 handles it great. Looks like a bug Even tho it's an implementation detail that we use Poetry, I think users will be frustrated if Pants's lockfile generation is buggy due to issues like this
a

ancient-vegetable-10556

08/11/2021, 6:34 PM
hmm.
h

hundreds-father-404

08/11/2021, 6:34 PM
So, I think next best option is a fork against 1.1.7, w/ that one line config change? Not ideal, but seems preferable to buggy lockfile generation
a

ancient-vegetable-10556

08/11/2021, 6:36 PM
I understand there’s a reason why they mark
pip
and
setuptools
as unsafe, when you’re actually using poetry for dependency management… Does that issue affect our use case?
h

hundreds-father-404

08/11/2021, 6:36 PM
(Another wildcard is for us to distribute Poetry as a pre-built PEX. They're no longer going to be doing GitHub releases for it w/ a pre-built binary, and you can only install now via
pip
and friends like
pipx
. But then we don't have a lockfile for Poetry itself, and I think it's a chicken and egg problem to have Pants use Poetry to generate a lockfile for Poetry That would give us deterministic installs and make it easy to patch this. But now it's harder for users to change the version used in the feature)
a

ancient-vegetable-10556

08/11/2021, 6:37 PM
(disclosure: I think a fork is OK, but it’s probably best we keep the feature as experimental/only partially supported until Poetry supports it properly)
h

hundreds-father-404

08/11/2021, 6:42 PM
Does that issue affect our use case?
I don't think so. For any projects not using PEP 517, we need to have
setuptools
included. In fact, this is one of our most common issues users ask for help w/ in Pants Slack. They have a 3rdparty dep that doesn't use PEP 517 and doesn't declare a dep on
setuptools
, so they have to manually add the dependency. We need to preserve that explicit dep in the lockfile
a

ancient-vegetable-10556

08/11/2021, 6:44 PM
I was wondering specifically whether the reason why poetry skips setuptools in 1.1 is likely to break our use of poetry if we don’t skip it
h

hundreds-father-404

08/11/2021, 7:00 PM
No, I don't think it will cause issues. If my read of https://github.com/python-poetry/poetry/issues/1584 is correct, it was originally because Poetry did not vendor setuptools et al? Doesn't look to be relevant anymore, and even if it was, we only do
poetry lock
, rather than
poetry install
Fwict, only downside is we now have to maintain a fork
a

ancient-vegetable-10556

08/11/2021, 7:01 PM
IN which case, maybe we make this experimental (with a fork maintained) until Poetry 1.2 comes out, and then we do the work to make sure Poetry 1.2 works before doing a final release?
h

hundreds-father-404

08/11/2021, 7:03 PM
I really want to land lockfiles in 2.7 (at least tool lockfiles). It's bad we're not pinning tool installations right now. So I want to have a fully stable, polished experience by end of the month hopefully I think best path forward is fork 1.1.7 w/ the one line change and verify it works fine, and then use the options system so that users can upgrade to Poetry 1.2+ in the future even if using Pants 2.7 a year from now
Still the wildcard of whether we should distribute Poetry as a pre-built PEX to make its own installation deterministic and workaround the chicken and egg problem. But that's a separate, albeit related decision
Does that sound sensible? cc @witty-crayon-22786
w

witty-crayon-22786

08/11/2021, 7:06 PM
if possible, i would defer the decision of whether to fork until the project is mostly done.
do we have workarounds for either bug that would defer doing this until other things are out of the way?
also, re: pre-built PEX: i don’t think there is a chicken and egg problem: poetry is only used to generate lockfiles. if you already have one, you can still build it. so afaict, no need for a PEX if you hand-make the lockfile (or have instructions for how to do it with poetry itself)
h

hundreds-father-404

08/11/2021, 7:09 PM
We don't have workarounds. If I use 1.2, pytest lockfile fails w/ that bug. If I use 1.1, we are missing
setuptools
and
wheel
from every lockfile, meaning
setuptools_lockfile.txt
is empty 👀
i don’t think there is a chicken and egg problem: poetry is only used to generate lockfiles. if you already have one, you can still build it. so afaict, no need for a PEX if you hand-make the lockfile (or have instructions for how to do it with poetry itself)
Got it, so that mechanism will have to be a little differnt, to be clear? You manually provide the lockfile, rather than Pants running Poetry to generate it for you. Makes sense to me!
w

witty-crayon-22786

08/11/2021, 7:11 PM
only if you want to use a different version… the same would go for generating a PEX
👍 1
the default version would be locked.
re: 1.2 + pytest: is the invalid version fixed in later versions of pytest…?
h

hundreds-father-404

08/11/2021, 7:15 PM
I haven't spent a ton of time debugging why that particular bug happens, it's something about it improperly converting a
3.5.*
constraint to
3.5.
, then PEP 440 erroring. (Not sure where 3.5.* even comes from) But my takeaway from it was: this is less stable than we want for our upcoming 2.7 release. If I figure out this particular bug, I'm sure we will have others
w

witty-crayon-22786

08/11/2021, 7:20 PM
you have already found one bug in 1.1 as well, so it’s not like either of them is bug free. but yea, the discussion on that in https://github.com/python-poetry/poetry/issues/4176 is not reassuring.
is the 1.1 bug something that could be monkey-patched rather than forking for?
a

ancient-vegetable-10556

08/11/2021, 7:22 PM
The unsafe packages list was defined inside a function
so it’s not as trivial as it could be
w

witty-crayon-22786

08/11/2021, 7:22 PM
can patch the function
h

hundreds-father-404

08/11/2021, 7:23 PM
1.1's issue had been known for a long time. and it wasn't a bug, it was intentional iiuc. only we disagree w/ their intention
a

ancient-vegetable-10556

08/11/2021, 7:23 PM
Oh, it’s a class attribute, not a function constant. +1 for patching in place if we can
w

witty-crayon-22786

08/11/2021, 7:23 PM
but it’s changed/fixed in 1.2 …?
h

hundreds-father-404

08/11/2021, 7:24 PM
monkey patch how? change the PEX itself? seems risky if users depend on different versions than we expected - or we'd only patch if it's 1.1?
a

ancient-vegetable-10556

08/11/2021, 7:25 PM
Drop some extra code into the pex, yes
w

witty-crayon-22786

08/11/2021, 7:26 PM
only patch if it’s 1.1. use a script-entrypoint… something like
-c 'import thing; <http://thing.to|thing.to>_change=fixed_definition; main()'
☝️ 2
🙌 1
h

hundreds-father-404

08/11/2021, 11:43 PM
It worked! Thanks for talking about this and the monkeypatch tip! FYI I had to use
-m
and create a custom file loaded via
--sources
.
-c
looked for console scripts defined in the 3rd party deps
w

witty-crayon-22786

08/11/2021, 11:44 PM
mm.. interesting. yea, i guess PEX mismatches
python
for that flag
(tangent: … i think that if you don’t specify an entrypoint to PEX, the result is a file that just invokes the interpreter. and then you can pass
-c
at runtime)
1