Skip to content

Commit 564e6ec

Browse files
authored
Force colors from mypy always, strip in pants when --no-colors (Cherry-pick of #16586) (#16808)
This fixes #16451 by continuing the color half of #16488 (terminal width). It does this by forcing mypy to always include color escapes, and then stripping them out in pants, based on the [GLOBAL].colors setting. This seems to only be possible via an undocumented environment variable: `MYPY_FORCE_COLOR`: python/mypy#7771, https://github.com/python/mypy/blob/03638dd670373db0b8f00cc3bcec256d09729d06/mypy/util.py#L543 Doing this also requires setting `TERM=linux` so that curses can find appropriate info in the terminfo database. [ci skip-rust] [ci skip-build-wheels]
1 parent 7270296 commit 564e6ec

File tree

4 files changed

+57
-3
lines changed

4 files changed

+57
-3
lines changed

src/python/pants/backend/python/typecheck/mypy/rules.py

+13-1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
from pants.engine.rules import Get, MultiGet, collect_rules, rule, rule_helper
4545
from pants.engine.target import CoarsenedTargets, FieldSet, Target
4646
from pants.engine.unions import UnionRule
47+
from pants.option.global_options import GlobalOptions
4748
from pants.util.logging import LogLevel
4849
from pants.util.ordered_set import FrozenOrderedSet, OrderedSet
4950
from pants.util.strutil import pluralize, shell_quote
@@ -144,6 +145,7 @@ async def mypy_typecheck_partition(
144145
mkdir: MkdirBinary,
145146
cp: CpBinary,
146147
mv: MvBinary,
148+
global_options: GlobalOptions,
147149
) -> CheckResult:
148150
# MyPy requires 3.5+ to run, but uses the typed-ast library to work with 2.7, 3.4, 3.5, 3.6,
149151
# and 3.7. However, typed-ast does not understand 3.8+, so instead we must run MyPy with
@@ -319,6 +321,13 @@ async def mypy_typecheck_partition(
319321
env = {
320322
"PEX_EXTRA_SYS_PATH": ":".join(all_used_source_roots),
321323
"MYPYPATH": ":".join(all_used_source_roots),
324+
# Always emit colors to improve cache hit rates, the results are post-processed to match the
325+
# global setting
326+
"MYPY_FORCE_COLOR": "1",
327+
# Mypy needs to know the terminal so it can use appropriate escape sequences. ansi is a
328+
# reasonable lowest common denominator for the sort of escapes mypy uses (NB. TERM=xterm
329+
# uses some additional codes that colors.strip_color doesn't remove).
330+
"TERM": "ansi",
322331
# Force a fixed terminal width. This is effectively infinite, disabling mypy's
323332
# builtin truncation and line wrapping. Terminals do an acceptable job of soft-wrapping
324333
# diagnostic text and source code is typically already hard-wrapped to a limited width.
@@ -342,7 +351,10 @@ async def mypy_typecheck_partition(
342351
result = await Get(FallibleProcessResult, Process, process)
343352
report = await Get(Digest, RemovePrefix(result.output_digest, REPORT_DIR))
344353
return CheckResult.from_fallible_process_result(
345-
result, partition_description=partition.description(), report=report
354+
result,
355+
partition_description=partition.description(),
356+
report=report,
357+
strip_formatting=not global_options.colors,
346358
)
347359

348360

src/python/pants/backend/python/typecheck/mypy/rules_integration_test.py

+2
Original file line numberDiff line numberDiff line change
@@ -900,4 +900,6 @@ class incredibly_long_type_name_to_force_wrapping_if_mypy_wrapped_error_messages
900900
assert re.search(
901901
"error:.*incredibly_long_type_name.*incredibly_long_attribute_name", result[0].stdout
902902
)
903+
# at least one escape sequence that sets text color (red)
904+
assert "\033[31m" in result[0].stdout
903905
assert result[0].report == EMPTY_DIGEST

src/python/pants/core/goals/check.py

+6-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
from dataclasses import dataclass
88
from typing import Any, Iterable, cast
99

10+
import colors
11+
1012
from pants.core.goals.lint import REPORT_DIR as REPORT_DIR # noqa: F401
1113
from pants.core.goals.style_request import (
1214
StyleRequest,
@@ -46,10 +48,13 @@ def from_fallible_process_result(
4648
*,
4749
partition_description: str | None = None,
4850
strip_chroot_path: bool = False,
51+
strip_formatting: bool = False,
4952
report: Digest = EMPTY_DIGEST,
5053
) -> CheckResult:
5154
def prep_output(s: bytes) -> str:
52-
return strip_v2_chroot_path(s) if strip_chroot_path else s.decode()
55+
chroot = strip_v2_chroot_path(s) if strip_chroot_path else s.decode()
56+
formatting = cast(str, colors.strip_color(chroot)) if strip_formatting else chroot
57+
return formatting
5358

5459
return CheckResult(
5560
exit_code=process_result.exit_code,

src/python/pants/core/goals/check_test.py

+36-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
from textwrap import dedent
99
from typing import Iterable, Optional, Sequence, Tuple, Type
1010

11+
import pytest
12+
1113
from pants.core.goals.check import (
1214
Check,
1315
CheckRequest,
@@ -18,7 +20,9 @@
1820
)
1921
from pants.core.util_rules.distdir import DistDir
2022
from pants.engine.addresses import Address
21-
from pants.engine.fs import Workspace
23+
from pants.engine.fs import EMPTY_DIGEST, EMPTY_FILE_DIGEST, Workspace
24+
from pants.engine.platform import Platform
25+
from pants.engine.process import FallibleProcessResult, ProcessResultMetadata
2226
from pants.engine.target import FieldSet, MultipleSourcesField, Target, Targets
2327
from pants.engine.unions import UnionMembership
2428
from pants.testutil.option_util import create_options_bootstrapper, create_subsystem
@@ -242,3 +246,34 @@ def test_streaming_output_partitions() -> None:
242246
243247
"""
244248
)
249+
250+
251+
@pytest.mark.parametrize(
252+
("strip_chroot_path", "strip_formatting", "expected"),
253+
[
254+
(False, False, "\033[0;31m/var/pants-sandbox-123/red/path.py\033[0m \033[1mbold\033[0m"),
255+
(False, True, "/var/pants-sandbox-123/red/path.py bold"),
256+
(True, False, "\033[0;31mred/path.py\033[0m \033[1mbold\033[0m"),
257+
(True, True, "red/path.py bold"),
258+
],
259+
)
260+
def test_from_fallible_process_result_output_prepping(
261+
strip_chroot_path: bool, strip_formatting: bool, expected: str
262+
) -> None:
263+
result = CheckResult.from_fallible_process_result(
264+
FallibleProcessResult(
265+
exit_code=0,
266+
stdout=b"stdout \033[0;31m/var/pants-sandbox-123/red/path.py\033[0m \033[1mbold\033[0m",
267+
stdout_digest=EMPTY_FILE_DIGEST,
268+
stderr=b"stderr \033[0;31m/var/pants-sandbox-123/red/path.py\033[0m \033[1mbold\033[0m",
269+
stderr_digest=EMPTY_FILE_DIGEST,
270+
output_digest=EMPTY_DIGEST,
271+
platform=Platform.current,
272+
metadata=ProcessResultMetadata(0, "ran_locally", 0),
273+
),
274+
strip_chroot_path=strip_chroot_path,
275+
strip_formatting=strip_formatting,
276+
)
277+
278+
assert result.stdout == "stdout " + expected
279+
assert result.stderr == "stderr " + expected

0 commit comments

Comments
 (0)