From ace3df657f9024af5886deee668213e080a67b09 Mon Sep 17 00:00:00 2001 From: Jonas Stendahl Date: Fri, 25 Mar 2022 03:30:40 +0100 Subject: [PATCH 1/6] Add Protobuf formatting using buf format --- .../codegen/protobuf/lint/buf/format_rules.py | 164 ++++++++++++++++++ .../lint/buf/format_rules_integration_test.py | 143 +++++++++++++++ .../lint/buf/{rules.py => lint_rules.py} | 4 +- ...test.py => lint_rules_integration_test.py} | 4 +- .../codegen/protobuf/lint/buf/register.py | 5 +- .../codegen/protobuf/lint/buf/subsystem.py | 14 +- 6 files changed, 321 insertions(+), 13 deletions(-) create mode 100644 src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py create mode 100644 src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py rename src/python/pants/backend/codegen/protobuf/lint/buf/{rules.py => lint_rules.py} (96%) rename src/python/pants/backend/codegen/protobuf/lint/buf/{rules_integration_test.py => lint_rules_integration_test.py} (97%) diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py new file mode 100644 index 00000000000..8619d6772b5 --- /dev/null +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py @@ -0,0 +1,164 @@ +# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). +from dataclasses import dataclass + +from pants.backend.codegen.protobuf.lint.buf.skip_field import SkipBufField +from pants.backend.codegen.protobuf.lint.buf.subsystem import BufSubsystem +from pants.backend.codegen.protobuf.target_types import ( + ProtobufDependenciesField, + ProtobufSourceField, +) +from pants.core.goals.fmt import FmtRequest, FmtResult +from pants.core.goals.lint import LintResult, LintResults, LintTargetsRequest +from pants.core.util_rules.external_tool import DownloadedExternalTool, ExternalToolRequest +from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest +from pants.core.util_rules.system_binaries import ( + SEARCH_PATHS, + BinaryPath, + BinaryPathRequest, + BinaryPaths, + BinaryShims, + BinaryShimsRequest, +) +from pants.engine.fs import Digest, MergeDigests +from pants.engine.internals.native_engine import Snapshot +from pants.engine.platform import Platform +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 + + +@dataclass(frozen=True) +class BufFieldSet(FieldSet): + required_fields = (ProtobufSourceField,) + + sources: ProtobufSourceField + dependencies: ProtobufDependenciesField + + @classmethod + def opt_out(cls, tgt: Target) -> bool: + return tgt.get(SkipBufField).value + + +class BufRequest(LintTargetsRequest, FmtRequest): + field_set_type = BufFieldSet + name = BufSubsystem.options_scope + + +@dataclass(frozen=True) +class SetupRequest: + request: BufRequest + check_only: bool + + +@dataclass(frozen=True) +class Setup: + process: Process + original_snapshot: Snapshot + + +class DiffBinary(BinaryPath): + pass + + +@rule(desc="Finding the `diff` binary", level=LogLevel.DEBUG) +async def find_diff() -> DiffBinary: + request = BinaryPathRequest(binary_name="diff", search_path=SEARCH_PATHS) + paths = await Get(BinaryPaths, BinaryPathRequest, request) + first_path = paths.first_path_or_raise( + request, rationale="buf format requires diff in linting mode" + ) + return DiffBinary(first_path.path, first_path.fingerprint) + + +@rule(level=LogLevel.DEBUG) +async def setup_buf_format(setup_request: SetupRequest, buf: BufSubsystem) -> Setup: + download_buf_get = Get( + DownloadedExternalTool, ExternalToolRequest, buf.get_request(Platform.current) + ) + binary_shims_get = Get( + BinaryShims, + BinaryShimsRequest, + BinaryShimsRequest.for_binaries( + "diff", + rationale="buf format requires diff in linting mode", + output_directory=".bin", + search_path=SEARCH_PATHS, + ), + ) + source_files_get = Get( + SourceFiles, + SourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets), + ) + downloaded_buf, binary_shims, source_files = await MultiGet( + download_buf_get, binary_shims_get, source_files_get + ) + + source_files_snapshot = ( + source_files.snapshot + if setup_request.request.prior_formatter_result is None + else setup_request.request.prior_formatter_result + ) + + input_digest = await Get( + Digest, + MergeDigests((source_files_snapshot.digest, downloaded_buf.digest, binary_shims.digest)), + ) + + argv = [ + downloaded_buf.exe, + "format", + # If linting, use `-d` to error with a diff. Else, write the change with `-w`. + *(["-d"] if setup_request.check_only else ["-w"]), + *buf.args, + "--path", + ",".join(source_files_snapshot.files), + ] + process = Process( + argv=argv, + input_digest=input_digest, + output_files=source_files_snapshot.files, + description=f"Run buf format on {pluralize(len(setup_request.request.field_sets), 'file')}.", + level=LogLevel.DEBUG, + env={ + "PATH": binary_shims.bin_directory, + }, + ) + return Setup(process, original_snapshot=source_files_snapshot) + + +@rule(desc="Format with buf format", level=LogLevel.DEBUG) +async def run_buf_format(request: BufRequest, buf: BufSubsystem) -> FmtResult: + if buf.skip: + return FmtResult.skip(formatter_name=request.name) + setup = await Get(Setup, SetupRequest(request, check_only=False)) + result = await Get(ProcessResult, Process, setup.process) + 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, + ) + + +@rule(desc="Lint with buf format", level=LogLevel.DEBUG) +async def run_buf_lint(request: BufRequest, buf: BufSubsystem) -> LintResults: + if buf.skip: + return LintResults([], linter_name=request.name) + setup = await Get(Setup, SetupRequest(request, check_only=True)) + result = await Get(FallibleProcessResult, Process, setup.process) + + return LintResults([LintResult.from_fallible_process_result(result)], linter_name=request.name) + + +def rules(): + return [ + *collect_rules(), + UnionRule(FmtRequest, BufRequest), + UnionRule(LintTargetsRequest, BufRequest), + ] diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py new file mode 100644 index 00000000000..7244ab9a462 --- /dev/null +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py @@ -0,0 +1,143 @@ +# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from __future__ import annotations + +import pytest + +from pants.backend.codegen.protobuf.lint.buf.format_rules import BufFieldSet, BufRequest +from pants.backend.codegen.protobuf.lint.buf.format_rules import rules as buf_rules +from pants.backend.codegen.protobuf.target_types import ProtobufSourcesGeneratorTarget +from pants.backend.codegen.protobuf.target_types import rules as target_types_rules +from pants.core.goals.fmt import FmtResult +from pants.core.goals.lint import LintResult, LintResults +from pants.core.util_rules import config_files, external_tool, source_files +from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest +from pants.engine.addresses import Address +from pants.engine.fs import CreateDigest, FileContent +from pants.engine.internals.native_engine import Digest, Snapshot +from pants.engine.target import Target +from pants.testutil.rule_runner import QueryRule, RuleRunner + + +@pytest.fixture +def rule_runner() -> RuleRunner: + return RuleRunner( + rules=[ + *buf_rules(), + *config_files.rules(), + *external_tool.rules(), + *source_files.rules(), + *target_types_rules(), + QueryRule(LintResults, [BufRequest]), + QueryRule(FmtResult, [BufRequest]), + QueryRule(SourceFiles, [SourceFilesRequest]), + ], + target_types=[ProtobufSourcesGeneratorTarget], + ) + + +GOOD_FILE = 'syntax = "proto3";\n\npackage foo.v1;\nmessage Foo {}\n' +BAD_FILE = 'syntax = "proto3";\n\npackage foo.v1;\nmessage Foo {\n}\n' + + +def run_buf( + rule_runner: RuleRunner, + targets: list[Target], + *, + extra_args: list[str] | None = None, +) -> tuple[tuple[LintResult, ...], FmtResult]: + rule_runner.set_options( + [ + "--backend-packages=pants.backend.codegen.protobuf.lint.buf", + *(extra_args or ()), + ], + env_inherit={"PATH"}, + ) + field_sets = [BufFieldSet.create(tgt) for tgt in targets] + results = rule_runner.request(LintResults, [BufRequest(field_sets)]) + input_sources = rule_runner.request( + SourceFiles, + [ + SourceFilesRequest(field_set.sources for field_set in field_sets), + ], + ) + fmt_result = rule_runner.request( + FmtResult, + [ + BufRequest(field_sets, prior_formatter_result=input_sources.snapshot), + ], + ) + + return results.results, fmt_result + + +def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot: + files = [FileContent(path, content.encode()) for path, content in source_files.items()] + digest = rule_runner.request(Digest, [CreateDigest(files)]) + return rule_runner.request(Snapshot, [digest]) + + +def test_passing(rule_runner: RuleRunner) -> None: + rule_runner.write_files({"f.proto": GOOD_FILE, "BUILD": "protobuf_sources(name='t')"}) + tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.proto")) + lint_results, fmt_result = run_buf(rule_runner, [tgt]) + assert len(lint_results) == 1 + assert lint_results[0].exit_code == 0 + assert lint_results[0].stdout == "" + assert lint_results[0].stderr == "" + assert fmt_result.stdout == "" + assert fmt_result.output == get_snapshot(rule_runner, {"f.proto": GOOD_FILE}) + assert fmt_result.did_change is False + + +def test_failing(rule_runner: RuleRunner) -> None: + rule_runner.write_files({"f.proto": BAD_FILE, "BUILD": "protobuf_sources(name='t')"}) + tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.proto")) + lint_results, fmt_result = run_buf(rule_runner, [tgt]) + assert len(lint_results) == 1 + assert lint_results[0].exit_code == 1 + assert "f.proto.orig" in lint_results[0].stdout + assert fmt_result.output == get_snapshot(rule_runner, {"f.proto": GOOD_FILE}) + assert fmt_result.did_change is True + + +def test_multiple_targets(rule_runner: RuleRunner) -> None: + rule_runner.write_files( + {"good.proto": GOOD_FILE, "bad.proto": BAD_FILE, "BUILD": "protobuf_sources(name='t')"} + ) + tgts = [ + rule_runner.get_target(Address("", target_name="t", relative_file_path="good.proto")), + rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.proto")), + ] + lint_results, fmt_result = run_buf(rule_runner, tgts) + assert len(lint_results) == 1 + assert lint_results[0].exit_code == 1 + assert "bad.proto.orig" in lint_results[0].stdout + assert "good.proto" not in lint_results[0].stdout + assert fmt_result.output == get_snapshot( + rule_runner, {"good.proto": GOOD_FILE, "bad.proto": GOOD_FILE} + ) + assert fmt_result.did_change is True + + +def test_passthrough_args(rule_runner: RuleRunner) -> None: + rule_runner.write_files({"f.proto": GOOD_FILE, "BUILD": "protobuf_sources(name='t')"}) + tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.proto")) + lint_results, fmt_result = run_buf(rule_runner, [tgt], extra_args=["--buf-lint-args=--debug"]) + assert len(lint_results) == 1 + assert lint_results[0].exit_code == 0 + assert lint_results[0].stdout == "" + assert "DEBUG" in lint_results[0].stderr + assert fmt_result.stdout == "" + assert fmt_result.output == get_snapshot(rule_runner, {"f.proto": GOOD_FILE}) + assert fmt_result.did_change is False + + +def test_skip(rule_runner: RuleRunner) -> None: + rule_runner.write_files({"f.proto": BAD_FILE, "BUILD": "protobuf_sources(name='t')"}) + tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.proto")) + lint_results, fmt_result = run_buf(rule_runner, [tgt], extra_args=["--buf-lint-skip"]) + assert not lint_results + assert fmt_result.skipped is True + assert fmt_result.did_change is False diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/rules.py b/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py similarity index 96% rename from src/python/pants/backend/codegen/protobuf/lint/buf/rules.py rename to src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py index 7940d0919da..a42359f2371 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/rules.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py @@ -39,7 +39,7 @@ class BufRequest(LintTargetsRequest): name = BufSubsystem.options_scope -@rule(desc="Lint with Buf", level=LogLevel.DEBUG) +@rule(desc="Lint with buf lint", level=LogLevel.DEBUG) async def run_buf(request: BufRequest, buf: BufSubsystem) -> LintResults: if buf.skip: return LintResults([], linter_name=request.name) @@ -96,7 +96,7 @@ async def run_buf(request: BufRequest, buf: BufSubsystem) -> LintResults: ",".join(target_sources_stripped.snapshot.files), ], input_digest=input_digest, - description=f"Run Buf on {pluralize(len(request.field_sets), 'file')}.", + description=f"Run buf lint on {pluralize(len(request.field_sets), 'file')}.", level=LogLevel.DEBUG, ), ) diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/rules_integration_test.py b/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules_integration_test.py similarity index 97% rename from src/python/pants/backend/codegen/protobuf/lint/buf/rules_integration_test.py rename to src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules_integration_test.py index 4506b2b5d3e..323257ba90a 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/rules_integration_test.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules_integration_test.py @@ -8,8 +8,8 @@ import pytest -from pants.backend.codegen.protobuf.lint.buf.rules import BufFieldSet, BufRequest -from pants.backend.codegen.protobuf.lint.buf.rules import rules as buf_rules +from pants.backend.codegen.protobuf.lint.buf.lint_rules import BufFieldSet, BufRequest +from pants.backend.codegen.protobuf.lint.buf.lint_rules import rules as buf_rules from pants.backend.codegen.protobuf.target_types import ProtobufSourcesGeneratorTarget from pants.backend.codegen.protobuf.target_types import rules as target_types_rules from pants.core.goals.lint import LintResult, LintResults diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/register.py b/src/python/pants/backend/codegen/protobuf/lint/buf/register.py index 43f805fccfa..75753b523a3 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/register.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/register.py @@ -2,8 +2,9 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from pants.backend.codegen.protobuf.lint.buf import skip_field -from pants.backend.codegen.protobuf.lint.buf.rules import rules as buf_rules +from pants.backend.codegen.protobuf.lint.buf.format_rules import rules as buf_format_rules +from pants.backend.codegen.protobuf.lint.buf.lint_rules import rules as buf_lint_rules def rules(): - return (*buf_rules(), *skip_field.rules()) + return (*buf_format_rules(), *buf_lint_rules(), *skip_field.rules()) diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/subsystem.py b/src/python/pants/backend/codegen/protobuf/lint/buf/subsystem.py index 55e694788fc..804703558d0 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/subsystem.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/subsystem.py @@ -11,14 +11,14 @@ class BufSubsystem(TemplatedExternalTool): options_scope = "buf-lint" name = "Buf" - help = "A linter for Protocol Buffers (https://github.com/bufbuild/buf)." + help = "A linter and formatter for Protocol Buffers (https://github.com/bufbuild/buf)." - default_version = "v1.0.0" + default_version = "v1.2.1" default_known_versions = [ - "v1.0.0|linux_arm64 |c4b095268fe0fc8de2ad76c7b4677ccd75f25623d5b1f971082a6b7f43ff1eb0|13378006", - "v1.0.0|linux_x86_64|5f0ff97576cde9e43ec86959046169f18ec5bcc08e31d82dcc948d057212f7bf|14511545", - "v1.0.0|macos_arm64 |e922c277487d941c4b056cac6c1b4c6e5004e8f3dda65ae2d72d8b10da193297|15147463", - "v1.0.0|macos_x86_64|8963e1ab7685aac59b8805cc7d752b06a572b1c747a6342a9e73b94ccdf89ddb|15187858", + "v1.2.1|linux_arm64 |8c9df682691436bd9f58efa44928e6fcd68ec6dd346e35eddac271786f4c0ae3|13940426", + "v1.2.1|linux_x86_64|eb227afeaf5f5c5a5f1d2aca92926d8c89be5b7a410e5afd6dd68f2ed0c00f22|15267079", + "v1.2.1|macos_arm64 |6877c9b8f895ec4962faff551c541d9d14e12f49b899ed7e553f0dc74a69b1b8|15388080", + "v1.2.1|macos_x86_64|652b407fd08e5e664244971f4a725763ef582f26778674490658ad2ce361fe95|15954329", ] default_url_template = ( "https://github.com/bufbuild/buf/releases/download/{version}/buf-{platform}.tar.gz" @@ -30,7 +30,7 @@ class BufSubsystem(TemplatedExternalTool): "linux_x86_64": "Linux-x86_64", } - skip = SkipOption("lint") + skip = SkipOption("fmt", "lint") args = ArgsListOption(example="--error-format json") def generate_exe(self, plat: Platform) -> str: From fb8503175c892daff5f74bfa72b71cb39031601f Mon Sep 17 00:00:00 2001 From: Jonas Stendahl Date: Fri, 25 Mar 2022 03:49:32 +0100 Subject: [PATCH 2/6] Fix exit code issue --- .../backend/codegen/protobuf/lint/buf/format_rules.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py index 8619d6772b5..df2b69395bb 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py @@ -153,7 +153,16 @@ async def run_buf_lint(request: BufRequest, buf: BufSubsystem) -> LintResults: setup = await Get(Setup, SetupRequest(request, check_only=True)) result = await Get(FallibleProcessResult, Process, setup.process) - return LintResults([LintResult.from_fallible_process_result(result)], linter_name=request.name) + return LintResults( + [ + LintResult( + exit_code=0 if not result.stdout else 1, # buf format always exits with code 0 + stdout=result.stdout.decode(), + stderr=result.stderr.decode(), + ) + ], + linter_name=request.name, + ) def rules(): From 0058ef1912452b3472baa148d9d72baf8fee7073 Mon Sep 17 00:00:00 2001 From: Jonas Stendahl Date: Fri, 25 Mar 2022 11:27:03 +0100 Subject: [PATCH 3/6] Fix review issues --- .../codegen/protobuf/lint/buf/format_rules.py | 28 ++++--------------- .../codegen/protobuf/lint/buf/lint_rules.py | 2 +- .../codegen/protobuf/lint/buf/subsystem.py | 2 +- .../pants/core/util_rules/system_binaries.py | 21 ++++++++++++++ 4 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py index df2b69395bb..c6211d15753 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py @@ -13,12 +13,10 @@ from pants.core.util_rules.external_tool import DownloadedExternalTool, ExternalToolRequest from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.core.util_rules.system_binaries import ( - SEARCH_PATHS, - BinaryPath, - BinaryPathRequest, - BinaryPaths, BinaryShims, BinaryShimsRequest, + DiffBinary, + DiffBinaryRequest, ) from pants.engine.fs import Digest, MergeDigests from pants.engine.internals.native_engine import Snapshot @@ -45,7 +43,7 @@ def opt_out(cls, tgt: Target) -> bool: class BufRequest(LintTargetsRequest, FmtRequest): field_set_type = BufFieldSet - name = BufSubsystem.options_scope + name = "buf-format" @dataclass(frozen=True) @@ -60,33 +58,19 @@ class Setup: original_snapshot: Snapshot -class DiffBinary(BinaryPath): - pass - - -@rule(desc="Finding the `diff` binary", level=LogLevel.DEBUG) -async def find_diff() -> DiffBinary: - request = BinaryPathRequest(binary_name="diff", search_path=SEARCH_PATHS) - paths = await Get(BinaryPaths, BinaryPathRequest, request) - first_path = paths.first_path_or_raise( - request, rationale="buf format requires diff in linting mode" - ) - return DiffBinary(first_path.path, first_path.fingerprint) - - @rule(level=LogLevel.DEBUG) async def setup_buf_format(setup_request: SetupRequest, buf: BufSubsystem) -> Setup: + diff_binary = await Get(DiffBinary, DiffBinaryRequest()) download_buf_get = Get( DownloadedExternalTool, ExternalToolRequest, buf.get_request(Platform.current) ) binary_shims_get = Get( BinaryShims, BinaryShimsRequest, - BinaryShimsRequest.for_binaries( - "diff", + BinaryShimsRequest.for_paths( + diff_binary, rationale="buf format requires diff in linting mode", output_directory=".bin", - search_path=SEARCH_PATHS, ), ) source_files_get = Get( diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py b/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py index a42359f2371..8a730351c0e 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py @@ -36,7 +36,7 @@ def opt_out(cls, tgt: Target) -> bool: class BufRequest(LintTargetsRequest): field_set_type = BufFieldSet - name = BufSubsystem.options_scope + name = "buf-lint" @rule(desc="Lint with buf lint", level=LogLevel.DEBUG) diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/subsystem.py b/src/python/pants/backend/codegen/protobuf/lint/buf/subsystem.py index 804703558d0..137ebd5d4f2 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/subsystem.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/subsystem.py @@ -9,7 +9,7 @@ class BufSubsystem(TemplatedExternalTool): - options_scope = "buf-lint" + options_scope = "buf" name = "Buf" help = "A linter and formatter for Protocol Buffers (https://github.com/bufbuild/buf)." diff --git a/src/python/pants/core/util_rules/system_binaries.py b/src/python/pants/core/util_rules/system_binaries.py index 9600c4c724c..bd4c922a498 100644 --- a/src/python/pants/core/util_rules/system_binaries.py +++ b/src/python/pants/core/util_rules/system_binaries.py @@ -316,6 +316,10 @@ class ChmodBinary(BinaryPath): pass +class DiffBinary(BinaryPath): + pass + + # ------------------------------------------------------------------------------------------- # Binaries Rules # ------------------------------------------------------------------------------------------- @@ -632,6 +636,14 @@ async def find_chmod() -> ChmodBinary: return ChmodBinary(first_path.path, first_path.fingerprint) +@rule(desc="Finding the `diff` binary", level=LogLevel.DEBUG) +async def find_diff() -> DiffBinary: + request = BinaryPathRequest(binary_name="diff", search_path=SEARCH_PATHS) + paths = await Get(BinaryPaths, BinaryPathRequest, request) + first_path = paths.first_path_or_raise(request, rationale="compare files line by line") + return DiffBinary(first_path.path, first_path.fingerprint) + + # ------------------------------------------------------------------------------------------- # Rules for lazy requests # TODO(#12946): Get rid of this when it becomes possible to use `Get()` with only one arg. @@ -662,6 +674,10 @@ class ChmodBinaryRequest: pass +class DiffBinaryRequest: + pass + + @rule async def find_zip_wrapper(_: ZipBinaryRequest, zip_binary: ZipBinary) -> ZipBinary: return zip_binary @@ -692,5 +708,10 @@ async def find_chmod_wrapper(_: ChmodBinaryRequest, chmod_binary: ChmodBinary) - return chmod_binary +@rule +async def find_diff_wrapper(_: DiffBinaryRequest, diff_binary: DiffBinary) -> DiffBinary: + return diff_binary + + def rules(): return [*collect_rules(), *python_bootstrap.rules()] From 649f5d3bc92283f98bd852282ec406b149cec9f3 Mon Sep 17 00:00:00 2001 From: Jonas Stendahl Date: Fri, 25 Mar 2022 11:58:53 +0100 Subject: [PATCH 4/6] Rename requests and options scope --- .../codegen/protobuf/lint/buf/format_rules.py | 12 ++++++------ .../lint/buf/format_rules_integration_test.py | 14 +++++++------- .../codegen/protobuf/lint/buf/lint_rules.py | 6 +++--- .../lint/buf/lint_rules_integration_test.py | 10 +++++----- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py index c6211d15753..c419b48ca7f 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py @@ -41,14 +41,14 @@ def opt_out(cls, tgt: Target) -> bool: return tgt.get(SkipBufField).value -class BufRequest(LintTargetsRequest, FmtRequest): +class BufFormatRequest(LintTargetsRequest, FmtRequest): field_set_type = BufFieldSet name = "buf-format" @dataclass(frozen=True) class SetupRequest: - request: BufRequest + request: BufFormatRequest check_only: bool @@ -115,7 +115,7 @@ async def setup_buf_format(setup_request: SetupRequest, buf: BufSubsystem) -> Se @rule(desc="Format with buf format", level=LogLevel.DEBUG) -async def run_buf_format(request: BufRequest, buf: BufSubsystem) -> FmtResult: +async def run_buf_format(request: BufFormatRequest, buf: BufSubsystem) -> FmtResult: if buf.skip: return FmtResult.skip(formatter_name=request.name) setup = await Get(Setup, SetupRequest(request, check_only=False)) @@ -131,7 +131,7 @@ async def run_buf_format(request: BufRequest, buf: BufSubsystem) -> FmtResult: @rule(desc="Lint with buf format", level=LogLevel.DEBUG) -async def run_buf_lint(request: BufRequest, buf: BufSubsystem) -> LintResults: +async def run_buf_lint(request: BufFormatRequest, buf: BufSubsystem) -> LintResults: if buf.skip: return LintResults([], linter_name=request.name) setup = await Get(Setup, SetupRequest(request, check_only=True)) @@ -152,6 +152,6 @@ async def run_buf_lint(request: BufRequest, buf: BufSubsystem) -> LintResults: def rules(): return [ *collect_rules(), - UnionRule(FmtRequest, BufRequest), - UnionRule(LintTargetsRequest, BufRequest), + UnionRule(FmtRequest, BufFormatRequest), + UnionRule(LintTargetsRequest, BufFormatRequest), ] diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py index 7244ab9a462..eb615ee5e3d 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py @@ -5,7 +5,7 @@ import pytest -from pants.backend.codegen.protobuf.lint.buf.format_rules import BufFieldSet, BufRequest +from pants.backend.codegen.protobuf.lint.buf.format_rules import BufFieldSet, BufFormatRequest from pants.backend.codegen.protobuf.lint.buf.format_rules import rules as buf_rules from pants.backend.codegen.protobuf.target_types import ProtobufSourcesGeneratorTarget from pants.backend.codegen.protobuf.target_types import rules as target_types_rules @@ -29,8 +29,8 @@ def rule_runner() -> RuleRunner: *external_tool.rules(), *source_files.rules(), *target_types_rules(), - QueryRule(LintResults, [BufRequest]), - QueryRule(FmtResult, [BufRequest]), + QueryRule(LintResults, [BufFormatRequest]), + QueryRule(FmtResult, [BufFormatRequest]), QueryRule(SourceFiles, [SourceFilesRequest]), ], target_types=[ProtobufSourcesGeneratorTarget], @@ -55,7 +55,7 @@ def run_buf( env_inherit={"PATH"}, ) field_sets = [BufFieldSet.create(tgt) for tgt in targets] - results = rule_runner.request(LintResults, [BufRequest(field_sets)]) + results = rule_runner.request(LintResults, [BufFormatRequest(field_sets)]) input_sources = rule_runner.request( SourceFiles, [ @@ -65,7 +65,7 @@ def run_buf( fmt_result = rule_runner.request( FmtResult, [ - BufRequest(field_sets, prior_formatter_result=input_sources.snapshot), + BufFormatRequest(field_sets, prior_formatter_result=input_sources.snapshot), ], ) @@ -124,7 +124,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: def test_passthrough_args(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.proto": GOOD_FILE, "BUILD": "protobuf_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.proto")) - lint_results, fmt_result = run_buf(rule_runner, [tgt], extra_args=["--buf-lint-args=--debug"]) + lint_results, fmt_result = run_buf(rule_runner, [tgt], extra_args=["--buf-args=--debug"]) assert len(lint_results) == 1 assert lint_results[0].exit_code == 0 assert lint_results[0].stdout == "" @@ -137,7 +137,7 @@ def test_passthrough_args(rule_runner: RuleRunner) -> None: def test_skip(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.proto": BAD_FILE, "BUILD": "protobuf_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.proto")) - lint_results, fmt_result = run_buf(rule_runner, [tgt], extra_args=["--buf-lint-skip"]) + lint_results, fmt_result = run_buf(rule_runner, [tgt], extra_args=["--buf-skip"]) assert not lint_results assert fmt_result.skipped is True assert fmt_result.did_change is False diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py b/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py index 8a730351c0e..9af84d35080 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py @@ -34,13 +34,13 @@ def opt_out(cls, tgt: Target) -> bool: return tgt.get(SkipBufField).value -class BufRequest(LintTargetsRequest): +class BufLintRequest(LintTargetsRequest): field_set_type = BufFieldSet name = "buf-lint" @rule(desc="Lint with buf lint", level=LogLevel.DEBUG) -async def run_buf(request: BufRequest, buf: BufSubsystem) -> LintResults: +async def run_buf(request: BufLintRequest, buf: BufSubsystem) -> LintResults: if buf.skip: return LintResults([], linter_name=request.name) @@ -106,4 +106,4 @@ async def run_buf(request: BufRequest, buf: BufSubsystem) -> LintResults: def rules(): - return [*collect_rules(), UnionRule(LintTargetsRequest, BufRequest)] + return [*collect_rules(), UnionRule(LintTargetsRequest, BufLintRequest)] diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules_integration_test.py b/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules_integration_test.py index 323257ba90a..8cc5e80082f 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules_integration_test.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules_integration_test.py @@ -8,7 +8,7 @@ import pytest -from pants.backend.codegen.protobuf.lint.buf.lint_rules import BufFieldSet, BufRequest +from pants.backend.codegen.protobuf.lint.buf.lint_rules import BufFieldSet, BufLintRequest from pants.backend.codegen.protobuf.lint.buf.lint_rules import rules as buf_rules from pants.backend.codegen.protobuf.target_types import ProtobufSourcesGeneratorTarget from pants.backend.codegen.protobuf.target_types import rules as target_types_rules @@ -28,7 +28,7 @@ def rule_runner() -> RuleRunner: *external_tool.rules(), *stripped_source_files.rules(), *target_types_rules(), - QueryRule(LintResults, [BufRequest]), + QueryRule(LintResults, [BufLintRequest]), ], target_types=[ProtobufSourcesGeneratorTarget], ) @@ -55,7 +55,7 @@ def run_buf( ) results = rule_runner.request( LintResults, - [BufRequest(BufFieldSet.create(tgt) for tgt in targets)], + [BufLintRequest(BufFieldSet.create(tgt) for tgt in targets)], ) return results.results @@ -133,7 +133,7 @@ def test_passthrough_args(rule_runner: RuleRunner) -> None: assert_success( rule_runner, tgt, - extra_args=[f"--buf-lint-args='--config={config}'"], + extra_args=[f"--buf-args='--config={config}'"], ) @@ -142,7 +142,7 @@ def test_skip(rule_runner: RuleRunner) -> None: {"foo/v1/f.proto": BAD_FILE, "foo/v1/BUILD": "protobuf_sources(name='t')"} ) tgt = rule_runner.get_target(Address("foo/v1", target_name="t", relative_file_path="f.proto")) - result = run_buf(rule_runner, [tgt], extra_args=["--buf-lint-skip"]) + result = run_buf(rule_runner, [tgt], extra_args=["--buf-skip"]) assert not result From 3883dc969ef036a020b8b9ba093fd703bb08e214 Mon Sep 17 00:00:00 2001 From: Jonas Stendahl Date: Fri, 25 Mar 2022 19:19:36 +0100 Subject: [PATCH 5/6] Better options naming and Buf 1.3.0 --- .../codegen/protobuf/lint/buf/format_rules.py | 27 +++++++------------ .../lint/buf/format_rules_integration_test.py | 8 +++--- .../codegen/protobuf/lint/buf/lint_rules.py | 8 +++--- .../lint/buf/lint_rules_integration_test.py | 4 +-- .../codegen/protobuf/lint/buf/skip_field.py | 16 ++++++++--- .../codegen/protobuf/lint/buf/subsystem.py | 16 ++++++----- src/python/pants/option/option_types.py | 7 ++--- 7 files changed, 44 insertions(+), 42 deletions(-) diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py index c419b48ca7f..1ad01e13f01 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules.py @@ -2,7 +2,7 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from dataclasses import dataclass -from pants.backend.codegen.protobuf.lint.buf.skip_field import SkipBufField +from pants.backend.codegen.protobuf.lint.buf.skip_field import SkipBufFormatField from pants.backend.codegen.protobuf.lint.buf.subsystem import BufSubsystem from pants.backend.codegen.protobuf.target_types import ( ProtobufDependenciesField, @@ -38,7 +38,7 @@ class BufFieldSet(FieldSet): @classmethod def opt_out(cls, tgt: Target) -> bool: - return tgt.get(SkipBufField).value + return tgt.get(SkipBufFormatField).value class BufFormatRequest(LintTargetsRequest, FmtRequest): @@ -95,9 +95,10 @@ async def setup_buf_format(setup_request: SetupRequest, buf: BufSubsystem) -> Se argv = [ downloaded_buf.exe, "format", - # If linting, use `-d` to error with a diff. Else, write the change with `-w`. - *(["-d"] if setup_request.check_only else ["-w"]), - *buf.args, + # If linting, use `-d` to error with a diff and `--exit-code` to exit with a non-zero exit code if + # the file is not already formatted. Else, write the change with `-w`. + *(["-d", "--exit-code"] if setup_request.check_only else ["-w"]), + *buf.format_args, "--path", ",".join(source_files_snapshot.files), ] @@ -116,7 +117,7 @@ async def setup_buf_format(setup_request: SetupRequest, buf: BufSubsystem) -> Se @rule(desc="Format with buf format", level=LogLevel.DEBUG) async def run_buf_format(request: BufFormatRequest, buf: BufSubsystem) -> FmtResult: - if buf.skip: + if buf.skip_format: return FmtResult.skip(formatter_name=request.name) setup = await Get(Setup, SetupRequest(request, check_only=False)) result = await Get(ProcessResult, Process, setup.process) @@ -132,21 +133,11 @@ async def run_buf_format(request: BufFormatRequest, buf: BufSubsystem) -> FmtRes @rule(desc="Lint with buf format", level=LogLevel.DEBUG) async def run_buf_lint(request: BufFormatRequest, buf: BufSubsystem) -> LintResults: - if buf.skip: + if buf.skip_format: return LintResults([], linter_name=request.name) setup = await Get(Setup, SetupRequest(request, check_only=True)) result = await Get(FallibleProcessResult, Process, setup.process) - - return LintResults( - [ - LintResult( - exit_code=0 if not result.stdout else 1, # buf format always exits with code 0 - stdout=result.stdout.decode(), - stderr=result.stderr.decode(), - ) - ], - linter_name=request.name, - ) + return LintResults([LintResult.from_fallible_process_result(result)], linter_name=request.name) def rules(): diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py index eb615ee5e3d..905c48e74f1 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/format_rules_integration_test.py @@ -96,7 +96,7 @@ def test_failing(rule_runner: RuleRunner) -> None: tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.proto")) lint_results, fmt_result = run_buf(rule_runner, [tgt]) assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 + assert lint_results[0].exit_code == 100 assert "f.proto.orig" in lint_results[0].stdout assert fmt_result.output == get_snapshot(rule_runner, {"f.proto": GOOD_FILE}) assert fmt_result.did_change is True @@ -112,7 +112,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: ] lint_results, fmt_result = run_buf(rule_runner, tgts) assert len(lint_results) == 1 - assert lint_results[0].exit_code == 1 + assert lint_results[0].exit_code == 100 assert "bad.proto.orig" in lint_results[0].stdout assert "good.proto" not in lint_results[0].stdout assert fmt_result.output == get_snapshot( @@ -124,7 +124,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None: def test_passthrough_args(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.proto": GOOD_FILE, "BUILD": "protobuf_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.proto")) - lint_results, fmt_result = run_buf(rule_runner, [tgt], extra_args=["--buf-args=--debug"]) + lint_results, fmt_result = run_buf(rule_runner, [tgt], extra_args=["--buf-format-args=--debug"]) assert len(lint_results) == 1 assert lint_results[0].exit_code == 0 assert lint_results[0].stdout == "" @@ -137,7 +137,7 @@ def test_passthrough_args(rule_runner: RuleRunner) -> None: def test_skip(rule_runner: RuleRunner) -> None: rule_runner.write_files({"f.proto": BAD_FILE, "BUILD": "protobuf_sources(name='t')"}) tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.proto")) - lint_results, fmt_result = run_buf(rule_runner, [tgt], extra_args=["--buf-skip"]) + lint_results, fmt_result = run_buf(rule_runner, [tgt], extra_args=["--buf-format-skip"]) assert not lint_results assert fmt_result.skipped is True assert fmt_result.did_change is False diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py b/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py index 9af84d35080..e157b5bb566 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py @@ -2,7 +2,7 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). from dataclasses import dataclass -from pants.backend.codegen.protobuf.lint.buf.skip_field import SkipBufField +from pants.backend.codegen.protobuf.lint.buf.skip_field import SkipBufLintField from pants.backend.codegen.protobuf.lint.buf.subsystem import BufSubsystem from pants.backend.codegen.protobuf.target_types import ( ProtobufDependenciesField, @@ -31,7 +31,7 @@ class BufFieldSet(FieldSet): @classmethod def opt_out(cls, tgt: Target) -> bool: - return tgt.get(SkipBufField).value + return tgt.get(SkipBufLintField).value class BufLintRequest(LintTargetsRequest): @@ -41,7 +41,7 @@ class BufLintRequest(LintTargetsRequest): @rule(desc="Lint with buf lint", level=LogLevel.DEBUG) async def run_buf(request: BufLintRequest, buf: BufSubsystem) -> LintResults: - if buf.skip: + if buf.skip_lint: return LintResults([], linter_name=request.name) transitive_targets = await Get( @@ -91,7 +91,7 @@ async def run_buf(request: BufLintRequest, buf: BufSubsystem) -> LintResults: argv=[ downloaded_buf.exe, "lint", - *buf.args, + *buf.lint_args, "--path", ",".join(target_sources_stripped.snapshot.files), ], diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules_integration_test.py b/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules_integration_test.py index 8cc5e80082f..8d39a6bfa39 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules_integration_test.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules_integration_test.py @@ -133,7 +133,7 @@ def test_passthrough_args(rule_runner: RuleRunner) -> None: assert_success( rule_runner, tgt, - extra_args=[f"--buf-args='--config={config}'"], + extra_args=[f"--buf-lint-args='--config={config}'"], ) @@ -142,7 +142,7 @@ def test_skip(rule_runner: RuleRunner) -> None: {"foo/v1/f.proto": BAD_FILE, "foo/v1/BUILD": "protobuf_sources(name='t')"} ) tgt = rule_runner.get_target(Address("foo/v1", target_name="t", relative_file_path="f.proto")) - result = run_buf(rule_runner, [tgt], extra_args=["--buf-skip"]) + result = run_buf(rule_runner, [tgt], extra_args=["--buf-lint-skip"]) assert not result diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/skip_field.py b/src/python/pants/backend/codegen/protobuf/lint/buf/skip_field.py index 8754f2b21c6..b359f792665 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/skip_field.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/skip_field.py @@ -8,14 +8,22 @@ from pants.engine.target import BoolField -class SkipBufField(BoolField): +class SkipBufFormatField(BoolField): + alias = "skip_buf_format" + default = False + help = "If true, don't run `buf format` on this target's code." + + +class SkipBufLintField(BoolField): alias = "skip_buf_lint" default = False - help = "If true, don't lint this target's code with Buf." + help = "If true, don't run `buf lint` on this target's code." def rules(): return [ - ProtobufSourceTarget.register_plugin_field(SkipBufField), - ProtobufSourcesGeneratorTarget.register_plugin_field(SkipBufField), + ProtobufSourceTarget.register_plugin_field(SkipBufFormatField), + ProtobufSourceTarget.register_plugin_field(SkipBufLintField), + ProtobufSourcesGeneratorTarget.register_plugin_field(SkipBufFormatField), + ProtobufSourcesGeneratorTarget.register_plugin_field(SkipBufLintField), ] diff --git a/src/python/pants/backend/codegen/protobuf/lint/buf/subsystem.py b/src/python/pants/backend/codegen/protobuf/lint/buf/subsystem.py index 137ebd5d4f2..7d7b03c7f3b 100644 --- a/src/python/pants/backend/codegen/protobuf/lint/buf/subsystem.py +++ b/src/python/pants/backend/codegen/protobuf/lint/buf/subsystem.py @@ -13,12 +13,12 @@ class BufSubsystem(TemplatedExternalTool): name = "Buf" help = "A linter and formatter for Protocol Buffers (https://github.com/bufbuild/buf)." - default_version = "v1.2.1" + default_version = "v1.3.0" default_known_versions = [ - "v1.2.1|linux_arm64 |8c9df682691436bd9f58efa44928e6fcd68ec6dd346e35eddac271786f4c0ae3|13940426", - "v1.2.1|linux_x86_64|eb227afeaf5f5c5a5f1d2aca92926d8c89be5b7a410e5afd6dd68f2ed0c00f22|15267079", - "v1.2.1|macos_arm64 |6877c9b8f895ec4962faff551c541d9d14e12f49b899ed7e553f0dc74a69b1b8|15388080", - "v1.2.1|macos_x86_64|652b407fd08e5e664244971f4a725763ef582f26778674490658ad2ce361fe95|15954329", + "v1.3.0|linux_arm64 |fbfd53c501451b36900247734bfa4cbe86ae05d0f51bc298de8711d5ee374ee5|13940828", + "v1.3.0|linux_x86_64|e29c4283b1cd68ada41fa493171c41d7605750d258fcd6ecdf692a63fae95213|15267162", + "v1.3.0|macos_arm64 |147985d7f2816a545792e38b26178ff4027bf16cd3712f6e387a4e3692a16deb|15391890", + "v1.3.0|macos_x86_64|3b6bd2e5a5dd758178aee01fb067261baf5d31bfebe93336915bfdf7b21928c4|15955291", ] default_url_template = ( "https://github.com/bufbuild/buf/releases/download/{version}/buf-{platform}.tar.gz" @@ -30,8 +30,10 @@ class BufSubsystem(TemplatedExternalTool): "linux_x86_64": "Linux-x86_64", } - skip = SkipOption("fmt", "lint") - args = ArgsListOption(example="--error-format json") + skip_format = SkipOption("fmt", "lint", flag_name="--format-skip") + skip_lint = SkipOption("lint", flag_name="--lint-skip") + format_args = ArgsListOption(example="--error-format json", flag_name="--format-args") + lint_args = ArgsListOption(example="--error-format json", flag_name="--lint-args") def generate_exe(self, plat: Platform) -> str: return "./buf/bin/buf" diff --git a/src/python/pants/option/option_types.py b/src/python/pants/option/option_types.py index ba6d6709100..eab333844d3 100644 --- a/src/python/pants/option/option_types.py +++ b/src/python/pants/option/option_types.py @@ -761,12 +761,12 @@ def _convert_(self, val: Any) -> dict[str, _ValueT]: class SkipOption(BoolOption[bool]): """A --skip option (for an invocable tool).""" - def __new__(cls, goal: str, *other_goals: str): + def __new__(cls, goal: str, *other_goals: str, flag_name: str | None = None): goals = (goal,) + other_goals invocation_str = " and ".join([f"`{bin_name()} {goal}`" for goal in goals]) return super().__new__( cls, # type: ignore[arg-type] - "--skip", + flag_name or "--skip", default=False, # type: ignore[arg-type] help=( lambda subsystem_cls: ( @@ -788,12 +788,13 @@ def __new__( # This should be set when callers can alternatively use "--" followed by the arguments, # instead of having to provide "--[scope]-args='--arg1 --arg2'". passthrough: bool | None = None, + flag_name: str | None = None, ): if extra_help: extra_help = "\n\n" + extra_help instance = super().__new__( cls, # type: ignore[arg-type] - "--args", + flag_name or "--args", help=( lambda subsystem_cls: ( f"Arguments to pass directly to {tool_name or subsystem_cls.name}, " From cdc25a20fe9b15a1ce550d9968553ea42ad2c280 Mon Sep 17 00:00:00 2001 From: Jonas Stendahl Date: Fri, 25 Mar 2022 19:34:21 +0100 Subject: [PATCH 6/6] Cleanup code --- src/python/pants/option/option_types.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/python/pants/option/option_types.py b/src/python/pants/option/option_types.py index eab333844d3..31f025d12c8 100644 --- a/src/python/pants/option/option_types.py +++ b/src/python/pants/option/option_types.py @@ -761,12 +761,12 @@ def _convert_(self, val: Any) -> dict[str, _ValueT]: class SkipOption(BoolOption[bool]): """A --skip option (for an invocable tool).""" - def __new__(cls, goal: str, *other_goals: str, flag_name: str | None = None): + def __new__(cls, goal: str, *other_goals: str, flag_name: str = "--skip"): goals = (goal,) + other_goals invocation_str = " and ".join([f"`{bin_name()} {goal}`" for goal in goals]) return super().__new__( cls, # type: ignore[arg-type] - flag_name or "--skip", + flag_name, default=False, # type: ignore[arg-type] help=( lambda subsystem_cls: ( @@ -788,13 +788,13 @@ def __new__( # This should be set when callers can alternatively use "--" followed by the arguments, # instead of having to provide "--[scope]-args='--arg1 --arg2'". passthrough: bool | None = None, - flag_name: str | None = None, + flag_name: str = "--args", ): if extra_help: extra_help = "\n\n" + extra_help instance = super().__new__( cls, # type: ignore[arg-type] - flag_name or "--args", + flag_name, help=( lambda subsystem_cls: ( f"Arguments to pass directly to {tool_name or subsystem_cls.name}, "