Skip to content

Commit

Permalink
Add [buf] config and config_discovery options (pantsbuild#17885)
Browse files Browse the repository at this point in the history
Enhancement for issue [17871](pantsbuild#17871).

Add `config` and `config_discovery` options for Buf backend.

Documentation pieces to be added to [buf options](https://www.pantsbuild.org/docs/reference-buf):
* configuration file is for `lint`, `build` and `breaking` Buf commands. Right now, only `lint` is enabled in Pants backend. There's no configuration file need for `format` command.
* the broader option `lint_args` may also contain a `--config something` entry. As placed in this order (config args then lint_args config), the second overrides the first
* Buf recommends to have the config within the folder where the protos are located. Discovery is impossible there, as the proto folders would not be known by pants, we should recommend to either have the config file where pants.toml is located, or specify the file path using `[buf].config`

Tests:
* [X] added integration tests for lint run (1 for config discovery, 1 for specific config file)
* [X] local integration test with [running-pants-from-sources-in-other-repos](https://www.pantsbuild.org/v2.15/docs/running-pants-from-sources#running-pants-from-sources-in-other-repos)
  • Loading branch information
anteverse authored Dec 30, 2022
1 parent 8a93b5a commit 21e1180
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 3 deletions.
18 changes: 16 additions & 2 deletions src/python/pants/backend/codegen/protobuf/lint/buf/lint_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
ProtobufSourceField,
)
from pants.core.goals.lint import LintResult, LintTargetsRequest, Partitions
from pants.core.util_rules.config_files import ConfigFiles, ConfigFilesRequest
from pants.core.util_rules.external_tool import DownloadedExternalTool, ExternalToolRequest
from pants.core.util_rules.source_files import SourceFilesRequest
from pants.core.util_rules.stripped_source_files import StrippedSourceFiles
Expand Down Expand Up @@ -79,8 +80,17 @@ async def run_buf(

download_buf_get = Get(DownloadedExternalTool, ExternalToolRequest, buf.get_request(platform))

target_sources_stripped, all_sources_stripped, downloaded_buf = await MultiGet(
target_stripped_sources_request, all_stripped_sources_request, download_buf_get
config_files_get = Get(
ConfigFiles,
ConfigFilesRequest,
buf.config_request,
)

target_sources_stripped, all_sources_stripped, downloaded_buf, config_files = await MultiGet(
target_stripped_sources_request,
all_stripped_sources_request,
download_buf_get,
config_files_get,
)

input_digest = await Get(
Expand All @@ -90,16 +100,20 @@ async def run_buf(
target_sources_stripped.snapshot.digest,
all_sources_stripped.snapshot.digest,
downloaded_buf.digest,
config_files.snapshot.digest,
)
),
)

config_arg = ["--config", buf.config] if buf.config else []

process_result = await Get(
FallibleProcessResult,
Process(
argv=[
downloaded_buf.exe,
"lint",
*config_arg,
*buf.lint_args,
"--path",
",".join(target_sources_stripped.snapshot.files),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,55 @@ def test_dependencies(rule_runner: RuleRunner) -> None:
assert_success(
rule_runner, tgt, source_roots=["src/python", "/src/protobuf", "/tests/protobuf"]
)


def test_config_discovery(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
"foo/v1/f.proto": BAD_FILE,
"foo/v1/BUILD": "protobuf_sources(name='t')",
"buf.yaml": dedent(
"""\
version: v1
lint:
ignore_only:
FIELD_LOWER_SNAKE_CASE:
- foo/v1/f.proto
"""
),
}
)

tgt = rule_runner.get_target(Address("foo/v1", target_name="t", relative_file_path="f.proto"))

assert_success(
rule_runner,
tgt,
)


def test_config_file_submitted(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
"foo/v1/f.proto": BAD_FILE,
"foo/v1/BUILD": "protobuf_sources(name='t')",
# Intentionally placed somewhere config_discovery can't see.
"foo/buf.yaml": dedent(
"""\
version: v1
lint:
ignore_only:
FIELD_LOWER_SNAKE_CASE:
- foo/v1/f.proto
""",
),
}
)

tgt = rule_runner.get_target(Address("foo/v1", target_name="t", relative_file_path="f.proto"))

assert_success(
rule_runner,
tgt,
extra_args=["--buf-config=foo/buf.yaml"],
)
41 changes: 40 additions & 1 deletion src/python/pants/backend/codegen/protobuf/lint/buf/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@

from __future__ import annotations

from pants.core.util_rules.config_files import ConfigFilesRequest
from pants.core.util_rules.external_tool import TemplatedExternalTool
from pants.engine.platform import Platform
from pants.option.option_types import ArgsListOption, SkipOption
from pants.option.option_types import ArgsListOption, BoolOption, FileOption, SkipOption
from pants.util.strutil import softwrap


class BufSubsystem(TemplatedExternalTool):
Expand Down Expand Up @@ -35,5 +37,42 @@ class BufSubsystem(TemplatedExternalTool):
format_args = ArgsListOption(example="--error-format json")
lint_args = ArgsListOption(example="--error-format json")

config = FileOption(
default=None,
advanced=True,
help=lambda cls: softwrap(
f"""
Path to a config file understood by Buf
(https://docs.buf.build/configuration/overview).
Setting this option will disable `[{cls.options_scope}].config_discovery`. Use
this option if the config is located in a non-standard location.
"""
),
)
config_discovery = BoolOption(
default=True,
advanced=True,
help=lambda cls: softwrap(
f"""
If true, Pants will include any relevant root config files during runs
(`buf.yaml`, `buf.lock`, `buf.gen.yaml` and `buf.work.yaml`).
Use `[{cls.options_scope}].config` instead if your config is in a non-standard location.
"""
),
)

@property
def config_request(self) -> ConfigFilesRequest:
# Refer to https://docs.buf.build/configuration/overview.
return ConfigFilesRequest(
specified=self.config,
specified_option_name=f"{self.options_scope}.config",
discovery=self.config_discovery,
check_existence=["buf.yaml", "buf.lock", "buf.gen.yaml", "buf.work.yaml"],
check_content={},
)

def generate_exe(self, plat: Platform) -> str:
return "./buf/bin/buf"

0 comments on commit 21e1180

Please sign in to comment.