Skip to content

bazel: add yamlfix support to fix_lint and lint_test - #11058

Merged
maliberty merged 5 commits into
The-OpenROAD-Project:masterfrom
oharboe:yamlfix-linter
Aug 4, 2026
Merged

bazel: add yamlfix support to fix_lint and lint_test#11058
maliberty merged 5 commits into
The-OpenROAD-Project:masterfrom
oharboe:yamlfix-linter

Conversation

@oharboe

@oharboe oharboe commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

Extends OpenROAD's Bazel-managed linter/tidy toolchain (fix_lint and lint_test) with native YAML formatting support using yamlfix.

Key Changes

  • bazel/requirements.in & requirements_lock_3_13.txt: Added yamlfix==1.19.1 to pip requirements.
  • bazel/BUILD: Defined py_console_script_binary(name = "yamlfix", pkg = "@openroad-pip//yamlfix").
  • bazel/yaml_tidy.sh & yaml_fmt_test.sh: Created helper scripts for formatting and checking *.yaml and *.yml files relative to ${BUILD_WORKSPACE_DIRECTORY} (respecting yamlfix.toml if present).
  • fix_lint.sh & BUILD.bazel: Integrated yamlfix into fix_lint and lint_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_lint or bazelisk run //tools/OpenROAD:fix_lint) across OpenROAD and downstream repositories (such as OpenROAD-flow-scripts).

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe
oharboe requested a review from a team as a code owner August 4, 2026 05:35
@oharboe
oharboe requested a review from eder-matheus August 4, 2026 05:35
@github-actions github-actions Bot added the size/M label Aug 4, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread bazel/yaml_fmt_test.sh
Comment thread bazel/yaml_tidy.sh Outdated
oharboe added 2 commits August 4, 2026 07:42
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
…t_test.sh

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe

oharboe commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-Up Updates & Resolution

  1. MODULE.bazel dev_dependency: Kept dev_dependency = True on buildifier_prebuilt (and yamlfix dependencies) in OpenROAD's MODULE.bazel so OpenROAD does not leak dev dependencies into downstream module graphs when consumed as a library.
  2. Empty File List Safety: Updated yaml_tidy.sh and yaml_fmt_test.sh to populate a FILES=() array from "$GIT" ls-files and execute $TOOL only when matching files exist, preventing hangs/infinite loops on empty file lists.

@maliberty

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread bazel/yaml_fmt_test.sh Outdated
Comment thread bazel/yaml_fmt_test.sh
@oharboe
oharboe requested review from a team as code owners August 4, 2026 17:23
@oharboe
oharboe requested review from gadfort and maliberty August 4, 2026 17:23
@github-actions github-actions Bot added size/XL and removed size/M labels Aug 4, 2026
- 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
maliberty enabled auto-merge August 4, 2026 17:50
@oharboe

oharboe commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

@maliberty CI outage, please re-trigger builds.

@maliberty

Copy link
Copy Markdown
Member

Not a CI outage:

[2026-08-04T19:04:02.958Z] //:fmt_yaml_test                                                         FAILED in 0.9s
[2026-08-04T19:04:02.958Z]   /home/ubuntu/.cache/bazel/_bazel_ubuntu/62e54d831f0ca88a7de859b7598af8d7/execroot/_main/bazel-out/k8-opt/testlogs/fmt_yaml_test/test.log
[2026-08-04T19:04:02.958Z] 

auto-merge was automatically disabled August 4, 2026 20:33

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>
@oharboe

oharboe commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

fixed.

@oharboe
oharboe requested a review from maliberty August 4, 2026 20:37
@maliberty
maliberty enabled auto-merge August 4, 2026 20:44
@maliberty
maliberty merged commit 5333308 into The-OpenROAD-Project:master Aug 4, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants