Skip to content

Commit

Permalink
stop swallowing warnings from Pex by default (pantsbuild#20480)
Browse files Browse the repository at this point in the history
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 pantsbuild#19957
  • Loading branch information
cburroughs authored Feb 15, 2024
1 parent 71600cc commit b3072b8
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 12 deletions.
7 changes: 5 additions & 2 deletions src/python/pants/backend/python/goals/lockfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
from pants.backend.python.util_rules.lockfile_diff import _generate_python_lockfile_diff
from pants.backend.python.util_rules.lockfile_metadata import PythonLockfileMetadata
from pants.backend.python.util_rules.pex_cli import PexCliProcess
from pants.backend.python.util_rules.pex_cli import PexCliProcess, maybe_log_pex_stderr
from pants.backend.python.util_rules.pex_environment import PexSubsystem
from pants.backend.python.util_rules.pex_requirements import (
PexRequirements,
ResolvePexConfig,
Expand Down Expand Up @@ -103,6 +104,7 @@ async def generate_lockfile(
req: GeneratePythonLockfile,
generate_lockfiles_subsystem: GenerateLockfilesSubsystem,
python_setup: PythonSetup,
pex_subsystem: PexSubsystem,
) -> GenerateLockfileResult:
pip_args_setup = await _setup_pip_args_and_constraints_file(req.resolve_name)

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

maybe_log_pex_stderr(result.stderr, pex_subsystem.verbosity)

initial_lockfile_digest_contents = await Get(DigestContents, Digest, result.output_digest)
metadata = PythonLockfileMetadata.new(
valid_for_interpreter_constraints=req.interpreter_constraints,
Expand Down
2 changes: 2 additions & 0 deletions src/python/pants/backend/python/goals/package_pex_binary.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ def generate_additional_args(self, pex_binary_defaults: PexBinaryDefaults) -> Tu
args = []
if self.emit_warnings.value_or_global_default(pex_binary_defaults) is False:
args.append("--no-emit-warnings")
elif self.emit_warnings.value_or_global_default(pex_binary_defaults) is True:
args.append("--emit-warnings")
if self.resolve_local_platforms.value_or_global_default(pex_binary_defaults) is True:
args.append("--resolve-local-platforms")
if self.ignore_errors.value is True:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ def test_include_sources_avoids_files_targets_warning(
"""\
python_sources(
name='sources',
interpreter_constraints=["CPython==3.10.*"]
)
python_distribution(
Expand All @@ -147,12 +148,14 @@ def test_include_sources_avoids_files_targets_warning(
name='my-dist',
version='1.2.3',
),
interpreter_constraints=["CPython==3.10.*"]
)
pex_binary(
dependencies=[':wheel'],
entry_point="none",
include_sources=False,
interpreter_constraints=["CPython==3.10.*"]
)
"""
),
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/python/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,7 @@ class PexEmitWarningsField(TriBoolField):
def value_or_global_default(self, pex_binary_defaults: PexBinaryDefaults) -> bool:
if self.value is None:
return pex_binary_defaults.emit_warnings

return self.value


Expand Down
13 changes: 3 additions & 10 deletions src/python/pants/backend/python/util_rules/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
)
from pants.backend.python.util_rules import pex_cli, pex_requirements
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
from pants.backend.python.util_rules.pex_cli import PexCliProcess, PexPEX
from pants.backend.python.util_rules.pex_cli import PexCliProcess, PexPEX, maybe_log_pex_stderr
from pants.backend.python.util_rules.pex_environment import (
CompletePexEnvironment,
PexEnvironment,
Expand Down Expand Up @@ -403,10 +403,7 @@ async def find_interpreter(
)
path, fingerprint = result.stdout.decode().strip().splitlines()

if pex_subsystem.verbosity > 0:
log_output = result.stderr.decode()
if log_output:
logger.info("%s", log_output)
maybe_log_pex_stderr(result.stderr, pex_subsystem.verbosity)

return PythonExecutable(path=path, fingerprint=fingerprint)

Expand Down Expand Up @@ -678,7 +675,6 @@ async def build_pex(
argv = [
"--output-file",
request.output_filename,
"--no-emit-warnings",
*request.additional_args,
]

Expand Down Expand Up @@ -758,10 +754,7 @@ async def build_pex(
),
)

if pex_subsystem.verbosity > 0:
log_output = result.stderr.decode()
if log_output:
logger.info("%s", log_output)
maybe_log_pex_stderr(result.stderr, pex_subsystem.verbosity)

digest = (
await Get(
Expand Down
15 changes: 15 additions & 0 deletions src/python/pants/backend/python/util_rules/pex_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from __future__ import annotations

import dataclasses
import logging
import os.path
from dataclasses import dataclass
from typing import Iterable, List, Mapping, Optional, Tuple
Expand All @@ -29,6 +30,8 @@
from pants.util.logging import LogLevel
from pants.util.meta import classproperty

logger = logging.getLogger(__name__)


class PexCli(TemplatedExternalTool):
options_scope = "pex-cli"
Expand Down Expand Up @@ -156,6 +159,8 @@ async def setup_pex_cli_process(

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

warnings_args = [] if pex_subsystem.emit_warnings else ["--no-emit-warnings"]

# NB: We should always pass `--python-path`, as that tells Pex where to look for interpreters
# when `--python` isn't an absolute path.
resolve_args = [
Expand All @@ -171,6 +176,7 @@ async def setup_pex_cli_process(
*request.subcommand,
*global_args,
*verbosity_args,
*warnings_args,
*pip_version_args,
*resolve_args,
# NB: This comes at the end because it may use `--` passthrough args, # which must come at
Expand Down Expand Up @@ -202,6 +208,15 @@ async def setup_pex_cli_process(
)


def maybe_log_pex_stderr(stderr: bytes, pex_verbosity: int) -> None:
"""Forward Pex's stderr to a Pants logger if conditions are met."""
log_output = stderr.decode()
if log_output and "PEXWarning:" in log_output:
logger.warning("%s", log_output)
elif log_output and pex_verbosity > 0:
logger.info("%s", log_output)


def rules():
return [
*collect_rules(),
Expand Down
8 changes: 8 additions & 0 deletions src/python/pants/backend/python/util_rules/pex_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,14 @@ def iter_path_entries():
),
advanced=True,
)
emit_warnings = BoolOption(
default=True,
help=softwrap(
"""
If warnings from Pex should be logged by Pants to the console.
"""
),
)

@property
def verbosity(self) -> int:
Expand Down

0 comments on commit b3072b8

Please sign in to comment.