https://pantsbuild.org/ logo
Join Slack
Powered by
Related, <@U06A03HV1> probably don't yet deprecate...
# development
h
Related, @witty-crayon-22786 probably don't yet deprecate 
[python].requirement_constraints
until we have landed local requirement support for Pex?
major "benefit" of constraints files is that it does not need to be exhaustive
e
What do you mean by landed? 2.13.0 ships?: https://github.com/pantsbuild/pex/releases/tag/v2.1.92
The functionality is landed in 2.13.x and 2.14.x.
h
Specifically that we need to wire up 
--path-mapping
, right?
which is probably worth us doing before 2.13, it's valuable for users to have sooner
e
What's the evidence for that? This is adding new features to a rc branch. I know we apparently do this now, but I don't want to take part in that.
IIRC there may have been actual evidence? An actual request for this? But it would be good to re-affirm there is a demand before shoehorning in an already-RC'd IMO.
h
I recall some users the past 2 months wanting to use local requirements with lockfiles and not being able to. But you raise a good point. Thank you for the caution 🙂 Before deciding on cherry-pick, I will try to get something up this afternoon so we can assess how risky it is
e
I mean, I've raised this point alot. I'm not sure how good it is! But thanks for being cautious in this case.
❤️ 1
h
In the meantime, the workaround is to host the files in a private repository / index and load it with 
[python-repos]
.
From our docs on local requirements. Given that there's an (awkward) workaround, sounds good to not cherry-pick
So given that local requirements will soon be supported, thoughts on removing 
[python].requirement_constraints
and pushing people to 
[python].resolves
? John has pointed out we're in a bad place because requirement constraints are genuinely useful, including when generating lockfiles, to force (transitive) deps to a particular value. The issue is we abused them to act like lockfiles, and now we can't safely retcon the semantics w/o breaking deprecation policy I would like to see requirement constraints support as pip intended it to be. I see two options: 1. Deprecate the old abused semantics. Once fully removed, add the option back, but with the different semantics 2. Add a new option now but with a different name. Still probably deprecate the old abused semantics, but we don't need to cc @happy-kitchen-89482
Ah, alternatively...
[python].resolve_all_constraints
is the real problem I think. We could: 1. Keep 
[python].requirement_constraints
forever 2. Hook up 
[python].requirement_constraints
to 
[python].resolves
, whereas right now they are mutually exclusive 3. Deprecate 
[python].resolve_all_constraints
e
Requirement constraints are at least a per-resolve thing; so the configuration should support that. More usefully, thinking about actually constraining things, they are probably a per-requirement thing. IOW, you often want to constrain a transitive dep of - say - pandas. If Pants let you do so (~ 
python_requirement("pandas", constraints=["requests==2.0.0"])
) that constraint should to apply to all resolves "pandas" was a part of. I absolutely do not care about spelling, I just want to try to get a complete picture of actually using constraints means to an end user before we / in case we get off track.
h
Requirement constraints are at least a per-resolve thing; so the configuration should support that.
Ah, thank you! That was one of the big things I was going to ask for input on: whether the constraints file should apply to tool lockfiles too
IOW, you often want to constrain a transitive dep of - say - pandas. If Pants let you do so (~ python_requirement("pandas", constraints=["requests==2.0.0"])) that constraint should to apply to all resolves "pandas" was a part of.
The resolve must have only one entry per dep, though, right? (Outside of env markers like 
; sys_platform
) So this would invite an error where dep A pins pandas to ==2.0, but dep B pins to ==3.0 Constraints-file-per-resolve makes sense to me
e
I consider that spelling - too far in the weeds. I just want to expand the discussion to the user level considerations.
👍 1
If I'm a user, I want to be able to squash pandas crazy deps.
Pants may or may not let me do that.
Pants may forces me to squash pandas crazy deps in a larger constraints file with a comment calling out why that one.
Not awesome, but OK.
h
So, whatever the spelling, it sounds like we want a file that is input to a 
lock
command, that constrains non-immediate deps. So at a first pass this could be a field on 
python_requirement()
, that you specify with overrides, and it's up to you to not specify conflicting constraints (which is true no matter what format we pick I suppose)
h
@happy-kitchen-89482 fyi this raises questions for me if we should have done 
--no-binary
as a per-resolve option? It seems easy to contrive a scenario where you need that; resolve A uses Django 1 which can't use binary deps, but resolve B uses Django 3 where it's safe to I think when you added that option, resolves were still pretty experimental. Now, it's converging to be the main way to use Python reqs. So it's less of a problem to only support the mechanism when using resolves
h
Alternatively, lockfiles should be first class BUILD file objects with properties, and the name of a lockfile should be its address...
h
yeah possibly. A very early design of tool lockfiles in Pants 2.7 used targets, then we realized it would be less boilerplate and less confusing to use the options system
But regardless of the spelling: 
--binary
should per-resolve, rather than global. Thoughts?
e
There are 0 thoughts involved here! 
--binary
simply is per-resolve. If Pants makes it global, that's Pants limiting things. Is there a good reason to do so? Super unlikely.
h
It is currently global: I am proposing changing it to be per-resolve, and checking in with Benjy as the original author if he agrees
h
Yes, that makes sense
But let's take time to get the spelling right ? We now have all sorts of inputs to lockfile generation that may make sense to belong in a BUILD file rather than config
h
We now have all sorts of inputs to lockfile generation that may make sense to belong in a BUILD file rather than config
Note that this option + new constraints option both should operate on tool lockfiles too. Whereas 
[python].resolves_to_interpreter_constraints
is solely for 
[python].resolves
. This implies that both tool lockfiles & user resolves will need to migrate to being targets: possibly a bigger change than you were thinking?
h
I have no interest in big changes, I'm just pointing out that a loose string being the only thing that identifies a resolve, and then having config identify itself as belonging to that string, is pretty error-prone and janky
But maybe we're too far down that path to change it
It also fights against what appears to be a common case of people bringing a bunch of disparate projects into a monorepo, and wanting each project to have its own resolve, in that we force resolves to be centrally defined
h
in that we force resolves to be centrally defined
Yeah, that was a very intentional design decision from Stu and me: only admins should be defining new resolves, or at a minimum, they should be pinged. This is the same argument for named interpreter constraints: https://github.com/pantsbuild/pants/issues/12652
is pretty error-prone
Eh, we have code that actively validates all the strings are recognized and gives a good error message if it's not. I don't see how it's any jankier than having to use a target address
h
Yeah, maybe not
Open in Slack
PreviousNext
Powered by