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

witty-crayon-22786

08/10/2021, 8:54 PM
a thing that might already be on the minds of lockfile folks is that we are currently in a situation where tests use one
local_only=True
lockfile resolve, and binaries use a
local_only=False
lockfile resolve.
is our expected course right now that we can do a single shared resolve for all relevant interpreters?
if so, can probably remove the
local_only
mode.
h

hundreds-father-404

08/10/2021, 9:11 PM
that was not on my mind, good point. Do you mean installing the resolve, not only generating the lock?
w

witty-crayon-22786

08/10/2021, 9:13 PM
um. if Brett is correct in the PEP and only generating the lockfile requires hitting the network, then that’s the most important part to unify
but yea… i suppose consuming the lockfile will either result in all of the wheels necessary for all interpreters, or not. so that portion might remain independent.
e

enough-analyst-54434

08/11/2021, 3:29 AM
As I pointed out elsewhere a binary that uses platforms will be challenging for any lock file generation scheme. Afaict due diligence on how lock files actually work and what guarantees they / Pants underwrite across all the Pants verbs has not been thought through.
w

witty-crayon-22786

08/11/2021, 5:01 AM
@enough-analyst-54434: Do you have time to discuss this this week?
e

enough-analyst-54434

08/12/2021, 3:53 AM
Yup. Just got back to civilization and I should have time to outline or detail all the holes I can find in a more systematic review so we don't waste more time handwaving.
h

hundreds-father-404

08/12/2021, 5:24 PM
Thanks, John. Perhaps a video chat meeting would be most conducive? (higher bandwidth)
w

witty-crayon-22786

08/12/2021, 5:25 PM
probably both.
write, read, then meet
h

hundreds-father-404

08/12/2021, 5:28 PM
Cool, fyi I'm writing up now my findings on Poetry and
--platform
into a GitHub issue
Proposal for how to robustly handle `--platform`: https://github.com/pantsbuild/pants/issues/12557 Credit to @ancient-vegetable-10556 for the breakthrough with post-processing! (And John for implementing that idea a few weeks ago)
e

enough-analyst-54434

08/12/2021, 7:58 PM
Post processing is too late for the style of case brought up by @plain-carpet-73994 here: https://pantsbuild.slack.com/archives/C046T6T9U/p1628380074163900?thread_ts=1628380074.163900&cid=C046T6T9U IOW if the latest compatible version of a package is sdist only, Poetry will rightly lock that. That will lead to no solution.
h

hundreds-father-404

08/12/2021, 8:03 PM
IOW if the latest compatible version of a package is sdist only, Poetry will rightly lock that. That will lead to no solution.
In which case Pants will eagerly error before even touching Pex, explaining that they need to get a wheel for that dep or remove the dep from use
e

enough-analyst-54434

08/12/2021, 8:50 PM
Right, the point is it was a needless error since 1 version older (say) had a published wheel.
h

hundreds-father-404

08/12/2021, 9:00 PM
Sure, then our error message could also include a tip to check if other releases of that dep do have the platform you require, right? I don't see how we could get better than that? Were we to use Pex, I don't imagine it would be querying PyPI to determine that if you used one version earlier that would work?
e

enough-analyst-54434

08/12/2021, 9:08 PM
It would, because we use Pip's
--only-binary :all:
.
w

witty-crayon-22786

08/12/2021, 9:11 PM
would still suggest a meeting now that everyone is paged in on the problem.
i’ll book something.
👍 1
h

hundreds-father-404

08/12/2021, 9:11 PM
Ah, true. To make sure I got this right, if: 1. you're using a dep w/ loose constraints, 2. and that dep's latest version does not support your platform, 3. and that dep's earlier releases within the range from #1 do support your platform then we will error when we could have instead handled it gracefully. Is that right?
e

enough-analyst-54434

08/12/2021, 9:23 PM
Exactly. I think this is likely a narrow enough corner to beg forgiveness, but it's much better to call the shot now than be bitten by not thinking through all the cases.
1
w

witty-crayon-22786

08/13/2021, 3:52 PM
is there anything i should read before the meeting?
e

enough-analyst-54434

08/13/2021, 4:10 PM
I'm still working. You guys didn't give me much time to do an exhaustive review since today is my 1st day back!
w

witty-crayon-22786

08/13/2021, 4:10 PM
sorry about that! happy to reschedule?
e

enough-analyst-54434

08/13/2021, 4:10 PM
No, I'm pretty angry about all this and best to blast through.
w

witty-crayon-22786

08/13/2021, 4:12 PM
also: to be clear the “anything i should read” question wasn’t directed exclusively at you. i’m not as up to speed as either of you, so pointers to @hundreds-father-404’s latest would be helpful too.
e

enough-analyst-54434

08/13/2021, 4:16 PM
Afaict Eric has only thought through the cases I've brought to his attention. He has not brought up any cases on his own. So the only relevant cases there you've seen IIUC.
Here's what I've come up with by looking at PythonRepos / PythonSetup / target options involved in resolves and env-var back doors:
Copy code
* Require Solution

PythonRepos:
+ --repos: No --find-links support in Poetry.

PythonSetup:
+ --requirement-constraints: Is there a way to map this concept all of the time or some of the time?

Support pip options for requirements <https://github.com/pantsbuild/pants/issues/12090>: Is the answer here that folks using pip options just don't get to use Pants generated lock files?

* Require care

There are some Pex options that affect resolves that Pants doesn't currently use (like --pre). If Pants decides to use them, care must be taken to map the Pex option to a Poetry equivalent when setting up a lock. In the `--pre` case there is a Poetry mapping of the concept that requires suffixing version with * and adding `allow-prereleases = true`.

* Solved

PexPlatformsField: We will produce locks that will cause Pex to needlessly error when building a PEX file whenever the latest compatible version of a dist is available as sdist only, but earlier compatible versions have compatible published wheels. 

* Checked

SubprocessEnvironment (Pex / Pip / requests) knobs:
+ It looks like there are no official docs calling out use of PEX_ or PIP_ to solve any requirement resolution problems.
+ Poetry uses requests so these (HTTP_PROXY, etc) should pass through.

--ca-certs-path: Can map via `poetry config --local` (i.e.: poetry.toml) or POETRY_ env var.

PANTS_ knobs: Configuring indexes with auth in url and not wanting this written in a file can be achieved with POETRY_ env vars.
w

witty-crayon-22786

08/13/2021, 4:47 PM
thank you… added to the doc
h

hundreds-father-404

08/13/2021, 11:04 PM
This is still a WIP, mostly formalizing what John already found + what I already was looking at. I will continue on Monday investigating things, including the two TODOs there https://github.com/pantsbuild/pants/issues/12568
e

enough-analyst-54434

08/16/2021, 2:07 PM
Alright - it does turn out you can get pip to do what's needed to generate Poetry / PDM style lock files. Its about 100 lines of additional patching here: https://github.com/pantsbuild/pex/blob/ffa0a513d18e2ef16895fb92f80bf0e918295dad/pex/pip.py#L334-L365
That gives you (optionally) platform independent locks, pip legacy and pip 2020 support, --find-links support, constraints support, pip args support, --platform support without crying wolf and tests locking pantsbuild.pants show slightly fatser (~10.4s lock vs ~11s lock with Poetry).
Let me know what further you want me to do if anything. To clean up my weekend hacks and do more exhaustive testing to get this added as a feature to Pex will probably be a few weeks of work.
The interesting cases I hadn't considered and that came up in my hacking were: 1. sdist resolution: Pip runs
python setup.py egg_info
(or PEP-517) to get the metadata to trace dependencies and you can't do this when you want to do a platform agnostic lock since you may not have an appropriate interpreter to run that setup.py with (some setup.py actively check sys.executable and fail fast if its version is undersired - functools32 is an exapmple). Poetry punts here and simply expects the sdist has a PKG-INFO file in it already. I expect this is ok for any modern sdist and it just dies on old ones. I had to hack Pip to do similar and use PKG-INFO if present. 2. You can't simply turn off tag evaulation and marker evaluation. For marker evaluation you must handle the
extras
marker and you must handle
python_version
and
python_full_version
- but just those three and no others.
👍 1
h

hundreds-father-404

08/16/2021, 5:24 PM
Thank you John for figuring that out. This sounds great. I'll copy what you wrote into https://github.com/pantsbuild/pants/issues/12568 and respond there so that the decision is better documented
👍 1
w

witty-crayon-22786

08/16/2021, 5:57 PM
reading both of these, my initial take is that it would be very reasonable to do this in PEX, and less risky than implementing workarounds for incompatibilities and then depending on poetry in the medium term.
h

hundreds-father-404

08/16/2021, 5:59 PM
Agreed! And likely less time to implement in Pex too, it seems. In part because Poetry hasn't been very responsive to us so it's not a guarantee we'd be able to land every change we need and cherry-pick to Poetry 1.1
w

witty-crayon-22786

08/16/2021, 6:06 PM
ok, given that:
Let me know what further you want me to do if anything. To clean up my weekend hacks and do more exhaustive testing to get this added as a feature to Pex will probably be a few weeks of work.
@enough-analyst-54434: it does seem like this would be worth pursuing, and probably more pressing than the native client work.
@enough-analyst-54434: and given that 1) this is the long pole on having lockfile support actually be stable, and 2) we’d love to get more folks involved in PEX… if you’re able to identify any portions of that work that would be delegatable to @ancient-vegetable-10556 or @hundreds-father-404 , that would be awesome too
👍 1
h

hundreds-father-404

08/16/2021, 6:09 PM
Cool, that sounds great to me. I'm happy to help however I can. In the meantime, @ancient-vegetable-10556 and I can focus on the Pants side of this project and not invest more time into the Poetry piece.
e

enough-analyst-54434

08/16/2021, 6:20 PM
So, as to not do this all again, has anyone looked into pdm yet? I have not.
h

hundreds-father-404

08/16/2021, 6:22 PM
I have not much, their lockfile support is mostly undocumented at https://pdm.fming.dev. But ack that alone is not adequate enough to justify closing the thread. I'll investigate today and open an issue documenting my findings
👍 1
e

enough-analyst-54434

08/16/2021, 6:37 PM
Ok. The two most interesting nails in the coffin would be --find-links and a resolver other than resolvelib / Pip.
I can probably check that here in a few minutes. I just wanted to see if that check had already been done.
Yeah so they do use resolvelib.
h

hundreds-father-404

08/16/2021, 6:49 PM
so far another blocker I've found is this when running
pdm export
, they warn:
The exported requirements file is no longer cross-platform. Using it on other platforms may cause unexpected result.
They reverted that feature, and we'd have to add it back. (
pdm.lock
looks to be cross-platform)
e

enough-analyst-54434

08/16/2021, 6:59 PM
Ok. I think thats +1 to my concern about the whole feature since it appears Poetry is the last man standing on that one! But agreed that iff we deem supporting that feature wise or important, then pdm is out.
h

hundreds-father-404

08/16/2021, 7:10 PM
Very plausible there are more issues. This seems like enough to not seriously consider PDM: https://github.com/pantsbuild/pants/issues/12580
I think thats +1 to my concern about the whole feature <cross-platform> since it appears Poetry is the last man standing on that one
Yeah, I do think it's important (if we can robustly support it). We know many Pants users use macOS for desktop and Linux for CI / some desktop users When updating lockfiles, ideally Linux users don't need to bother a macOS dev to also regenerate the lockfile - imagine if you had to do that whenever changing pantsbuild/pants
e

enough-analyst-54434

08/16/2021, 7:15 PM
I would never do that. I would make lockfile generation and commit a CI step.
The fact we all look past ~daily with large lockfiles is no one reads them anyway. They're collapsed in GH views for exmaple. I don't read them (Cargo.lock) anyhow! We only care tests still pass.
I won't battle to hard on this. I think dynamic dependency resolution is a lie in most ecosystems - the jvm and python for sure - but that is a belief held by almost no-one afaict and a lockfile covering all platforms is just one falsehood aspect. Clearly lots of folks like this and are fine with it.
👍 1
Alright - I'll spin up some Pex issues to write down / track what's needed here.
🙌 1
w

witty-crayon-22786

08/16/2021, 7:25 PM
Yea. Until and unless cross-platform remote execution is ubiquitous, needing to actually generate the lockfile on all platforms a repository will be consumed on would be a lot less usable than being able to cross-build. And cross-building is always a pretty powerful feature in terms of flexibility of hardware/platform choices.
e

enough-analyst-54434

08/16/2021, 7:58 PM
https://github.com/pantsbuild/pex/issues/1400 I'll likely be forking issues from there, but that's the issue that will get checked off in a Pex release.
Ok, and the major work is now all linked back to #1400 and in https://github.com/pantsbuild/pex/projects/1 Everything is blocked on #1401 but once that is complete (platform specific locks), then some parallel paths open up if others want to get involved.
👍 1
🙌 1
w

witty-crayon-22786

08/17/2021, 6:52 PM
thanks a ton.
yea, once #1401 is done, let’s talk about how Eric and Chris can get involved
👍 2