Skip to content

Commit

Permalink
Check parent directories too for config files for tools Pants runs (p…
Browse files Browse the repository at this point in the history
…antsbuild#11992)

This audits each tool to make sure we are properly capturing all their config files. For now, we only warn if we detect them and they're not configured, but soon we will autodiscover them.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano authored Apr 30, 2021
1 parent 9021f79 commit 04819a1
Show file tree
Hide file tree
Showing 19 changed files with 177 additions and 125 deletions.
17 changes: 14 additions & 3 deletions src/python/pants/backend/python/goals/coverage_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,10 @@ def register_options(cls, register):
type=file_option,
default=None,
advanced=True,
help="Path to `.coveragerc` or alternative coverage config file",
help=(
"Path to `.coveragerc` config file. Pants does not yet support alternative formats "
"like `setup.cfg` and `pyproject.toml`."
),
)

@property
Expand All @@ -161,10 +164,17 @@ def config(self) -> Optional[str]:

@property
def config_request(self) -> ConfigFilesRequest:
# Refer to https://coverage.readthedocs.io/en/stable/config.html.
return ConfigFilesRequest(
specified=self.config,
check_existence=[".coveragerc"],
check_content={"setup.cfg": b"[coverage:"},
# TODO: Support alternative config formats. Perhaps we start validating the user has
# set the correct config and we stop trying to manually set it.
# check_content={
# "setup.cfg": b"[coverage:",
# "tox.ini": "[coverage:]",
# "pyproject.toml": "[tool.coverage]"
# },
option_name=f"[{self.options_scope}].config",
)

Expand Down Expand Up @@ -193,7 +203,8 @@ def _validate_and_update_config(
relative_files_str = run_section.get("relative_files", "True")
if relative_files_str.lower() != "true":
raise ValueError(
f"relative_files under the 'run' section must be set to True. config file: {config_path}"
"relative_files under the 'run' section must be set to True in the config "
f"file {config_path}"
)
coverage_config.set("run", "relative_files", "True")
omit_elements = [em for em in run_section.get("omit", "").split("\n")] or ["\n"]
Expand Down
12 changes: 7 additions & 5 deletions src/python/pants/backend/python/goals/pytest_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,6 @@ async def setup_pytest_for_target(
),
)

config_files_get = Get(ConfigFiles, ConfigFilesRequest, pytest.config_request)

extra_output_directory_digest_get = Get(Digest, CreateDigest([Directory(_EXTRA_OUTPUT_DIR)]))

prepared_sources_get = Get(
Expand Down Expand Up @@ -184,18 +182,16 @@ async def setup_pytest_for_target(
requirements_pex,
prepared_sources,
field_set_source_files,
config_files,
extra_output_directory_digest,
) = await MultiGet(
pytest_pex_get,
requirements_pex_get,
prepared_sources_get,
field_set_source_files_get,
config_files_get,
extra_output_directory_digest_get,
)

pytest_runner_pex = await Get(
pytest_runner_pex_get = Get(
VenvPex,
PexRequest(
output_filename="pytest_runner.pex",
Expand All @@ -205,6 +201,12 @@ async def setup_pytest_for_target(
pex_path=[pytest_pex, requirements_pex],
),
)
config_files_get = Get(
ConfigFiles,
ConfigFilesRequest,
pytest.config_request(field_set_source_files.snapshot.dirs),
)
pytest_runner_pex, config_files = await MultiGet(pytest_runner_pex_get, config_files_get)

input_digest = await Get(
Digest,
Expand Down
8 changes: 3 additions & 5 deletions src/python/pants/backend/python/lint/bandit/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ class BanditPartition:
interpreter_constraints: PexInterpreterConstraints


def generate_args(
*, source_files: SourceFiles, bandit: Bandit, report_file_name: Optional[str]
def generate_argv(
source_files: SourceFiles, bandit: Bandit, *, report_file_name: Optional[str]
) -> Tuple[str, ...]:
args = []
if bandit.config is not None:
Expand Down Expand Up @@ -92,9 +92,7 @@ async def bandit_lint_partition(
FallibleProcessResult,
VenvPexProcess(
bandit_pex,
argv=generate_args(
source_files=source_files, bandit=bandit, report_file_name=report_file_name
),
argv=generate_argv(source_files, bandit, report_file_name=report_file_name),
input_digest=input_digest,
description=f"Run Bandit on {pluralize(len(partition.field_sets), 'file')}.",
output_files=(report_file_name,) if report_file_name else None,
Expand Down
17 changes: 10 additions & 7 deletions src/python/pants/backend/python/lint/bandit/subsystem.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from typing import Optional, Tuple, cast
from __future__ import annotations

from typing import cast

from pants.backend.python.subsystems.python_tool_base import PythonToolBase
from pants.backend.python.target_types import ConsoleScript
Expand All @@ -11,7 +13,7 @@

class Bandit(PythonToolBase):
options_scope = "bandit"
help = """A tool for finding security issues in Python code (https://bandit.readthedocs.io)."""
help = "A tool for finding security issues in Python code (https://bandit.readthedocs.io)."

default_version = "bandit>=1.6.2,<1.7"
# `setuptools<45` is for Python 2 support. `stevedore` is because the 3.0 release breaks Bandit.
Expand Down Expand Up @@ -41,22 +43,23 @@ def register_options(cls, register):
type=file_option,
default=None,
advanced=True,
help="Path to a Bandit YAML config file",
help="Path to a Bandit YAML config file.",
)

@property
def skip(self) -> bool:
return cast(bool, self.options.skip)

@property
def args(self) -> Tuple[str, ...]:
def args(self) -> tuple[str, ...]:
return tuple(self.options.args)

@property
def config(self) -> Optional[str]:
return cast(Optional[str], self.options.config)
def config(self) -> str | None:
return cast("str | None", self.options.config)

@property
def config_request(self) -> ConfigFilesRequest:
# Note that there are no default locations for Bandit config files.
# Refer to https://bandit.readthedocs.io/en/latest/config.html. Note that there are no
# default locations for Bandit config files.
return ConfigFilesRequest(specified=self.config, option_name=f"{self.options_scope}.config")
14 changes: 6 additions & 8 deletions src/python/pants/backend/python/lint/black/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class Setup:
original_digest: Digest


def generate_args(*, source_files: SourceFiles, black: Black, check_only: bool) -> Tuple[str, ...]:
def generate_argv(source_files: SourceFiles, black: Black, *, check_only: bool) -> Tuple[str, ...]:
args = []
if check_only:
args.append("--check")
Expand Down Expand Up @@ -97,21 +97,21 @@ async def setup_black(
),
)

config_files_get = Get(ConfigFiles, ConfigFilesRequest, black.config_request)
source_files_get = Get(
SourceFiles,
SourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets),
)

source_files, black_pex, config_files = await MultiGet(
source_files_get, black_pex_get, config_files_get
)
source_files, black_pex = await MultiGet(source_files_get, black_pex_get)
source_files_snapshot = (
source_files.snapshot
if setup_request.request.prior_formatter_result is None
else setup_request.request.prior_formatter_result
)

config_files = await Get(
ConfigFiles, ConfigFilesRequest, black.config_request(source_files_snapshot.dirs)
)
input_digest = await Get(
Digest, MergeDigests((source_files_snapshot.digest, config_files.snapshot.digest))
)
Expand All @@ -120,9 +120,7 @@ async def setup_black(
Process,
VenvPexProcess(
black_pex,
argv=generate_args(
source_files=source_files, black=black, check_only=setup_request.check_only
),
argv=generate_argv(source_files, black, check_only=setup_request.check_only),
input_digest=input_digest,
output_files=source_files_snapshot.files,
description=f"Run Black on {pluralize(len(setup_request.request.field_sets), 'file')}.",
Expand Down
21 changes: 13 additions & 8 deletions src/python/pants/backend/python/lint/black/subsystem.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from typing import Optional, Tuple, cast
from __future__ import annotations

import os.path
from typing import Iterable, cast

from pants.backend.python.subsystems.python_tool_base import PythonToolBase
from pants.backend.python.target_types import ConsoleScript
Expand Down Expand Up @@ -46,25 +49,27 @@ def register_options(cls, register):
type=file_option,
default=None,
advanced=True,
help="Path to Black's pyproject.toml config file",
help="Path to a TOML config file understood by Black.",
)

@property
def skip(self) -> bool:
return cast(bool, self.options.skip)

@property
def args(self) -> Tuple[str, ...]:
def args(self) -> tuple[str, ...]:
return tuple(self.options.args)

@property
def config(self) -> Optional[str]:
return cast(Optional[str], self.options.config)
def config(self) -> str | None:
return cast("str | None", self.options.config)

@property
def config_request(self) -> ConfigFilesRequest:
def config_request(self, dirs: Iterable[str]) -> ConfigFilesRequest:
# Refer to https://github.com/psf/black#where-black-looks-for-the-file for how Black
# discovers config.
candidates = {os.path.join(d, "pyproject.toml"): b"[tool.black]" for d in ("", *dirs)}
return ConfigFilesRequest(
specified=self.config,
check_content={"pyproject.toml": b"[tool.black]"},
check_content=candidates,
option_name=f"[{self.options_scope}].config",
)
8 changes: 3 additions & 5 deletions src/python/pants/backend/python/lint/flake8/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ class Flake8Partition:
interpreter_constraints: PexInterpreterConstraints


def generate_args(
*, source_files: SourceFiles, flake8: Flake8, report_file_name: Optional[str]
def generate_argv(
source_files: SourceFiles, flake8: Flake8, *, report_file_name: Optional[str]
) -> Tuple[str, ...]:
args = []
if flake8.config:
Expand Down Expand Up @@ -90,9 +90,7 @@ async def flake8_lint_partition(
FallibleProcessResult,
VenvPexProcess(
flake8_pex,
argv=generate_args(
source_files=source_files, flake8=flake8, report_file_name=report_file_name
),
argv=generate_argv(source_files, flake8, report_file_name=report_file_name),
input_digest=input_digest,
output_files=(report_file_name,) if report_file_name else None,
description=f"Run Flake8 on {pluralize(len(partition.field_sets), 'file')}.",
Expand Down
14 changes: 9 additions & 5 deletions src/python/pants/backend/python/lint/flake8/subsystem.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from typing import Optional, Tuple, cast
from __future__ import annotations

from typing import cast

from pants.backend.python.subsystems.python_tool_base import PythonToolBase
from pants.backend.python.target_types import ConsoleScript
Expand Down Expand Up @@ -43,23 +45,25 @@ def register_options(cls, register):
type=file_option,
default=None,
advanced=True,
help="Path to `.flake8` or alternative Flake8 config file",
help="Path to `.flake8` or alternative Flake8 config file.",
)

@property
def skip(self) -> bool:
return cast(bool, self.options.skip)

@property
def args(self) -> Tuple[str, ...]:
def args(self) -> tuple[str, ...]:
return tuple(self.options.args)

@property
def config(self) -> Optional[str]:
return cast(Optional[str], self.options.config)
def config(self) -> str | None:
return cast("str | None", self.options.config)

@property
def config_request(self) -> ConfigFilesRequest:
# See https://flake8.pycqa.org/en/latest/user/configuration.html#configuration-locations
# for how Flake8 discovers config files.
return ConfigFilesRequest(
specified=self.config,
check_existence=["flake8", ".flake8"],
Expand Down
19 changes: 8 additions & 11 deletions src/python/pants/backend/python/lint/isort/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,12 @@ class Setup:
original_digest: Digest


def generate_args(*, source_files: SourceFiles, isort: Isort, check_only: bool) -> Tuple[str, ...]:
def generate_argv(source_files: SourceFiles, isort: Isort, *, check_only: bool) -> Tuple[str, ...]:
# NB: isort auto-discovers config files. There is no way to hardcode them via command line
# flags. So long as the files are in the Pex's input files, isort will use the config.
args = []
args = [*isort.args]
if check_only:
args.append("--check-only")
args.extend(isort.args)
args.extend(source_files.files)
return tuple(args)

Expand All @@ -74,22 +73,22 @@ async def setup_isort(setup_request: SetupRequest, isort: Isort) -> Setup:
main=isort.main,
),
)

config_files_get = Get(ConfigFiles, ConfigFilesRequest, isort.config_request)
source_files_get = Get(
SourceFiles,
SourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets),
)
source_files, isort_pex = await MultiGet(source_files_get, isort_pex_get)

source_files, isort_pex, config_files = await MultiGet(
source_files_get, isort_pex_get, config_files_get
)
source_files_snapshot = (
source_files.snapshot
if setup_request.request.prior_formatter_result is None
else setup_request.request.prior_formatter_result
)

config_files = await Get(
ConfigFiles, ConfigFilesRequest, isort.config_request(source_files_snapshot.dirs)
)

input_digest = await Get(
Digest, MergeDigests((source_files_snapshot.digest, config_files.snapshot.digest))
)
Expand All @@ -98,9 +97,7 @@ async def setup_isort(setup_request: SetupRequest, isort: Isort) -> Setup:
Process,
VenvPexProcess(
isort_pex,
argv=generate_args(
source_files=source_files, isort=isort, check_only=setup_request.check_only
),
argv=generate_argv(source_files, isort, check_only=setup_request.check_only),
input_digest=input_digest,
output_files=source_files_snapshot.files,
description=f"Run isort on {pluralize(len(setup_request.request.field_sets), 'file')}.",
Expand Down
Loading

0 comments on commit 04819a1

Please sign in to comment.