FmtResult now operates on Snapshots (#14865)

In order to handle the upcoming change to "unify" both `fmt` and `lint` processes for formatters, we need to have the `message` of `FmtResult` be based on a diff of 2 `Snapshot`s. Since `message` can't participate in `async/await`, we need to change `FmtResult` so it already has those snapshots available (namely `input` and `output`). This is unfortunately boilerplate for the callers, but that can potentially be cleaned up later a myriad of ways. This also should be essentially no overhead as `fmt.py` was already doing this query (although it was doing it conditionally. I suspect doing it unconditionally will have very little impact on perf because this is guaranteed to be cached if the output digest differs from the input). [ci skip-rust]
pantsbuild · Mar 22, 2022 · dac2076 · dac2076
1 parent 7f2fefd
commit dac2076
Show file tree

Hide file tree

Showing 25 changed files with 225 additions and 178 deletions.
diff --git a/src/python/pants/backend/go/lint/gofmt/rules.py b/src/python/pants/backend/go/lint/gofmt/rules.py
@@ -16,6 +16,7 @@
 from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
 from pants.engine.fs import Digest
+from pants.engine.internals.native_engine import Snapshot
 from pants.engine.internals.selectors import Get
 from pants.engine.process import FallibleProcessResult, Process, ProcessResult
 from pants.engine.rules import collect_rules, rule
@@ -50,7 +51,7 @@ class SetupRequest:
 @dataclass(frozen=True)
 class Setup:
     process: Process
-    original_digest: Digest
+    original_snapshot: Snapshot
 
 
 @rule(level=LogLevel.DEBUG)
@@ -77,7 +78,7 @@ async def setup_gofmt(setup_request: SetupRequest, goroot: GoRoot) -> Setup:
         description=f"Run gofmt on {pluralize(len(source_files_snapshot.files), 'file')}.",
         level=LogLevel.DEBUG,
     )
-    return Setup(process=process, original_digest=source_files_snapshot.digest)
+    return Setup(process=process, original_snapshot=source_files_snapshot)
 
 
 @rule(desc="Format with gofmt")
@@ -86,8 +87,13 @@ async def gofmt_fmt(request: GofmtRequest, gofmt: GofmtSubsystem) -> FmtResult:
         return FmtResult.skip(formatter_name=request.name)
     setup = await Get(Setup, SetupRequest(request, check_only=False))
     result = await Get(ProcessResult, Process, setup.process)
-    return FmtResult.from_process_result(
-        result, original_digest=setup.original_digest, formatter_name=request.name
+    output_snapshot = await Get(Snapshot, Digest, result.output_digest)
+    return FmtResult(
+        setup.original_snapshot,
+        output_snapshot,
+        stdout=result.stdout.decode(),
+        stderr=result.stderr.decode(),
+        formatter_name=request.name,
     )
 
 

diff --git a/src/python/pants/backend/go/lint/gofmt/rules_integration_test.py b/src/python/pants/backend/go/lint/gofmt/rules_integration_test.py
@@ -26,6 +26,7 @@
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
 from pants.engine.addresses import Address
 from pants.engine.fs import CreateDigest, Digest, FileContent
+from pants.engine.internals.native_engine import Snapshot
 from pants.engine.target import Target
 from pants.testutil.rule_runner import QueryRule, RuleRunner
 
@@ -128,9 +129,10 @@ def run_gofmt(
     return lint_results.results, fmt_result
 
 
-def get_digest(rule_runner: RuleRunner, source_files: dict[str, str]) -> Digest:
+def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot:
     files = [FileContent(path, content.encode()) for path, content in source_files.items()]
-    return rule_runner.request(Digest, [CreateDigest(files)])
+    digest = rule_runner.request(Digest, [CreateDigest(files)])
+    return rule_runner.request(Snapshot, [digest])
 
 
 def test_passing(rule_runner: RuleRunner) -> None:
@@ -143,7 +145,7 @@ def test_passing(rule_runner: RuleRunner) -> None:
     assert lint_results[0].exit_code == 0
     assert lint_results[0].stderr == ""
     assert fmt_result.stdout == ""
-    assert fmt_result.output == get_digest(rule_runner, {"f.go": GOOD_FILE})
+    assert fmt_result.output == get_snapshot(rule_runner, {"f.go": GOOD_FILE})
     assert fmt_result.did_change is False
 
 
@@ -157,7 +159,7 @@ def test_failing(rule_runner: RuleRunner) -> None:
     assert lint_results[0].exit_code == 1
     assert "f.go" in lint_results[0].stdout
     assert fmt_result.stderr == ""
-    assert fmt_result.output == get_digest(rule_runner, {"f.go": FIXED_BAD_FILE})
+    assert fmt_result.output == get_snapshot(rule_runner, {"f.go": FIXED_BAD_FILE})
     assert fmt_result.did_change is True
 
 
@@ -176,7 +178,7 @@ def test_mixed_sources(rule_runner: RuleRunner) -> None:
     assert lint_results[0].exit_code == 1
     assert "bad.go" in lint_results[0].stdout
     assert "good.go" not in lint_results[0].stdout
-    assert fmt_result.output == get_digest(
+    assert fmt_result.output == get_snapshot(
         rule_runner, {"good.go": GOOD_FILE, "bad.go": FIXED_BAD_FILE}
     )
     assert fmt_result.did_change is True
@@ -199,7 +201,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None:
     assert lint_results[0].exit_code == 1
     assert "bad/f.go" in lint_results[0].stdout
     assert "good/f.go" not in lint_results[0].stdout
-    assert fmt_result.output == get_digest(
+    assert fmt_result.output == get_snapshot(
         rule_runner, {"good/f.go": GOOD_FILE, "bad/f.go": FIXED_BAD_FILE}
     )
     assert fmt_result.did_change is True

diff --git a/src/python/pants/backend/java/lint/google_java_format/rules.py b/src/python/pants/backend/java/lint/google_java_format/rules.py
@@ -12,6 +12,7 @@
 from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
 from pants.engine.fs import Digest
+from pants.engine.internals.native_engine import Snapshot
 from pants.engine.internals.selectors import Get, MultiGet
 from pants.engine.process import FallibleProcessResult, ProcessResult
 from pants.engine.rules import collect_rules, rule
@@ -22,7 +23,7 @@
 from pants.jvm.resolve.coursier_fetch import ToolClasspath, ToolClasspathRequest
 from pants.jvm.resolve.jvm_tool import GenerateJvmLockfileFromTool
 from pants.util.logging import LogLevel
-from pants.util.strutil import pluralize
+from pants.util.strutil import pluralize, strip_v2_chroot_path
 
 logger = logging.getLogger(__name__)
 
@@ -56,7 +57,7 @@ class SetupRequest:
 @dataclass(frozen=True)
 class Setup:
     process: JvmProcess
-    original_digest: Digest
+    original_snapshot: Snapshot
 
 
 @rule(level=LogLevel.DEBUG)
@@ -118,7 +119,7 @@ async def setup_google_java_format(
         level=LogLevel.DEBUG,
     )
 
-    return Setup(process, original_digest=source_files_snapshot.digest)
+    return Setup(process, original_snapshot=source_files_snapshot)
 
 
 @rule(desc="Format with Google Java Format", level=LogLevel.DEBUG)
@@ -129,11 +130,13 @@ async def google_java_format_fmt(
         return FmtResult.skip(formatter_name=request.name)
     setup = await Get(Setup, SetupRequest(request, check_only=False))
     result = await Get(ProcessResult, JvmProcess, setup.process)
-    return FmtResult.from_process_result(
-        result,
-        original_digest=setup.original_digest,
+    output_snapshot = await Get(Snapshot, Digest, result.output_digest)
+    return FmtResult(
+        setup.original_snapshot,
+        output_snapshot,
+        stdout=strip_v2_chroot_path(result.stdout),
+        stderr=strip_v2_chroot_path(result.stderr),
         formatter_name=request.name,
-        strip_chroot_path=True,
     )
 
 

diff --git a/src/python/pants/backend/java/lint/google_java_format/rules_integration_test.py b/src/python/pants/backend/java/lint/google_java_format/rules_integration_test.py
@@ -19,6 +19,7 @@
 from pants.core.util_rules import config_files
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
 from pants.engine.fs import CreateDigest, Digest, FileContent
+from pants.engine.internals.native_engine import Snapshot
 from pants.engine.rules import QueryRule
 from pants.engine.target import Target
 from pants.jvm import classpath, jdk_rules
@@ -102,9 +103,10 @@ def run_google_java_format(
     return lint_results.results, fmt_result
 
 
-def get_digest(rule_runner: RuleRunner, source_files: dict[str, str]) -> Digest:
+def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot:
     files = [FileContent(path, content.encode()) for path, content in source_files.items()]
-    return rule_runner.request(Digest, [CreateDigest(files)])
+    digest = rule_runner.request(Digest, [CreateDigest(files)])
+    return rule_runner.request(Snapshot, [digest])
 
 
 def test_passing(rule_runner: RuleRunner) -> None:
@@ -113,7 +115,7 @@ def test_passing(rule_runner: RuleRunner) -> None:
     lint_results, fmt_result = run_google_java_format(rule_runner, [tgt])
     assert len(lint_results) == 1
     assert lint_results[0].exit_code == 0
-    assert fmt_result.output == get_digest(rule_runner, {"Foo.java": GOOD_FILE})
+    assert fmt_result.output == get_snapshot(rule_runner, {"Foo.java": GOOD_FILE})
     assert fmt_result.did_change is False
 
 
@@ -124,7 +126,7 @@ def test_failing(rule_runner: RuleRunner) -> None:
     assert len(lint_results) == 1
     assert lint_results[0].exit_code == 1
     assert "The following Java files require formatting:\nBar.java\n\n" == lint_results[0].stdout
-    assert fmt_result.output == get_digest(rule_runner, {"Bar.java": FIXED_BAD_FILE})
+    assert fmt_result.output == get_snapshot(rule_runner, {"Bar.java": FIXED_BAD_FILE})
     assert fmt_result.did_change is True
 
 
@@ -140,7 +142,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None:
     assert len(lint_results) == 1
     assert lint_results[0].exit_code == 1
     assert "The following Java files require formatting:\nBar.java\n\n" == lint_results[0].stdout
-    assert fmt_result.output == get_digest(
+    assert fmt_result.output == get_snapshot(
         rule_runner, {"Foo.java": GOOD_FILE, "Bar.java": FIXED_BAD_FILE}
     )
     assert fmt_result.did_change is True
diff --git a/src/python/pants/backend/python/lint/autoflake/rules.py b/src/python/pants/backend/python/lint/autoflake/rules.py
@@ -13,12 +13,13 @@
 from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
 from pants.engine.fs import Digest
+from pants.engine.internals.native_engine import Snapshot
 from pants.engine.process import FallibleProcessResult, Process, ProcessResult
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
 from pants.engine.target import FieldSet, Target
 from pants.engine.unions import UnionRule
 from pants.util.logging import LogLevel
-from pants.util.strutil import pluralize
+from pants.util.strutil import pluralize, strip_v2_chroot_path
 
 
 @dataclass(frozen=True)
@@ -47,7 +48,7 @@ class SetupRequest:
 @dataclass(frozen=True)
 class Setup:
     process: Process
-    original_digest: Digest
+    original_snapshot: Snapshot
 
 
 def generate_argv(
@@ -91,7 +92,7 @@ async def setup_autoflake(setup_request: SetupRequest, autoflake: Autoflake) ->
             level=LogLevel.DEBUG,
         ),
     )
-    return Setup(process, original_digest=source_files_snapshot.digest)
+    return Setup(process, original_snapshot=source_files_snapshot)
 
 
 @rule(desc="Format with Autoflake", level=LogLevel.DEBUG)
@@ -100,11 +101,13 @@ async def autoflake_fmt(request: AutoflakeRequest, autoflake: Autoflake) -> FmtR
         return FmtResult.skip(formatter_name=request.name)
     setup = await Get(Setup, SetupRequest(request, check_only=False))
     result = await Get(ProcessResult, Process, setup.process)
-    return FmtResult.from_process_result(
-        result,
-        original_digest=setup.original_digest,
+    output_snapshot = await Get(Snapshot, Digest, result.output_digest)
+    return FmtResult(
+        setup.original_snapshot,
+        output_snapshot,
+        stdout=strip_v2_chroot_path(result.stdout),
+        stderr=strip_v2_chroot_path(result.stderr),
         formatter_name=request.name,
-        strip_chroot_path=True,
     )
 
 

diff --git a/src/python/pants/backend/python/lint/autoflake/rules_integration_test.py b/src/python/pants/backend/python/lint/autoflake/rules_integration_test.py
@@ -17,6 +17,7 @@
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
 from pants.engine.addresses import Address
 from pants.engine.fs import CreateDigest, Digest, FileContent
+from pants.engine.internals.native_engine import Snapshot
 from pants.engine.target import Target
 from pants.testutil.python_interpreter_selection import all_major_minor_python_versions
 from pants.testutil.rule_runner import QueryRule, RuleRunner
@@ -71,9 +72,10 @@ def run_autoflake(
     return lint_results.results, fmt_result
 
 
-def get_digest(rule_runner: RuleRunner, source_files: dict[str, str]) -> Digest:
+def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot:
     files = [FileContent(path, content.encode()) for path, content in source_files.items()]
-    return rule_runner.request(Digest, [CreateDigest(files)])
+    digest = rule_runner.request(Digest, [CreateDigest(files)])
+    return rule_runner.request(Snapshot, [digest])
 
 
 @pytest.mark.platform_specific_behavior
@@ -93,7 +95,7 @@ def test_passing_source(rule_runner: RuleRunner, major_minor_interpreter: str) -
     assert lint_results[0].exit_code == 0
     assert lint_results[0].stderr == ""
     assert fmt_result.stdout == ""
-    assert fmt_result.output == get_digest(rule_runner, {"f.py": GOOD_FILE})
+    assert fmt_result.output == get_snapshot(rule_runner, {"f.py": GOOD_FILE})
     assert fmt_result.did_change is False
 
 
@@ -104,7 +106,7 @@ def test_failing_source(rule_runner: RuleRunner) -> None:
     assert len(lint_results) == 1
     assert lint_results[0].exit_code == 1
     assert "f.py: Unused imports/variables detected" in lint_results[0].stdout
-    assert fmt_result.output == get_digest(rule_runner, {"f.py": FIXED_BAD_FILE})
+    assert fmt_result.output == get_snapshot(rule_runner, {"f.py": FIXED_BAD_FILE})
     assert fmt_result.did_change is True
 
 
@@ -121,7 +123,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None:
     assert lint_results[0].exit_code == 1
     assert "bad.py: Unused imports/variables detected" in lint_results[0].stdout
     assert "good.py" not in lint_results[0].stderr
-    assert fmt_result.output == get_digest(
+    assert fmt_result.output == get_snapshot(
         rule_runner, {"good.py": GOOD_FILE, "bad.py": FIXED_BAD_FILE}
     )
     assert fmt_result.did_change is True
@@ -154,7 +156,7 @@ def test_stub_files(rule_runner: RuleRunner) -> None:
     lint_results, fmt_result = run_autoflake(rule_runner, good_tgts)
     assert len(lint_results) == 1 and lint_results[0].exit_code == 0
     assert lint_results[0].stderr == "" and fmt_result.stdout == ""
-    assert fmt_result.output == get_digest(
+    assert fmt_result.output == get_snapshot(
         rule_runner, {"good.py": GOOD_FILE, "good.pyi": GOOD_FILE}
     )
     assert not fmt_result.did_change
@@ -169,7 +171,7 @@ def test_stub_files(rule_runner: RuleRunner) -> None:
     # autoflake non-deterministically outputs only one the files that needed changes, not
     # all of them.
     assert "Unused imports/variables detected" in lint_results[0].stdout
-    assert fmt_result.output == get_digest(
+    assert fmt_result.output == get_snapshot(
         rule_runner, {"bad.py": FIXED_BAD_FILE, "bad.pyi": FIXED_BAD_FILE}
     )
     assert fmt_result.did_change
diff --git a/src/python/pants/backend/python/lint/black/rules.py b/src/python/pants/backend/python/lint/black/rules.py
@@ -16,12 +16,13 @@
 from pants.core.util_rules.config_files import ConfigFiles, ConfigFilesRequest
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
 from pants.engine.fs import Digest, MergeDigests
+from pants.engine.internals.native_engine import Snapshot
 from pants.engine.process import FallibleProcessResult, Process, ProcessResult
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
 from pants.engine.target import FieldSet, Target
 from pants.engine.unions import UnionRule
 from pants.util.logging import LogLevel
-from pants.util.strutil import pluralize
+from pants.util.strutil import pluralize, strip_v2_chroot_path
 
 
 @dataclass(frozen=True)
@@ -50,7 +51,7 @@ class SetupRequest:
 @dataclass(frozen=True)
 class Setup:
     process: Process
-    original_digest: Digest
+    original_snapshot: Snapshot
 
 
 def generate_argv(source_files: SourceFiles, black: Black, *, check_only: bool) -> Tuple[str, ...]:
@@ -126,7 +127,7 @@ async def setup_black(
             level=LogLevel.DEBUG,
         ),
     )
-    return Setup(process, original_digest=source_files_snapshot.digest)
+    return Setup(process, original_snapshot=source_files_snapshot)
 
 
 @rule(desc="Format with Black", level=LogLevel.DEBUG)
@@ -135,11 +136,13 @@ async def black_fmt(request: BlackRequest, black: Black) -> FmtResult:
         return FmtResult.skip(formatter_name=request.name)
     setup = await Get(Setup, SetupRequest(request, check_only=False))
     result = await Get(ProcessResult, Process, setup.process)
-    return FmtResult.from_process_result(
-        result,
-        original_digest=setup.original_digest,
+    output_snapshot = await Get(Snapshot, Digest, result.output_digest)
+    return FmtResult(
+        setup.original_snapshot,
+        output_snapshot,
+        stdout=strip_v2_chroot_path(result.stdout),
+        stderr=strip_v2_chroot_path(result.stderr),
         formatter_name=request.name,
-        strip_chroot_path=True,
     )