Plumb Pip's `--{no,only}-binary`. #2346

jsirois · 2024-01-26T07:26:48Z

Previously, Pex just allowed specifying --no-wheel or --no-build
globally. Now, individual projects can be slated for --only-wheel or
--only-build just as they can with Pip. In the lock case, this
information is saved so that lock updates carry the configuration
forward.

Fixes #2343

Previously, Pex just allowed specifying `--no-wheel` or `--no-build` globally. Now, individual projects can be slated for `--only-wheel` or `--only-build` just as they can with Pip. In the lock case, this information is saved so that lock updates carry the configuration forward. Fixes pex-tool#2343

kaos · 2024-01-26T08:00:34Z

pex/pip/tool.py

+        if build_configuration.use_pep517 is not None:
+            yield "--use-pep517" if build_configuration.use_pep517 else "--no-use-pep517"


Q: what makes the pep517 option different from the other bool options?

(realized I could probably reply to myself here by reading pip install --help)
A: the difference aiui is that pep517 is used by default, so the option really is about disabling it.

Thanks for digging there, but I'm still crying a bit. Did you check out Pex's own docs?:
https://github.com/pantsbuild/pex/blob/015a077d3496205824491bf97136afd719277b12/pex/resolve/resolver_options.py#L206-L224

Assuming those are read, if they still don't make sense, it would be good to know so I can fix.

I fixed that bad URL.

Ah, my bad for not realizing the option was of course documented for pex as well (got tunnel visioned onto the pip arg..).

Yea, the pex option has a lot of information to digest. Reading it through a few times makes it almost clear. I wonder if there is a typo here:

--- If PEP-517 is forced (--use-pep517 is passed) +++ If PEP-517 is forced (--force-pep517 is passed)

then again, I would expect that --force-pep517 is the same as --use-pep517 (based on my understanding of how ArgumentParser works), but the help has so many layers that I get the impression that they are not.. (also, that it reads default: None rather than default: True doesn't help which would align with the help text stating that it uses pep517 by default)
So, I feel there's a bit of language ... (failed to find the word I was looking for) here.

I think this option would be clearer using an enum value, but I guess the current state is due to upholding backwards compatibility..? In which case I think making the relationship between --force-pep517 and --use-pep517 clearer would help, and perhaps also mentioning that the default behavior is based on not providing a value for the option (e.g. explain that this flag is a tri-bool)

I can see why you cry a bit over this.

Ok, so to sum up - I think you figured this out, but in case not:

The option is a None|True|False tri-state

The default is None

The option names are all just synonyms (with --no- prefixes negating)

So the default, None, means do the right thing: if there is a pyproject.toml build system, use it; if not use setup.py.

True means use (force) pep 517 always (concoct a setuptools legacy build backend as needed).

False means do not use pep 517.

I have no clue what use cases benefit from the True or False cases, certainly the False case makes no sense I can see. I just exposed this Pip knob to not get between users and Pip. Perhaps my sin was in trying to make the Pip help different / more clear in up in Pex. I clearly failed there!

Thanks. Yea, that summation aligns with my current understanding.

pex/resolve/resolver_configuration.py

Co-authored-by: Andreas Stenius <git@astekk.se>

jsirois · 2024-01-26T15:59:27Z

Ok, @cburroughs I knocked this out and will release 2.1.161 as soon as it lands green here in the hour. My time got cut short this stint and I only have through Saturday evening to bang out #2344. I think I'll get that done, but in case not, this should unblock your Pants work on the sharper knife path if you don't want to potentially wait until ~February 9th when I next pick back up work after going AFK this Sunday.

cburroughs · 2024-01-26T16:24:20Z

Thank for the update, context, and enjoy your trip! I'll try to give it a look right now.

It lookslike the pex has an extra validation that stops the pip special :all: and :none values from flowing through:

$ python3.10 -m pex.cli lock create '--indent=2'  --manylinux manylinux2014 --interpreter-constraint $'CPython==3.10.*' --style=universal --pip-version=23.3.2 --resolver-version pip-2020-resolver --target-system=linux --target-system=mac --only-binary :all:  ansicolors==1.1.5 kaleido==0.1.0.post1   -o tmp/k2.json 
usage: /usr/bin/python3.10 -m pex.cli lock create [-h] [--style {strict,sources,universal}] [--target-system {linux,mac,windows}]
                                                  [--path-mapping PATH_MAPPINGS] [-o PATH] [--indent INDENT] [-r FILE or URL]
                                                  [--constraints FILE or URL] [--python PYTHON] [--python-path PYTHON_PATH]
                                                  [--interpreter-constraint INTERPRETER_CONSTRAINT] [--platform PLATFORMS]
                                                  [--complete-platform COMPLETE_PLATFORMS] [--manylinux [ASSUME_MANYLINUX]]
                                                  [--resolve-local-platforms]
                                                  [--resolver-version {pip-legacy-resolver,pip-2020-resolver}]
                                                  [--pip-version {latest,vendored,20.3.4-patched,22.2.2,22.3,22.3.1,23.0,23.0.1,23.1,23.1.1,23.1.2,23.2,23.3.1,23.3.2}]
                                                  [--allow-pip-version-fallback] [--use-pip-config] [--pypi] [-f PATH/URL]
                                                  [-i URL] [--retries RETRIES] [--timeout SECS] [--proxy PROXY] [--cert PATH]
                                                  [--client-cert PATH] [--cache-ttl DEPRECATED] [-H DEPRECATED] [--pre] [--wheel]
                                                  [--only-binary ONLY_WHEELS] [--build] [--only-build ONLY_BUILDS]
                                                  [--prefer-wheel] [--force-pep517] [--build-isolation] [--transitive] [-j JOBS]
                                                  [--preserve-pip-download-log] [-v] [--emit-warnings] [--pex-root PEX_ROOT]
                                                  [--disable-cache] [--cache-dir CACHE_DIR] [--tmpdir TMPDIR] [--rcfile RC_FILE]
                                                  [requirements ...]
/usr/bin/python3.10 -m pex.cli lock create: error: argument --only-binary/--only-wheel: The given project name ':all:' is not a valid. It must conform to the regex '^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$' as specified in https://peps.python.org/pep-0508/#names

jsirois · 2024-01-26T17:09:24Z

Sorry about that! It looks like I edited your comment instead of replying. Very bad.

It looks like the pex has an extra validation that stops the pip special :all: and :none values from flowing through:

Correct! That's what the pre-existing --no-{wheel,build} options are for. In other words these are not direct pass throughs. They only logically pass through. See here for the exact mapping step setting up Pip's args: https://github.com/pantsbuild/pex/pull/2346/files#diff-443e54ea41451726af072f7976469d8bdbb37d285fd49fc1a4bc6447588591c3R409

For context, the --no-{wheel,build} options were introduced back in Pex "OLD" (2014) when it had a bespoke resolver and did not use Pip. Since Pex doesn't break existing users, the option names and meanings stayed constant after adoption of Pip as the underlying resolver in the 2.x series.

jsirois · 2024-03-21T23:49:51Z

pex/resolve/lock_resolver.py

-        self._use_pep517 = use_pep517
-        self._build_isolation = build_isolation
+        self._build_configuration = attr.evolve(
+            build_configuration, allow_wheels=False, prefer_older_binary=False


This is a bug. The incoming BuildConfiguration from the user may say allow_builds=False, allow_wheels=True (the known useful example being a user constraining a --style universal lock to just wheels), in which case just flipping allow_wheels=False here leads immediately to BuildConfiguration failing a pre-condition check (allow_wheels=False, allow_builds=False makes no sense and is illegal). The VCSArtifactDownloadManager should complete its thought and say it uses the user BuildConfiguration but trumps allow_wheels=False, allow_builds=True since the VCSArtifactDownloadManager must be able to build a wheel from the VCS checkout to do anything useful at all.

See #2389 for the fallout.

Since #2346 which was released in Pex 2.1.161, using `--no-build` with a `--lock` would fail fast even if the lock itself was created with `--no-build`, which should be compatible. Fixes #2389

jsirois requested review from cburroughs, kaos and benjyw January 26, 2024 07:26

kaos approved these changes Jan 26, 2024

View reviewed changes

jsirois and others added 4 commits January 26, 2024 06:43

@kaos's wording fix.

3257bbb

Co-authored-by: Andreas Stenius <git@astekk.se>

Fix --use-pep517 help PEP URL.

e924df4

DRY up BuildConfiguration access.

c00afb7

Prepare the 2.1.161 release.

6aa4ea4

Fix import formatting.

a1f0aad

Fix test verbage expectation.

992c544

jsirois merged commit 23c3c06 into pex-tool:main Jan 26, 2024
26 checks passed

jsirois deleted the issues/2343 branch January 26, 2024 18:23

jsirois mentioned this pull request Mar 21, 2024

unable to create pex with --no-build flag from lockfile #2389

Closed

jsirois commented Mar 21, 2024

View reviewed changes

jsirois mentioned this pull request Mar 22, 2024

Fix pex --no-build --lock .... #2390

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plumb Pip's `--{no,only}-binary`. #2346

Plumb Pip's `--{no,only}-binary`. #2346

jsirois commented Jan 26, 2024

kaos Jan 26, 2024

jsirois Jan 26, 2024 •

edited

jsirois Jan 26, 2024

kaos Jan 29, 2024

jsirois Jan 29, 2024 •

edited

kaos Jan 31, 2024

jsirois commented Jan 26, 2024

cburroughs commented Jan 26, 2024 •

edited by jsirois

jsirois commented Jan 26, 2024 •

edited

jsirois Mar 21, 2024 •

edited

		if build_configuration.use_pep517 is not None:
		yield "--use-pep517" if build_configuration.use_pep517 else "--no-use-pep517"

Plumb Pip's --{no,only}-binary. #2346

Plumb Pip's --{no,only}-binary. #2346

Conversation

jsirois commented Jan 26, 2024

kaos Jan 26, 2024

Choose a reason for hiding this comment

jsirois Jan 26, 2024 • edited

Choose a reason for hiding this comment

jsirois Jan 26, 2024

Choose a reason for hiding this comment

kaos Jan 29, 2024

Choose a reason for hiding this comment

jsirois Jan 29, 2024 • edited

Choose a reason for hiding this comment

kaos Jan 31, 2024

Choose a reason for hiding this comment

jsirois commented Jan 26, 2024

cburroughs commented Jan 26, 2024 • edited by jsirois

jsirois commented Jan 26, 2024 • edited

jsirois Mar 21, 2024 • edited

Choose a reason for hiding this comment

Plumb Pip's `--{no,only}-binary`. #2346

Plumb Pip's `--{no,only}-binary`. #2346

jsirois Jan 26, 2024 •

edited

jsirois Jan 29, 2024 •

edited

cburroughs commented Jan 26, 2024 •

edited by jsirois

jsirois commented Jan 26, 2024 •

edited

jsirois Mar 21, 2024 •

edited