https://pantsbuild.org/ logo
Powered by
e

enough-analyst-54434

07/20/2021, 5:46 PM
What's the status of lockfile generation for Pants? 
./pants lock ::
doesn't work due to 
testprojects/
afaict (due to the 
badreq
req) and running with 
./pants lock src:: tests::
completes, but produces surprising results (with only a pex 2.1.42 -> 2.1.43 change, two other requirements, pystache and types-requests - are removed completely from the lockfile). Afaict there is no script or doc or comment in the generated lockfile that give a hint at what command to run to generate the lockfile.
h

hundreds-father-404

07/20/2021, 5:47 PM
./pants --tag=-lockfile_ignore lock ::
Sorry. I should have manually updated the checked in lockfile's header to say how to run this command -- I'll put up a patch now
e

enough-analyst-54434

07/20/2021, 5:49 PM
Ok, ty. I'll absolutely never remember that and no-one will remember to hand edit the generated lockfile header. I'll look into something more automated.
I think the hand edit is not a good idea.
h

hundreds-father-404

07/20/2021, 5:50 PM
I'll absolutely never remember that and no-one will remember to hand edit the generated lockfile header. I'll look into something more automated.
That work is already underway: https://github.com/pantsbuild/pants/projects/20#card-64990418 Things are awkward in between, but only pantsbuild/pants is experiencing this because lockfiles are all feature gated behind 
pants.experimental.python
e

enough-analyst-54434

07/20/2021, 5:51 PM
Sure, I understand the experimental only Pants bit.
Um, ok - great - tracked. I might pluck that then since its tripping me up.
As to the req nukes - those are disturbing and more important, I'll dig into that presently.
h

hundreds-father-404

07/20/2021, 5:54 PM
I would encourage not spending your time on an automated fix - imho, your time is more valuable elsewhere like native client. That header generation is one of the less tricky parts of this project, only that it requires time investment. Anyone can do it, it doesn't require your specific wisdom/skills How about I manually update each lockfile w/ how to regen for now? Ack that that's not sustainable: it's meant to staunch the bleeding
As to the req nukes
because you left out 
build-support::
e

enough-analyst-54434

07/20/2021, 5:54 PM
What does wisdom have to do with it? Any dev should take on any task.
It blocks me, I should fix - no?
Ok, great - as you say, using the tag exclusion which includes build support removed the disturbing.
👍 1
w

witty-crayon-22786

07/20/2021, 5:58 PM
yea, there are changes landing to this code daily, pretty volatile. as long as Eric is tracking it, can expect more changes soon.
h

hundreds-father-404

07/20/2021, 5:59 PM
there are changes landing to this code daily, pretty volatile.
Indeed. For example, that lockfile generation code is about to see some disruptive changes to handle multiple interpreter constraints
e

enough-analyst-54434

07/20/2021, 6:00 PM
Ok ... I am baffled. We use git to handle churn. I've never herd of locking people out of construbutiing. But I can back off if you insist.
w

witty-crayon-22786

07/20/2021, 6:01 PM
obviously blocking people is no good. but in service of landing incremental changes, a bit of volatility (on a spectrum, of course) can be preferable to huge PRs that implement the whole world
e

enough-analyst-54434

07/20/2021, 6:04 PM
Ok, I guess I had no clue what you and Eric's last two comments were about. It now appears you were justifying landing without the header support and not asking me to back off with those two comments?
I'm always on board with the incremental thing.
w

witty-crayon-22786

07/20/2021, 7:39 PM
sorry for the confusion: i think it was more that Eric likely already has a few PRs in flight for those codepaths, and collisions were likely.
e

enough-analyst-54434

07/29/2021, 5:39 PM
@hundreds-father-404 what is the extra magic needed to get 
./pants run build-support/bin/generate_all_lockfiles.py
working when actually bumping a requirement in 3rdparty/python/requirements.txt. When I bump I get an error like:
Copy code
Failed to resolve requirements from PEX environment @ /tmp/process-executionSw7SFG/lockfile.pex.
Needed manylinux_2_33_x86_64-cp-37-cp37m compatible dependencies for:
 1: ansicolors==1.1.7
    But this pex only contains:
      ansicolors-1.1.8-py2.py3-none-any.whl
 2: pex==2.1.44
    But this pex only contains:
      pex-2.1.43-py2.py3-none-any.whl
There I bumped Pex from 2.1.43 -> 2.1.44 and later added an artificial dump? of ansicolors from 1.1.8 -> 1.1.7 to sanity check this wasn't a Pex-is-special thing.
If there is no magic, I can go ahead and hand edit the lockfile for now.
Ok, a hand edit did the trick. I'll roll with that. That said, it looks like a bug that distributions not actually resolved are being included in the lockfile. That seems like a supply-chain bug. I had to edit a hash for both the whl (actually used) and the sdists (not actually used - so not vetted and could be an attack). I don't see an issue covering this but can file.
h

hundreds-father-404

07/29/2021, 5:54 PM
Hm there should be no magic..changing the input requirements like you did should cause the lockfile to be regenerated when rerunning 
./pants run build-support/bin/generate_all_lockfiles.py
, and hand edits shouldn't be necessary for 
user_reqs.txt
I'm trying to understand what this issue is - definitely filing on GH would be appreciated. I'm not expecting any of the behavior you're saying
e

enough-analyst-54434

07/29/2021, 5:56 PM
Ok, it should repro. I'll file for the regen issue.
What about the other security issue of including unused sdists for example?
I can file that as well.
h

hundreds-father-404

07/29/2021, 5:57 PM
Meaning pip-compile includes hashes for all release files for that version, even if only wheels are used?
e

enough-analyst-54434

07/29/2021, 6:00 PM
Yes.
There is nothing about PyPI or any other package registry that assures the wheels were built from the sdist for example. So it seems to me pip-compile produces broken lockfiles that rely on goodwill and or not messing up uploads.
I'll file to track this. FWICT poetry has the same bug.
... and the ~defunct Pipfile.lock. Maybe I'm missing something but these all seem to be relying on good will / proper uploads. Perhaps Pants own goal of also mitigating supply chain attacks should not be generally conflated with a lockfile.
The lockfile gen issue: https://github.com/pantsbuild/pants/issues/12457 I'll see if I can't shave that yak later today,
h

hundreds-father-404

07/29/2021, 6:39 PM
Iiuc, pip-compile/Poetry/Pants are making a tradeoff. You generate on a single platform, but may use that lockfile on others If I generated a lockfile with pantsbuild.pants wheel on my M1, it would only include the hash for the macos_arm64 wheel, even though people using the lockfile need the other wheels' hashes. Trading off some security for flexibility to use the lockfile on multiple platforms. Does that sound right?
e

enough-analyst-54434

07/29/2021, 6:54 PM
I think that's what these lock file schemes think they're doing - yes. I filed https://github.com/pantsbuild/pants/issues/12458 without the bug label since what we want to do - if anything is definitely debatable.
Cargo is interesting to compare since it smartly mostly avoids the problem by compiling everything from source.
You could still add malicious or just buggy conditionally compiled code in a rust crate of course.
I may be missing something on the mixing of the desire to have lockfiles for stable resolves and the desire to have lockfiles to avoid supply chain attacks. @happy-kitchen-89482 has thought most about the supply chain side.
h

happy-kitchen-89482

07/29/2021, 7:18 PM
As you've observed elsewhere, lockfiles only help with supply chain attacks in that you're guaranteed to resolve the same thing every time. There is an implicit assumption that youv'e vetted that one thing to be safe. In practice...
e

enough-analyst-54434

07/29/2021, 7:52 PM
The problem though is one thing expands to N here. The sdist plus N-1 wheels. I need to read pip-compile code, but it's not clear how they even decide to pick all N. If they resolve just one (say the sdist) and then just assume all wheels at the same version provide valid solutions too ... that's incorrect depending on env markers.
Powered by