Skip to content

Commit b3072b8

Browse files
authored
stop swallowing warnings from Pex by default (#20480)
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
1 parent 71600cc commit b3072b8

File tree

7 files changed

+37
-12
lines changed

7 files changed

+37
-12
lines changed

src/python/pants/backend/python/goals/lockfile.py

+5-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@
1818
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
1919
from pants.backend.python.util_rules.lockfile_diff import _generate_python_lockfile_diff
2020
from pants.backend.python.util_rules.lockfile_metadata import PythonLockfileMetadata
21-
from pants.backend.python.util_rules.pex_cli import PexCliProcess
21+
from pants.backend.python.util_rules.pex_cli import PexCliProcess, maybe_log_pex_stderr
22+
from pants.backend.python.util_rules.pex_environment import PexSubsystem
2223
from pants.backend.python.util_rules.pex_requirements import (
2324
PexRequirements,
2425
ResolvePexConfig,
@@ -103,6 +104,7 @@ async def generate_lockfile(
103104
req: GeneratePythonLockfile,
104105
generate_lockfiles_subsystem: GenerateLockfilesSubsystem,
105106
python_setup: PythonSetup,
107+
pex_subsystem: PexSubsystem,
106108
) -> GenerateLockfileResult:
107109
pip_args_setup = await _setup_pip_args_and_constraints_file(req.resolve_name)
108110

@@ -113,7 +115,6 @@ async def generate_lockfile(
113115
subcommand=("lock", "create"),
114116
extra_args=(
115117
"--output=lock.json",
116-
"--no-emit-warnings",
117118
# See https://github.com/pantsbuild/pants/issues/12458. For now, we always
118119
# generate universal locks because they have the best compatibility. We may
119120
# want to let users change this, as `style=strict` is safer.
@@ -158,6 +159,8 @@ async def generate_lockfile(
158159
),
159160
)
160161

162+
maybe_log_pex_stderr(result.stderr, pex_subsystem.verbosity)
163+
161164
initial_lockfile_digest_contents = await Get(DigestContents, Digest, result.output_digest)
162165
metadata = PythonLockfileMetadata.new(
163166
valid_for_interpreter_constraints=req.interpreter_constraints,

src/python/pants/backend/python/goals/package_pex_binary.py

+2
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ def generate_additional_args(self, pex_binary_defaults: PexBinaryDefaults) -> Tu
9292
args = []
9393
if self.emit_warnings.value_or_global_default(pex_binary_defaults) is False:
9494
args.append("--no-emit-warnings")
95+
elif self.emit_warnings.value_or_global_default(pex_binary_defaults) is True:
96+
args.append("--emit-warnings")
9597
if self.resolve_local_platforms.value_or_global_default(pex_binary_defaults) is True:
9698
args.append("--resolve-local-platforms")
9799
if self.ignore_errors.value is True:

src/python/pants/backend/python/goals/package_pex_binary_integration_test.py

+3
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ def test_include_sources_avoids_files_targets_warning(
135135
"""\
136136
python_sources(
137137
name='sources',
138+
interpreter_constraints=["CPython==3.10.*"]
138139
)
139140
140141
python_distribution(
@@ -147,12 +148,14 @@ def test_include_sources_avoids_files_targets_warning(
147148
name='my-dist',
148149
version='1.2.3',
149150
),
151+
interpreter_constraints=["CPython==3.10.*"]
150152
)
151153
152154
pex_binary(
153155
dependencies=[':wheel'],
154156
entry_point="none",
155157
include_sources=False,
158+
interpreter_constraints=["CPython==3.10.*"]
156159
)
157160
"""
158161
),

src/python/pants/backend/python/target_types.py

+1
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,7 @@ class PexEmitWarningsField(TriBoolField):
606606
def value_or_global_default(self, pex_binary_defaults: PexBinaryDefaults) -> bool:
607607
if self.value is None:
608608
return pex_binary_defaults.emit_warnings
609+
609610
return self.value
610611

611612

src/python/pants/backend/python/util_rules/pex.py

+3-10
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
)
3232
from pants.backend.python.util_rules import pex_cli, pex_requirements
3333
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
34-
from pants.backend.python.util_rules.pex_cli import PexCliProcess, PexPEX
34+
from pants.backend.python.util_rules.pex_cli import PexCliProcess, PexPEX, maybe_log_pex_stderr
3535
from pants.backend.python.util_rules.pex_environment import (
3636
CompletePexEnvironment,
3737
PexEnvironment,
@@ -403,10 +403,7 @@ async def find_interpreter(
403403
)
404404
path, fingerprint = result.stdout.decode().strip().splitlines()
405405

406-
if pex_subsystem.verbosity > 0:
407-
log_output = result.stderr.decode()
408-
if log_output:
409-
logger.info("%s", log_output)
406+
maybe_log_pex_stderr(result.stderr, pex_subsystem.verbosity)
410407

411408
return PythonExecutable(path=path, fingerprint=fingerprint)
412409

@@ -678,7 +675,6 @@ async def build_pex(
678675
argv = [
679676
"--output-file",
680677
request.output_filename,
681-
"--no-emit-warnings",
682678
*request.additional_args,
683679
]
684680

@@ -758,10 +754,7 @@ async def build_pex(
758754
),
759755
)
760756

761-
if pex_subsystem.verbosity > 0:
762-
log_output = result.stderr.decode()
763-
if log_output:
764-
logger.info("%s", log_output)
757+
maybe_log_pex_stderr(result.stderr, pex_subsystem.verbosity)
765758

766759
digest = (
767760
await Get(

src/python/pants/backend/python/util_rules/pex_cli.py

+15
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from __future__ import annotations
55

66
import dataclasses
7+
import logging
78
import os.path
89
from dataclasses import dataclass
910
from typing import Iterable, List, Mapping, Optional, Tuple
@@ -29,6 +30,8 @@
2930
from pants.util.logging import LogLevel
3031
from pants.util.meta import classproperty
3132

33+
logger = logging.getLogger(__name__)
34+
3235

3336
class PexCli(TemplatedExternalTool):
3437
options_scope = "pex-cli"
@@ -156,6 +159,8 @@ async def setup_pex_cli_process(
156159

157160
verbosity_args = [f"-{'v' * pex_subsystem.verbosity}"] if pex_subsystem.verbosity > 0 else []
158161

162+
warnings_args = [] if pex_subsystem.emit_warnings else ["--no-emit-warnings"]
163+
159164
# NB: We should always pass `--python-path`, as that tells Pex where to look for interpreters
160165
# when `--python` isn't an absolute path.
161166
resolve_args = [
@@ -171,6 +176,7 @@ async def setup_pex_cli_process(
171176
*request.subcommand,
172177
*global_args,
173178
*verbosity_args,
179+
*warnings_args,
174180
*pip_version_args,
175181
*resolve_args,
176182
# NB: This comes at the end because it may use `--` passthrough args, # which must come at
@@ -202,6 +208,15 @@ async def setup_pex_cli_process(
202208
)
203209

204210

211+
def maybe_log_pex_stderr(stderr: bytes, pex_verbosity: int) -> None:
212+
"""Forward Pex's stderr to a Pants logger if conditions are met."""
213+
log_output = stderr.decode()
214+
if log_output and "PEXWarning:" in log_output:
215+
logger.warning("%s", log_output)
216+
elif log_output and pex_verbosity > 0:
217+
logger.info("%s", log_output)
218+
219+
205220
def rules():
206221
return [
207222
*collect_rules(),

src/python/pants/backend/python/util_rules/pex_environment.py

+8
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,14 @@ def iter_path_entries():
8181
),
8282
advanced=True,
8383
)
84+
emit_warnings = BoolOption(
85+
default=True,
86+
help=softwrap(
87+
"""
88+
If warnings from Pex should be logged by Pants to the console.
89+
"""
90+
),
91+
)
8492

8593
@property
8694
def verbosity(self) -> int:

0 commit comments

Comments
 (0)