bazel: add yamlfix support to fix_lint and lint_test - #11058
Conversation
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
There was a problem hiding this comment.
Code Review
This pull request integrates YAML formatting and checking into the Bazel build pipeline using yamlfix. It introduces new test and tidy targets, updates Python requirements, and hooks the formatter into the existing linting scripts. The review feedback highlights two critical issues: first, the formatting test script (yaml_fmt_test.sh) performs in-place modifications on the user's workspace instead of being read-only; second, both the test and tidy scripts can hang or enter infinite loops if no YAML files are found in the repository. Actionable suggestions are provided to safely run the test in a temporary directory and to handle empty file lists gracefully.
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
…t_test.sh Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Follow-Up Updates & Resolution
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce95b57e4e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Use temporary directory for formatting check to avoid workspace pollution - Fix runfiles resolution for yamlfix by preserving TOOL path - Resolve MODULE.bazel runfiles to find WORKSPACE - Handle case where no files are found to format to prevent infinite loops Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
@maliberty CI outage, please re-trigger builds. |
|
Not a CI outage: |
Head branch was pushed to by a user without write access
… linter - Create yamlfix.toml as Source of Truth for OpenROAD YAML formatting - Create yamlfix.ignore as single Source of Truth (DRI) for blacklisted YAML files - Update yaml_fmt_test.sh and yaml_tidy.sh to filter files via yamlfix.ignore - Include yamlfix.toml and yamlfix.ignore in BUILD.bazel data dependencies for fmt_yaml_test, tidy_yaml, and fix_lint Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
fixed. |
Summary
Extends OpenROAD's Bazel-managed linter/tidy toolchain (
fix_lintandlint_test) with native YAML formatting support usingyamlfix.Key Changes
bazel/requirements.in&requirements_lock_3_13.txt: Addedyamlfix==1.19.1to pip requirements.bazel/BUILD: Definedpy_console_script_binary(name = "yamlfix", pkg = "@openroad-pip//yamlfix").bazel/yaml_tidy.sh&yaml_fmt_test.sh: Created helper scripts for formatting and checking*.yamland*.ymlfiles relative to${BUILD_WORKSPACE_DIRECTORY}(respectingyamlfix.tomlif present).fix_lint.sh&BUILD.bazel: Integratedyamlfixintofix_lintandlint_test.Motivation & Relation to PR #11057
This PR works in tandem with PR #11057 to provide a single, unified linter/formatter toolchain (
bazelisk run //:fix_lintorbazelisk run //tools/OpenROAD:fix_lint) across OpenROAD and downstream repositories (such asOpenROAD-flow-scripts).