-
Notifications
You must be signed in to change notification settings - Fork 5
Add buildifier composite action and a test workflow #50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| # A workflow to test buildifier composite action | ||
|
|
||
| name: Test CI Buildifier Action | ||
| # Run on pull_request | ||
| on: | ||
| pull_request: | ||
| paths: | ||
| - ci_buildifier/** | ||
| - tests/ci_buildifier/** | ||
| - .github/workflows/test-ci-buildifier.yaml | ||
| branches: | ||
| - main | ||
| defaults: | ||
| run: | ||
| shell: bash | ||
| jobs: | ||
| run-buildifier: | ||
| runs-on: "linux-x86-n2-16" | ||
| container: "us-central1-docker.pkg.dev/tensorflow-sigs/tensorflow/ml-build:latest" | ||
| timeout-minutes: 60 | ||
| if: | | ||
| github.event.sender.type == 'User' || | ||
| contains(github.event.pull_request.body, 'RUN_BUILDIFIER') | ||
|
Comment on lines
+21
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like the other one i think we should just run it, the only bot we have is one we would want to have PRs run against anyway. For example if dependabot updates the sha of checkouts below, or of the aciton.yaml. I am more strongly suspecting XLA did this to prevent running on copybara created PRs. |
||
| steps: | ||
| - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 | ||
| with: | ||
| persist-credentials: false | ||
| - name: "Run buildifier action" | ||
| uses: ./ci_buildifier/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| # CI Buildifier | ||
|
|
||
| This composite action helps you maintain consistent formatting for your Bazel | ||
| `BUILD` and `.bzl` files. It automatically runs `buildifier` on any modified | ||
| files in your pull requests, ensuring code quality and adherence to style | ||
| conventions. If buildifier detects any formatting violations, your workflow | ||
| will fail, preventing unformatted code from being merged. | ||
|
|
||
| ## Resolving Formatting Failures | ||
| If a workflow run fails due to formatting violations, you're expected to | ||
| fix the issues locally. Simply run `buildifier` on the problematic | ||
| files, e.g., using | ||
| `buildifier -v <files>`, | ||
| and then commit the formatted code to your pull request. | ||
|
|
||
| ## Requirements | ||
| This action expects `buildifier` to be available in your workflow's runtime | ||
| environment. If `buildifier` is not pre-installed in your chosen runner image | ||
| or container, you will need to install it before this action runs. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # Copyright 2025 Google LLC | ||
|
|
||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
|
|
||
| # https://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| name: "Run buildifier" | ||
| description: 'Action to run buildifier on changed build files' | ||
|
|
||
| runs: | ||
| using: "composite" | ||
| steps: | ||
| - name: "Checking out repository" | ||
| uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 | ||
| with: | ||
| fetch-depth: 0 # Fetch full history for accurate diffing | ||
|
Comment on lines
+21
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same convo as the other one but we can have that conversation over there |
||
| - name: Run buildifier check | ||
| id: buildifier-check | ||
| shell: bash | ||
| env: | ||
| TARGET_BRANCH: ${{ github.base_ref }} | ||
| run: | | ||
| FILE_PATTERNS="BUILD */BUILD *.bzl" | ||
|
|
||
| # Add this line to resolve the dubious git ownership error | ||
| git config --global --add safe.directory "$GITHUB_WORKSPACE" | ||
|
|
||
| # Compare PR head against the base branch to find changed files. | ||
| GIT_DIFF_CMD="git diff -z --name-only --diff-filter=d origin/$TARGET_BRANCH HEAD -- $FILE_PATTERNS" | ||
|
|
||
| $GIT_DIFF_CMD | xargs -0 buildifier --mode=check -v | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as other, we need to decide what we do if its a push event. Do you know how long buildifier takes to run on the entire JAX repo? |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # This is a correctly formatted BUILD file. | ||
|
|
||
| package(default_visibility = ["//visibility:public"]) | ||
|
|
||
| licenses(["notice"]) # Apache 2.0 | ||
|
|
||
| cc_library( | ||
| name = "my_library", | ||
| srcs = [ | ||
| "my_library.cc", | ||
| ], | ||
| hdrs = [ | ||
| "my_library.h", | ||
| ], | ||
| deps = [ | ||
| "//third_party/some_lib", | ||
| "//util:common_utils", | ||
| ], | ||
| ) | ||
|
|
||
| cc_binary( | ||
| name = "my_binary", | ||
| srcs = [ | ||
| "main.cc", | ||
| ], | ||
| deps = [ | ||
| ":my_library", | ||
| "//framework:base", | ||
| ], | ||
| ) | ||
|
|
||
| py_library( | ||
| name = "py_utils", | ||
| srcs = ["py_utils.py"], | ||
| deps = [ | ||
| "//other_python:dependencies", | ||
| ], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| # This BUILD file contained intentional formatting errors | ||
| # and is then formatted by running the buildifier locally. | ||
|
|
||
| package(default_visibility = ["//visibility:public"]) | ||
|
|
||
| licenses(["notice"]) # Apache 2.0 | ||
|
|
||
| cc_library( | ||
| name = "my_library", | ||
| srcs = [ | ||
| "my_library.cc", | ||
| ], | ||
| hdrs = [ | ||
| "my_library.h", | ||
| ], | ||
| deps = [ | ||
| "//third_party/some_lib", | ||
| "//util:common_utils", | ||
| ], | ||
| ) | ||
|
|
||
| cc_binary( | ||
| name = "my_binary", | ||
| srcs = [ | ||
| "main.cc", | ||
| ], | ||
| deps = [ | ||
| ":my_library", | ||
| "//framework:base", | ||
| ], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| """ | ||
| This is a correctly formatted .bzl file for testing. | ||
| It defines a simple custom rule. | ||
| """ | ||
|
|
||
| load("@bazel_skylib//lib:attrs.bzl", "attrs") | ||
| load("@bazel_skylib//lib:collections.bzl", "dedup_list") | ||
| load("//some/dependency:defs.bzl", "dependency_macro") | ||
|
|
||
| def _my_custom_rule_impl(ctx): | ||
| """Implementation for my_custom_rule.""" | ||
| input_file = ctx.files.src[0] | ||
| output_file = ctx.actions.declare_file(ctx.attr.name + ".out") | ||
|
|
||
| ctx.actions.run( | ||
| inputs = [input_file], | ||
| outputs = [output_file], | ||
| executable = "/usr/bin/some_tool", | ||
| arguments = [ | ||
| "--input", | ||
| input_file.path, | ||
| "--output", | ||
| output_file.path, | ||
| "--verbose", | ||
| ], | ||
| mnemonic = "MyCustomRule", | ||
| ) | ||
|
|
||
| dependency_macro( | ||
| name = ctx.attr.name + "_dep", | ||
| data = ctx.attr.data, | ||
| ) | ||
|
|
||
| return [DefaultInfo(files = depset([output_file]))] | ||
|
|
||
| my_custom_rule = rule( | ||
| implementation = _my_custom_rule_impl, | ||
| attrs = { | ||
| "data": attrs.list( | ||
| default = [], | ||
| allow_files = True, | ||
| doc = "Additional data files.", | ||
| ), | ||
| "src": attrs.label( | ||
| allow_single_file = True, | ||
| mandatory = True, | ||
| doc = "The primary source file.", | ||
| ), | ||
| }, | ||
| doc = "A custom rule that processes a source file.", | ||
| ) | ||
|
|
||
| def another_macro(name, value, some_list = []): | ||
| """Another example macro.""" | ||
| print("Running another_macro with value: %s" % value) | ||
| dedup_list(some_list) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, i though zizmor should have flagged this but missing a permisions:{} block