In v1,

interpreter_constraints

on a

pex_binary

target would override the transitive closure. In v2, we "fixed" it to instead act like all other interpreter constraints, to be mixed in. The practical impact is described in https://www.pantsbuild.org/docs/python-interpreter-compatibility#using-multiple-python-versions-in-the-same-project

This means that every dependency of a target must also be compatible with its interpreter constraints.

Thoughts if that was a mistake or not?

Before answering that, its good to firm up epistimlogies. Why was the 1st decision considered rock solid at the time?

Context is that I want to add

interpreter_constraints

to

python_distribution

to likely fix an issue @big-xylophone-43403 reported, and because I think it's sensible for

python_distribution

to have the field similar to how

pex_binary

has it So I'm revisiting if we should override vs. mix-in

The decision you made

(Trying to find the issue where I made the change to behavior)

If its anything else and the code still works one or more of the ICs invloved is a lie.

This is interesting from V1 land, we went out of our way to only use `pex_binary`: https://github.com/pantsbuild/pants/issues/7775

How do you read that? Stus OP suggests verifying intersection in point 2. But point 1 is only about cheaply side-stepping the OR issue.

Did I? We build the Pex with the whole closure and that constraint gets baked into the PEX-INFO last I checked. I agree #2 is solved tho

OK, So The issue Stu file about was we used all constraints, but in a list - so they got ORed - which was wrong. His fix suggestion was go the step 2 check and just pick one constraint for to write down after that - the easiest one to pick being the pex_binary one. I thought you improved that by implementing merging logic with Benjy's help with some demorgans law stuff?

Oh! Okay I see what you're getting at. Before, that BUILD file would result in:

['CPython>=2.7,<4', 'CPython>=3.6']

Now it correctly results in:

['CPython>=2.7,>=3.6,<4']

Yeah, like that. So you improved things with merging.

Cool, thank you for walking through that. I'm convinced

OK. You need to watch out. You have old man brain like me and forget what you thought up yesterday already!

Yeah, too many things like this floating around in my head Solid PR descriptions and issues are a helpful remedy

So presumably you'll be mapping IC to Requires-Python metadata for the distribution? Is that what the bug is about?

We still don't autogenerate Requires-Python, which we maybe should be. Hm. A complication there is it expects a single requirement, whereas the constraints could be a list A workaround is that users can manually set

provides=setup_py()

-- I'm waiting for user to confirm in https://pantsbuild.slack.com/archives/C046T6T9U/p1621624847091700, but I'm pretty sure the bug is that they have a

python_distribution

with no dependencies on

python_library

targets. Meaning transitive closure has no constraints. So Py2 is getting picked up for them, which they can't use Adding

interpreter_constraints

to

python_distribution

fixes this

Hrm. A pure resource distribution? That's certainly valid but odd.

Agreed it's odd. But because it's legal, we should fix imo

I mean, a pure resource distribution has no IC by definition though.

That works fine until you set

setup_py_commands

because we need to know which interpreter to run

setuptools

. You removed

[setuptools].interprerter_constraints

in 2.4 for correctness reasons

it seems to me that the global constraints should still apply in this case?

That is the impact of my PR, we will use global constraints in the default case, whereas we currently would use no constraints at all when running setuptools. I'll put this up rn

Why does adding the setup_py_commands field change the IC answer?

we don't run setuptools if

setup_py_commands

isn't set, so the ICs are completely ignored. ICs are only used with

setup_py.py

to determine what to run

setuptools

with + if we have Py2 for namespace packages handling

I'm totally confused. There is an algorithm to determine IC using target deps. If none, then the default IC. Now, mix in setup_py_commands - the answer should not change.

There is an algorithm to determine IC using target deps. If none, then the default IC.

Ah, that may be the source of confusion. The algorithm only uses

[python-setup].interpreter_constraints

as a default for targets who have an

interpreter_constraints

field defined. Otherwise it skips the target. So, if you have only a single

python_distribution

target w/ no deps, you get empty constraints back

So change the algorithm!

Hm, my response to that was realizing it'd be great to do this rather than how

release.sh

sets `PYTHON_SETUP_INTERPRETER_CONSTRAINTS`:

python_distribution(
    name="pants-packaged-py37", interpreter_constraints=["==3.7.*"], **_pants_packaged_kwargs
)
python_distribution(
    name="pants-packaged-py38", interpreter_constraints=["==3.8.*"], **_pants_packaged_kwargs
)
python_distribution(
    name="pants-packaged-py39", interpreter_constraints=["==3.9.*"], **_pants_packaged_kwargs
)

But that fails due to multiple owners of the same first party code 🤔 -- Even with that, I do think there's value in letting you set constraints on an individual

python_distribution

. Examples: * Your library code works with 3.6+. But you must run setuptools with specifically 3.8, e.g. for the correct native platform to be used. You shouldn't need to constrain your

python_library

to do that * You have no first party code in your dist. Your repo's default is Py2 or Py3.5+. But you're using a newer version of setuptools so you can't run it with Py2 These types of situations are why we added back

interpreter_constraints

to

pex_binary

and

python_awslambda

. @jolly-midnight-72759 and @helpful-lunch-92084 shared that they were setting that field a lot and were very happy to see it come back.

Neither of your examples make sense. 1: How can a distribution work with 3.6+ but need to build a 3.8 specific .so? 2: If you have a dist with no python dependencies, but you do have a repo default IC how can you have an opinion of what version is setuptools to use? Isn't the latest compatible with the interpreter picked fine? Are you trying to concoct problems or are these real? They seem contrived and unreal.

I'm trying to solve the user's bug in a way that's consistent with how we handle other distributions (

pex_binary

and

python_awslambda

) and that is resilient to situations we can't fully anticipate, where it's not adequate to use

[python-setup].interpreter_constraints

to determine what to run setuptools with

yes, in v1 we heavily added interpreter_constraints to python_binary and python_test targets and left python_library targets largely without them (unless they needed to be explicitly set, e.g. they were a py3-only library and we didn’t want a cross project py2 dependency accidentally trying to use it). We also had many pex_binary targets that didn’t rely upon a library for like simple scripts. For example:

pex_binary(
  name = "some_script",
  source="some_script.py",
  interpreter_constraints=["~3.6"]
)

or

pex_binary(
    name="aws",
    entry_point="awscli.clidriver:main",
    dependencies=["3rdparty/python:awscli"],
    interpreter_constraints=["~3.6"],
)

So, the only situation in play is 0 python dependency distributions. Is that right?

Two things going on: - User-reported bug when there are no deps on

python_library

targets, so no constraints used - Last month, Benjy and I realized it was a mistake to remove

interpreter_constraints

from

pex_binary

and

python_awslambda

. We fixed that. I've been pondering past month if we should update

python_distribution

to be consistent

The 1st seems more pertinent. The second assumes consistency is a universal good. The question is why do pex_binaryband python_awslambda need ICs. I can't see a reason why python_distribution does.

here is one reason for pex_binary

pex_binary(
    name="aws",
    entry_point="awscli.clidriver:main",
    dependencies=["3rdparty/python:awscli"],
    interpreter_constraints=["~3.6"],
)

Ah, yes. Ok, great. Good reason.

assume that awscli requires 3.6 but your default repo constraints are [2.7, 3.5+]

Gotta run to meeting but I tried to summarize convo in https://github.com/pantsbuild/pants/pull/12207