Skip to content

Commit 79baf2b

Browse files
authored
support custom dir (#7145)
### Summary This PR adds an option to specify BC linter config path, which will be used in vllm-project/vllm#21234 ### Test plan Locally test against PR: vllm-project/vllm#24614 Before: ``` ../test-infra/tools/stronghold/bin/check-api-compatibility --base-commit=b5e383cd8b62975dec605bed05e22d273c296c7a --head-commit=38ce6fa81660f0be6e0e61672fdc499bcd7fabc4 ::group::fetch github.event.pull_request.base.sha From ssh://github.com/zhewenl/vllm * branch b5e383cd8b62975dec605bed05e22d273c296c7a -> FETCH_HEAD ::endgroup:: fatal: path 'vllm/_bc_linter.py' exists on disk, but not in 'b5e383cd8b62975dec605bed05e22d273c296c7a' ::warning file=vllm/v1/core/sched/output.py,line=99::Function CachedRequestData: num_computed_tokens changed from list[int] to list[float] ::warning file=vllm/v1/core/sched/scheduler.py,line=1::Function Scheduler.get_grammar_bitmask: function deleted ::warning file=vllm/v1/core/sched/scheduler.py,line=1::Function Scheduler.update_draft_token_ids: function deleted ``` After: ``` ../test-infra/tools/stronghold/bin/check-api-compatibility --base-commit=b5e383cd8b62975dec605bed05e22d273c296c7a --head-commit=38ce6fa81660f0be6e0e61672fdc499bcd7fabc4 --config-dir=.github ::group::fetch github.event.pull_request.base.sha From ssh://github.com/zhewenl/vllm * branch b5e383cd8b62975dec605bed05e22d273c296c7a -> FETCH_HEAD ::endgroup:: BC-linter: Using .bc-linter.yml (parsed successfully) fatal: path 'vllm/_bc_linter.py' exists on disk, but not in 'b5e383cd8b62975dec605bed05e22d273c296c7a' ::warning file=vllm/v1/core/sched/output.py,line=99::Function CachedRequestData: num_computed_tokens changed from list[int] to list[float] > ../test-infra/tools/stronghold/bin/check-api-compatibility --base-commit=b5e383cd8b62975dec605bed05e22d273c296c7a --head-commit=38ce6fa81660f0be6e0e61672fdc499bcd7fabc4 ``` Note the difference in `scheduler.py`, before the we specifying the local config the bc linter yml was not respected - so the default rules are applied and we check ALL python files; after specifying the yml the warnings are restricted to ONLY the functions with annotations
1 parent 3a2dc97 commit 79baf2b

5 files changed

Lines changed: 50 additions & 11 deletions

File tree

.github/actions/bc-lint/action.yml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ inputs:
1919
description: 'Link to the docs to display in case of failure'
2020
required: false
2121
default: ''
22+
config_dir:
23+
description: 'Directory to load .bc-linter.yml from (defaults to repository root)'
24+
required: false
25+
default: ''
2226
runs:
2327
using: 'composite'
2428
steps:
@@ -66,7 +70,8 @@ runs:
6670
../_test-infra/tools/stronghold/bin/check-api-compatibility \
6771
--base-commit=${{ inputs.base_sha }} \
6872
--head-commit=${{ steps.merge_changes.outputs.new_head_sha }} \
69-
${{ inputs.suppression == 'true' && '--suppressed' || '' }}
73+
${{ inputs.suppression == 'true' && '--suppressed' || '' }} \
74+
${{ inputs.config_dir != '' && format('--config-dir={0}', inputs.config_dir) || '' }}
7075
7176
- name: Display documentation link if failed
7277
if: ${{ failure() && inputs.docs_link }}

tools/stronghold/docs/bc_linter_config.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ The config enables repo‑specific path selection, rule suppression, and custom
55
annotations to include/exclude specific APIs.
66

77
### Config file location
8-
- Place a YAML file named `.bc-linter.yml` at the repository root being linted
9-
(the target repo).
8+
- By default the linter searches for a `.bc-linter.yml` file at the root of
9+
the repository being linted.
10+
- Provide an alternative directory with `--config-dir` if the file lives
11+
somewhere else.
1012
- If the file is missing or empty, defaults are applied (see below).
1113

1214
### Schema (YAML)

tools/stronghold/src/api/checker.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ def run() -> None:
3232
action="store_true",
3333
help="Enable verbose output",
3434
)
35+
parser.add_argument(
36+
"--config-dir",
37+
type=str,
38+
default=None,
39+
help="Directory to load .bc-linter.yml from (defaults to repository root)",
40+
)
3541
args = parser.parse_args(sys.argv[1:])
3642

3743
repo = api.git.Repository(pathlib.Path("."))
@@ -46,7 +52,13 @@ def run() -> None:
4652
print("::endgroup::")
4753

4854
# Load config and optionally print when detected
49-
cfg, cfg_status = api.config.load_config_with_status(repo.dir)
55+
# By default, configuration is loaded from the repository root. A custom
56+
# configuration directory may be provided via ``config_dir`` either as an
57+
# absolute path or a path relative to ``repo_root``.
58+
cfg_path = pathlib.Path(args.config_dir) if args.config_dir else repo.dir
59+
if not cfg_path.is_absolute():
60+
cfg_path = repo.dir / cfg_path
61+
cfg, cfg_status = api.config.load_config_with_status(cfg_path)
5062
if cfg_status == "parsed":
5163
# Explicitly log successful config discovery and parsing
5264
print("BC-linter: Using .bc-linter.yml (parsed successfully)")

tools/stronghold/src/api/config.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,16 @@ def default_config() -> Config:
5454
return Config()
5555

5656

57-
def load_config_with_status(repo_root: pathlib.Path) -> tuple[Config, str]:
58-
"""Loads configuration from `.bc-linter.yml` in the given repository root.
57+
def load_config_with_status(config_dir: pathlib.Path) -> tuple[Config, str]:
58+
"""Loads configuration from `.bc-linter.yml` in the given directory.
5959
6060
Returns (config, status) where status is one of:
6161
- 'parsed' -> config file existed and parsed successfully
6262
- 'default_missing' -> no config file found
6363
- 'default_error' -> file existed but YAML missing/invalid or parser unavailable
6464
"""
65-
cfg_path = repo_root / ".bc-linter.yml"
65+
66+
cfg_path = config_dir / ".bc-linter.yml"
6667
if not cfg_path.exists():
6768
return (default_config(), "default_missing")
6869

@@ -159,12 +160,11 @@ def _ann_list(raw: Any) -> list[AnnotationSpec]:
159160
return (cfg, "parsed")
160161

161162

162-
def load_config(repo_root: pathlib.Path) -> Config:
163-
"""Loads configuration from `.bc-linter.yml` in the given repository root.
164-
163+
def load_config(config_dir: pathlib.Path) -> Config:
164+
"""Loads configuration from `.bc-linter.yml` in the given directory.
165165
If the file does not exist or cannot be parsed, returns defaults.
166166
"""
167-
cfg, _ = load_config_with_status(repo_root)
167+
cfg, _ = load_config_with_status(config_dir)
168168
return cfg
169169

170170

tools/stronghold/tests/api/test_config_loader.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,3 +149,23 @@ def test_warn_on_unknown_top_level_keys(tmp_path: pathlib.Path, capsys) -> None:
149149
assert status == "parsed"
150150
out = capsys.readouterr().out
151151
assert "::warning::BC-linter: Unknown keys in .bc-linter.yml: ['typpo']" in out
152+
153+
154+
def test_load_config_from_custom_directory(tmp_path: pathlib.Path) -> None:
155+
config_dir = tmp_path / "subdir"
156+
config_dir.mkdir()
157+
yml = textwrap.dedent(
158+
"""
159+
version: 1
160+
include: ["src/**/*.py"]
161+
"""
162+
)
163+
(config_dir / ".bc-linter.yml").write_text(yml)
164+
165+
cfg_rel, status_rel = load_config_with_status(config_dir)
166+
assert status_rel == "parsed"
167+
assert cfg_rel.include == ["src/**/*.py"]
168+
169+
cfg_abs, status_abs = load_config_with_status(config_dir)
170+
assert status_abs == "parsed"
171+
assert cfg_abs.include == ["src/**/*.py"]

0 commit comments

Comments
 (0)