Skip to content

Commit

Permalink
Discover config files automatically for tools Pants runs (pantsbuild#…
Browse files Browse the repository at this point in the history
…11995)

If a tool supports autodiscovery/autoloading of config files, Pants will now autodiscover those config files and include them in the chroot. This makes it more likely for Pants to work out-of-the-box.

### FYI: Semantics of `[tool].config` vs. `[tool].config_discovery`

Some tools have both these options, and some only one but not the other.

`[tool].config` is intended for custom config file locations. Pants will include the file in the chroot _and_ instruct the tool to load it via its option like `--config`/`--rcfile`. If the tool does not have an option like `--config` (Shellcheck, shfmt, and Pytest), then Pants should not have a `[tool].config` option.

`[tool].config_discovery` will locate the relevant files and include them in the chroot, but _will not_ change the argv for the tool, i.e. will not use its `--config/--rcfile` option. This avoids a major problem: what to do if we detect >1 config file but the tool only supports 1 config file. Each tool handles that case differently, normally using "first one wins". Pants should not attempt to disambiguate there, and it's a bad UX for us to error. Instead, we simply populate the chroot with the files and let the tool do its thing. This means that we only offer `[tool].config_discovery` if the tool knows how to automatically load those config files without changing the argv.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano authored May 1, 2021
1 parent f5f59bb commit 3acdae9
Show file tree
Hide file tree
Showing 22 changed files with 300 additions and 180 deletions.
5 changes: 0 additions & 5 deletions pants.toml
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ root_patterns = [
requirement_constraints = "3rdparty/python/constraints.txt"
interpreter_constraints = [">=3.7,<3.10"]

[black]
config = "pyproject.toml"

[docformatter]
args = ["--wrap-summaries=100", "--wrap-descriptions=100"]

Expand All @@ -89,7 +86,6 @@ extra_requirements.add = [

[isort]
version = "isort[pyproject,colors]>=5.5.1,<5.6"
config = ["pyproject.toml"]

[mypy]
config = "build-support/mypy/mypy.ini"
Expand Down Expand Up @@ -170,7 +166,6 @@ extra_env_vars = [

[coverage-py]
interpreter_constraints = [">=3.7,<3.10"]
config = "pyproject.toml"

[sourcefile-validation]
config = "@build-support/regexes/config.yaml"
Expand Down
28 changes: 23 additions & 5 deletions src/python/pants/backend/python/goals/coverage_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,26 @@ def register_options(cls, register):
default=None,
advanced=True,
help=(
"Path to an INI or TOML config file understood by coverage.py.\n\nNote: to use "
"a TOML config file, you should add the `toml` library to "
"Path to an INI or TOML config file understood by coverage.py "
"(https://coverage.readthedocs.io/en/stable/config.html).\n\n"
f"Setting this option will disable `[{cls.options_scope}].config_discovery`. Use "
f"this option if the config is located in a non-standard location.\n\n"
"To use a TOML config file, you should add the `toml` library to "
"`[pytest].pytest_plugins`."
),
)
register(
"--config-discovery",
type=bool,
default=True,
advanced=True,
help=(
"If true, Pants will include any relevant config files during runs "
"(`.coveragerc`, `setup.cfg`, `tox.ini`, and `pyproject.toml`)."
f"\n\nUse `[{cls.options_scope}].config` instead if your config is in a "
f"non-standard location."
),
)

@property
def filter(self) -> Tuple[str, ...]:
Expand All @@ -170,13 +185,14 @@ def config_request(self) -> ConfigFilesRequest:
# Refer to https://coverage.readthedocs.io/en/stable/config.html.
return ConfigFilesRequest(
specified=self.config,
specified_option_name=f"[{self.options_scope}].config",
discovery=cast(bool, self.options.config_discovery),
check_existence=[".coveragerc"],
check_content={
"setup.cfg": b"[coverage:",
"tox.ini": b"[coverage:]",
"pyproject.toml": b"[tool.coverage",
},
option_name=f"[{self.options_scope}].config",
)


Expand Down Expand Up @@ -226,7 +242,8 @@ def _update_config(fc: FileContent) -> FileContent:
except toml.TomlDecodeError as exc:
raise InvalidCoverageConfigError(
f"Failed to parse the coverage.py config `{fc.path}` as TOML. Please either fix "
f"the config or update `[coverage-py].config`.\n\nParse error: {repr(exc)}"
f"the config or update `[coverage-py].config` and/or "
f"`[coverage-py].config_discovery`.\n\nParse error: {repr(exc)}"
)
tool = all_config.setdefault("tool", {})
coverage = tool.setdefault("coverage", {})
Expand All @@ -242,7 +259,8 @@ def _update_config(fc: FileContent) -> FileContent:
except configparser.Error as exc:
raise InvalidCoverageConfigError(
f"Failed to parse the coverage.py config `{fc.path}` as INI. Please either fix "
f"the config or update `[coverage-py].config`.\n\nParse error: {repr(exc)}"
f"the config or update `[coverage-py].config` and/or `[coverage-py].config_discovery`."
f"\n\nParse error: {repr(exc)}"
)
run_section = "coverage:run" if fc.path in ("tox.ini", "setup.cfg") else "run"
if not cp.has_section(run_section):
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/python/goals/coverage_py_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@


def resolve_config(path: str | None, content: str | None) -> str:
coverage_subsystem = create_subsystem(CoverageSubsystem, config=path)
coverage_subsystem = create_subsystem(CoverageSubsystem, config=path, config_discovery=True)
resolved_config: list[str] = []

def mock_find_existing_config(request: ConfigFilesRequest) -> ConfigFiles:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ def test() -> None:
assert f"{PACKAGE}/tests.py ." in result.stdout


def test_respects_passthrough_args(rule_runner: RuleRunner) -> None:
def test_passthrough_args(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
f"{PACKAGE}/tests.py": dedent(
Expand All @@ -234,7 +234,7 @@ def test_ignore_me():
assert "collected 2 items / 1 deselected / 1 selected" in result.stdout


def test_respects_config(rule_runner: RuleRunner) -> None:
def test_config_file(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
"pytest.ini": dedent(
Expand All @@ -253,7 +253,7 @@ def test():
}
)
tgt = rule_runner.get_target(Address(PACKAGE, relative_file_path="tests.py"))
result = run_pytest(rule_runner, tgt, extra_args=["--pytest-config=pytest.ini"])
result = run_pytest(rule_runner, tgt)
assert result.exit_code == 0
assert "All good!" in result.stdout and "Captured" not in result.stdout

Expand Down
9 changes: 7 additions & 2 deletions src/python/pants/backend/python/lint/bandit/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ 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 "
"(https://bandit.readthedocs.io/en/latest/config.html)."
),
)

@property
Expand All @@ -62,4 +65,6 @@ def config(self) -> str | None:
def config_request(self) -> ConfigFilesRequest:
# 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")
return ConfigFilesRequest(
specified=self.config, specified_option_name=f"{self.options_scope}.config"
)
Original file line number Diff line number Diff line change
Expand Up @@ -117,18 +117,20 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None:
assert fmt_result.did_change is True


def test_respects_config_file(rule_runner: RuleRunner) -> None:
@pytest.mark.parametrize(
"config_path,extra_args",
(["pyproject.toml", []], ["custom_config.toml", ["--black-config=custom_config.toml"]]),
)
def test_config_file(rule_runner: RuleRunner, config_path: str, extra_args: list[str]) -> None:
rule_runner.write_files(
{
"f.py": NEEDS_CONFIG_FILE,
"pyproject.toml": "[tool.black]\nskip-string-normalization = 'true'\n",
"BUILD": "python_library(name='t')",
config_path: "[tool.black]\nskip-string-normalization = 'true'\n",
}
)
tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
lint_results, fmt_result = run_black(
rule_runner, [tgt], extra_args=["--black-config=pyproject.toml"]
)
lint_results, fmt_result = run_black(rule_runner, [tgt], extra_args=extra_args)
assert len(lint_results) == 1
assert lint_results[0].exit_code == 0
assert "1 file would be left unchanged" in lint_results[0].stderr
Expand All @@ -137,7 +139,7 @@ def test_respects_config_file(rule_runner: RuleRunner) -> None:
assert fmt_result.did_change is False


def test_respects_passthrough_args(rule_runner: RuleRunner) -> None:
def test_passthrough_args(rule_runner: RuleRunner) -> None:
rule_runner.write_files({"f.py": NEEDS_CONFIG_FILE, "BUILD": "python_library(name='t')"})
tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
lint_results, fmt_result = run_black(
Expand Down
21 changes: 19 additions & 2 deletions src/python/pants/backend/python/lint/black/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,23 @@ def register_options(cls, register):
type=file_option,
default=None,
advanced=True,
help="Path to a TOML config file understood by Black.",
help=(
"Path to a TOML config file understood by Black "
"(https://github.com/psf/black#configuration-format).\n\n"
f"Setting this option will disable `[{cls.options_scope}].config_discovery`. Use "
f"this option if the config is located in a non-standard location."
),
)
register(
"--config-discovery",
type=bool,
default=True,
advanced=True,
help=(
"If true, Pants will include any relevant pyproject.toml config files during runs."
f"\n\nUse `[{cls.options_scope}].config` instead if your config is in a "
f"non-standard location."
),
)

@property
Expand All @@ -70,6 +86,7 @@ def config_request(self, dirs: Iterable[str]) -> ConfigFilesRequest:
candidates = {os.path.join(d, "pyproject.toml"): b"[tool.black]" for d in ("", *dirs)}
return ConfigFilesRequest(
specified=self.config,
specified_option_name=f"[{self.options_scope}].config",
discovery=cast(bool, self.options.config_discovery),
check_content=candidates,
option_name=f"[{self.options_scope}].config",
)
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,23 @@ def test_uses_correct_python_version(rule_runner: RuleRunner) -> None:
assert batched_py3_result.stdout.strip() == ""


def test_respects_config_file(rule_runner: RuleRunner) -> None:
@pytest.mark.parametrize(
"config_path,extra_args",
([".flake8", []], ["custom_config.ini", ["--flake8-config=custom_config.ini"]]),
)
def test_config_file(rule_runner: RuleRunner, config_path: str, extra_args: list[str]) -> None:
rule_runner.write_files(
{
"f.py": BAD_FILE,
"BUILD": "python_library(name='t')",
".flake8": "[flake8]\nignore = F401\n",
config_path: "[flake8]\nignore = F401\n",
}
)
tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
assert_success(rule_runner, tgt, extra_args=["--flake8-config=.flake8"])
assert_success(rule_runner, tgt, extra_args=extra_args)


def test_respects_passthrough_args(rule_runner: RuleRunner) -> None:
def test_passthrough_args(rule_runner: RuleRunner) -> None:
rule_runner.write_files({"f.py": BAD_FILE, "BUILD": "python_library(name='t')"})
tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
assert_success(rule_runner, tgt, extra_args=["--flake8-args='--ignore=F401'"])
Expand Down
22 changes: 20 additions & 2 deletions src/python/pants/backend/python/lint/flake8/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,24 @@ def register_options(cls, register):
type=file_option,
default=None,
advanced=True,
help="Path to `.flake8` or alternative Flake8 config file.",
help=(
"Path to an INI config file understood by Flake8 "
"(https://flake8.pycqa.org/en/latest/user/configuration.html).\n\n"
f"Setting this option will disable `[{cls.options_scope}].config_discovery`. Use "
f"this option if the config is located in a non-standard location."
),
)
register(
"--config-discovery",
type=bool,
default=True,
advanced=True,
help=(
"If true, Pants will include any relevant config files during "
"runs (`.flake8`, `flake8`, `setup.cfg`, and `tox.ini`)."
f"\n\nUse `[{cls.options_scope}].config` instead if your config is in a "
f"non-standard location."
),
)

@property
Expand All @@ -66,7 +83,8 @@ def config_request(self) -> ConfigFilesRequest:
# for how Flake8 discovers config files.
return ConfigFilesRequest(
specified=self.config,
specified_option_name=f"[{self.options_scope}].config",
discovery=cast(bool, self.options.config_discovery),
check_existence=["flake8", ".flake8"],
check_content={"setup.cfg": b"[flake8]", "tox.ini": b"[flake8]"},
option_name=f"[{self.options_scope}].config",
)
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None:


@pytest.mark.parametrize(
"path,extra_args",
((".isort.cfg", ["--isort-config=.isort.cfg"]), ("custom.ini", ["--isort-config=custom.ini"])),
"path,extra_args", ((".isort.cfg", []), ("custom.ini", ["--isort-config=custom.ini"]))
)
def test_config_file(rule_runner: RuleRunner, path: str, extra_args: list[str]) -> None:
rule_runner.write_files(
Expand Down
20 changes: 18 additions & 2 deletions src/python/pants/backend/python/lint/isort/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,26 @@ def register_options(cls, register):
member_type=file_option,
advanced=True,
help=(
"Path to `isort.cfg` or alternative isort config file(s).\n\n"
"Path to config file understood by isort "
"(https://pycqa.github.io/isort/docs/configuration/config_files/).\n\n"
f"Setting this option will disable `[{cls.options_scope}].config_discovery`. Use "
f"this option if the config is located in a non-standard location.\n\n"
"If using isort 5+ and you specify only 1 config file, Pants will configure "
"isort's argv to point to your config file."
),
)
register(
"--config-discovery",
type=bool,
default=True,
advanced=True,
help=(
"If true, Pants will include any relevant config files during "
"runs (`.isort.cfg`, `pyproject.toml`, `setup.cfg`, `tox.ini` and `.editorconfig`)."
f"\n\nUse `[{cls.options_scope}].config` instead if your config is in a "
f"non-standard location."
),
)

@property
def skip(self) -> bool:
Expand Down Expand Up @@ -93,7 +108,8 @@ def config_request(self, dirs: Iterable[str]) -> ConfigFilesRequest:

return ConfigFilesRequest(
specified=self.config,
specified_option_name=f"[{self.options_scope}].config",
discovery=cast(bool, self.options.config_discovery),
check_existence=check_existence,
check_content=check_content,
option_name=f"[{self.options_scope}].config",
)
Original file line number Diff line number Diff line change
Expand Up @@ -149,19 +149,23 @@ def test_uses_correct_python_version(rule_runner: RuleRunner) -> None:
assert "Your code has been rated at 10.00/10" in batched_py3_result.stdout.strip()


def test_respects_config_file(rule_runner: RuleRunner) -> None:
@pytest.mark.parametrize(
"config_path,extra_args",
(["pylintrc", []], ["custom_config.ini", ["--pylint-config=custom_config.ini"]]),
)
def test_config_file(rule_runner: RuleRunner, config_path: str, extra_args: list[str]) -> None:
rule_runner.write_files(
{
f"{PACKAGE}/f.py": BAD_FILE,
f"{PACKAGE}/BUILD": "python_library()",
"pylintrc": "[pylint]\ndisable = C0103",
config_path: "[pylint]\ndisable = C0103",
}
)
tgt = rule_runner.get_target(Address(PACKAGE, relative_file_path="f.py"))
assert_success(rule_runner, tgt, extra_args=["--pylint-config=pylintrc"])
assert_success(rule_runner, tgt, extra_args=extra_args)


def test_respects_passthrough_args(rule_runner: RuleRunner) -> None:
def test_passthrough_args(rule_runner: RuleRunner) -> None:
rule_runner.write_files({f"{PACKAGE}/f.py": BAD_FILE, f"{PACKAGE}/BUILD": "python_library()"})
tgt = rule_runner.get_target(Address(PACKAGE, relative_file_path="f.py"))
assert_success(rule_runner, tgt, extra_args=["--pylint-args='--disable=C0103'"])
Expand Down
22 changes: 20 additions & 2 deletions src/python/pants/backend/python/lint/pylint/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,24 @@ def register_options(cls, register):
type=file_option,
default=None,
advanced=True,
help="Path to `pylintrc` or alternative Pylint config file.",
help=(
"Path to a config file understood by Pylint "
"(http://pylint.pycqa.org/en/latest/user_guide/run.html#command-line-options).\n\n"
f"Setting this option will disable `[{cls.options_scope}].config_discovery`. Use "
f"this option if the config is located in a non-standard location."
),
)
register(
"--config-discovery",
type=bool,
default=True,
advanced=True,
help=(
"If true, Pants will include any relevant config files during "
"runs (`.pylintrc`, `pylintrc`, `pyproject.toml`, and `setup.cfg`)."
f"\n\nUse `[{cls.options_scope}].config` instead if your config is in a "
f"non-standard location."
),
)
register(
"--source-plugins",
Expand Down Expand Up @@ -84,9 +101,10 @@ def config_request(self, dirs: Iterable[str]) -> ConfigFilesRequest:
# how config files are discovered.
return ConfigFilesRequest(
specified=self.config,
specified_option_name=f"[{self.options_scope}].config",
discovery=cast(bool, self.options.config_discovery),
check_existence=[".pylinrc", *(os.path.join(d, "pylintrc") for d in ("", *dirs))],
check_content={"pyproject.toml": b"[tool.pylint]", "setup.cfg": b"[pylint."},
option_name=f"[{self.options_scope}].config",
)

@property
Expand Down
Loading

0 comments on commit 3acdae9

Please sign in to comment.