Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose pex's --check validation #19957

Closed
huonw opened this issue Oct 1, 2023 · 3 comments
Closed

Expose pex's --check validation #19957

huonw opened this issue Oct 1, 2023 · 3 comments
Labels
backend: Python Python backend-related issues enhancement

Comments

@huonw
Copy link
Contributor

huonw commented Oct 1, 2023

Is your feature request related to a problem? Please describe.

In 2.1.148 (pex-tool/pex#2247 / pex-tool/pex#2253), PEX adds support for --check=... to give build-time warnings/errors about --layout=zipapp PEXes that are likely to not be able to be loaded by a Python interpreter, e.g. so many files or large enough to require "ZIP64" extensions, which zipimport doesn't handle (python/cpython#77140).

This likely affects large PEXes including machine learning packages like PyTorch.

Describe the solution you'd like

For the pex_binary target:

  • ensure that any warnings are exposed
  • consider providing a pass-through check option somehow

Describe alternatives you've considered

A general mechanism for showing warnings

Additional context

N/A

@huonw huonw added enhancement backend: Python Python backend-related issues labels Oct 1, 2023
@huonw
Copy link
Contributor Author

huonw commented Oct 2, 2023

See pex-tool/pex#2253 (comment) for exploration of showing this via --pex-verbosity=1.

cburroughs added a commit to cburroughs/pants that referenced this issue Feb 1, 2024
Pants has traditionally run Pex with --no-emit-warnings which as the
name implies... doesn't emit warnings.  This hides pertinent
information like "the pex file you created won't actually work" from
the user and sends them off on a debugging hunt.

 * Create an `emit_warnings` option, defaulting to True like the
 underlying tool..
 * A little logic for pipeing Pex's stderr to Pants' logging.

ref pantsbuild#19957
cburroughs added a commit to cburroughs/pants that referenced this issue Feb 1, 2024
Pex exposes the `--check` flag to "check" if the resulting zipapp can
be opened by CPython.  Plumb that through so that Pants stops making
PEXs that won't work at runtime.

NOTE: The default of `error` differs from Pex.  I'm struggling to
think of a case where one would want a zipapp CPython can't open and
Pants has a history of swalling warnings from Pex (`warn` is Pex's
default).

ref pantsbuild#19957
huonw pushed a commit that referenced this issue Feb 8, 2024
Pex exposes the `--check` flag to "check" if the resulting zipapp can be
opened by CPython. Plumb that through so that Pants stops making PEXs
that won't work at runtime.

ref #19957
kaos pushed a commit that referenced this issue Feb 15, 2024
Pants has traditionally run Pex with --no-emit-warnings which as the
name implies... doesn't emit warnings. This hides pertinent information
like "the pex file you created won't actually work" from the user and
sends them off on a debugging hunt.

* Create an `emit_warnings` option, defaulting to True like the
underlying tool..
 * A little logic for pipeing Pex's stderr to Pants' logging.
* Fix the logic for old per target `emit_warnings` that didn't quite
work.

ref #19957
@cburroughs
Copy link
Contributor

This can be closed now, correct?

@huonw
Copy link
Contributor Author

huonw commented Feb 29, 2024

Yeah! #20481 / #20480 seen to get this done. Thanks @cburroughs

@huonw huonw closed this as completed Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Python Python backend-related issues enhancement
Projects
None yet
Development

No branches or pull requests

2 participants