Skip to content

Commit

Permalink
Move the preserve hint to a more specific place.
Browse files Browse the repository at this point in the history
[ci skip-build-wheels]

[ci skip-rust]
  • Loading branch information
stuhood committed Sep 26, 2021
1 parent 1b8c258 commit dd905a4
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from pants.engine.process import BashBinary, FallibleProcessResult, Process, ProcessExecutionFailure
from pants.engine.rules import Get, MultiGet, collect_rules, rule
from pants.jvm.resolve.coursier_fetch import MaterializedClasspath, MaterializedClasspathRequest
from pants.option.global_options import GlobalOptions
from pants.util.logging import LogLevel

logger = logging.getLogger(__name__)
Expand All @@ -32,6 +33,7 @@ class FallibleJavaSourceDependencyAnalysisResult:
@rule(level=LogLevel.DEBUG)
async def resolve_fallible_result_to_analysis(
fallible_result: FallibleJavaSourceDependencyAnalysisResult,
global_options: GlobalOptions,
) -> JavaSourceDependencyAnalysis:
# TODO(#12725): Just convert directly to a ProcessResult like this:
# result = await Get(ProcessResult, FallibleProcessResult, fallible_result.process_result)
Expand All @@ -46,6 +48,7 @@ async def resolve_fallible_result_to_analysis(
fallible_result.process_result.stdout,
fallible_result.process_result.stderr,
"Java source dependency analysis failed.",
local_cleanup=global_options.options.process_execution_local_cleanup,
)


Expand Down
3 changes: 3 additions & 0 deletions src/python/pants/backend/python/goals/coverage_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
from pants.engine.target import TransitiveTargets, TransitiveTargetsRequest
from pants.engine.unions import UnionRule
from pants.option.custom_types import file_option
from pants.option.global_options import GlobalOptions
from pants.source.source_root import AllSourceRoots
from pants.util.docutil import git_url
from pants.util.logging import LogLevel
Expand Down Expand Up @@ -476,6 +477,7 @@ async def generate_coverage_reports(
coverage_setup: CoverageSetup,
coverage_config: CoverageConfig,
coverage_subsystem: CoverageSubsystem,
global_options: GlobalOptions,
) -> CoverageReports:
"""Takes all Python test results and generates a single coverage report."""
transitive_targets = await Get(
Expand Down Expand Up @@ -552,6 +554,7 @@ async def generate_coverage_reports(
res.stdout,
res.stderr,
proc.description,
local_cleanup=global_options.options.process_execution_local_cleanup,
)

# In practice if one result triggers --fail-under, they all will, but no need to rely on that.
Expand Down
37 changes: 25 additions & 12 deletions src/python/pants/engine/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from pants.engine.internals.selectors import MultiGet
from pants.engine.platform import Platform
from pants.engine.rules import Get, collect_rules, rule, side_effecting
from pants.option.global_options import GlobalOptions
from pants.util.frozendict import FrozenDict
from pants.util.logging import LogLevel
from pants.util.meta import frozen_after_init
Expand Down Expand Up @@ -209,25 +210,34 @@ class ProcessExecutionFailure(Exception):
"""

def __init__(
self, exit_code: int, stdout: bytes, stderr: bytes, process_description: str
self,
exit_code: int,
stdout: bytes,
stderr: bytes,
process_description: str,
*,
local_cleanup: bool,
) -> None:
# These are intentionally "public" members.
self.exit_code = exit_code
self.stdout = stdout
self.stderr = stderr
# NB: We don't use dedent on a single format string here because it would attempt to
# interpret the stdio content.
super().__init__(
"\n".join(
[
f"Process '{process_description}' failed with exit code {exit_code}.",
"stdout:",
stdout.decode(),
"stderr:",
stderr.decode(),
]
err_strings = [
f"Process '{process_description}' failed with exit code {exit_code}.",
"stdout:",
stdout.decode(),
"stderr:",
stderr.decode(),
]
if local_cleanup:
err_strings.append(
"\n\n"
"Use --no-process-execution-local-cleanup to preserve process chroots "
"for inspection."
)
)
super().__init__("\n".join(err_strings))


@rule
Expand All @@ -243,7 +253,9 @@ def upcast_process(req: Process) -> MultiPlatformProcess:

@rule
def fallible_to_exec_result_or_raise(
fallible_result: FallibleProcessResult, description: ProductDescription
fallible_result: FallibleProcessResult,
description: ProductDescription,
global_options: GlobalOptions,
) -> ProcessResult:
"""Converts a FallibleProcessResult to a ProcessResult or raises an error."""

Expand All @@ -260,6 +272,7 @@ def fallible_to_exec_result_or_raise(
fallible_result.stdout,
fallible_result.stderr,
description.value,
local_cleanup=global_options.options.process_execution_local_cleanup,
)


Expand Down
18 changes: 4 additions & 14 deletions src/python/pants/init/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,20 +42,17 @@ def flush(self) -> None:
class _ExceptionFormatter(Formatter):
"""Possibly render the stacktrace and possibly give debug hints, based on global options."""

def __init__(self, level: LogLevel, *, print_stacktrace: bool, local_cleanup: bool) -> None:
def __init__(self, level: LogLevel, *, print_stacktrace: bool) -> None:
super().__init__(None)
self.level = level
self.print_stacktrace = print_stacktrace
self.local_cleanup = local_cleanup

def formatException(self, exc_info):
stacktrace = super().formatException(exc_info) if self.print_stacktrace else ""

debug_instructions = []
if not self.print_stacktrace:
debug_instructions.append("--print-stacktrace for more error details")
if self.local_cleanup:
debug_instructions.append("--no-process-execution-local-cleanup to inspect chroots")
if self.level not in {LogLevel.DEBUG, LogLevel.TRACE}:
debug_instructions.append("-ldebug for more logs")
debug_instructions = (
Expand Down Expand Up @@ -102,9 +99,7 @@ def stdio_destination_use_color(use_color: bool) -> None:


@contextmanager
def _python_logging_setup(
level: LogLevel, *, print_stacktrace: bool, local_cleanup: bool
) -> Iterator[None]:
def _python_logging_setup(level: LogLevel, *, print_stacktrace: bool) -> Iterator[None]:
"""Installs a root Python logger that routes all logging through a Rust logger."""

def trace_fn(self, message, *args, **kwargs):
Expand All @@ -130,9 +125,7 @@ def set_logging_handlers(handlers):
# This routes warnings through our loggers instead of straight to raw stderr.
logging.captureWarnings(True)
handler = _NativeHandler()
exc_formatter = _ExceptionFormatter(
level, print_stacktrace=print_stacktrace, local_cleanup=local_cleanup
)
exc_formatter = _ExceptionFormatter(level, print_stacktrace=print_stacktrace)
handler.setFormatter(exc_formatter)
logger.addHandler(handler)
level.set_level_for(logger)
Expand Down Expand Up @@ -169,7 +162,6 @@ def initialize_stdio(global_bootstrap_options: OptionValueContainer) -> Iterator
show_target = global_bootstrap_options.show_log_target
log_levels_by_target = _get_log_levels_by_target(global_bootstrap_options)
print_stacktrace = global_bootstrap_options.print_stacktrace
local_cleanup = global_bootstrap_options.process_execution_local_cleanup

literal_filters = []
regex_filters = []
Expand Down Expand Up @@ -205,9 +197,7 @@ def initialize_stdio(global_bootstrap_options: OptionValueContainer) -> Iterator

sys.__stdin__, sys.__stdout__, sys.__stderr__ = sys.stdin, sys.stdout, sys.stderr
# Install a Python logger that will route through the Rust logger.
with _python_logging_setup(
global_level, print_stacktrace=print_stacktrace, local_cleanup=local_cleanup
):
with _python_logging_setup(global_level, print_stacktrace=print_stacktrace):
yield
finally:
sys.stdin, sys.stdout, sys.stderr = original_stdin, original_stdout, original_stderr
Expand Down

0 comments on commit dd905a4

Please sign in to comment.