Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .github/workflows/test-ci-buildifier.yaml
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:
Copy link
Collaborator

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

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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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/
19 changes: 19 additions & 0 deletions ci_buildifier/README.md
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.
38 changes: 38 additions & 0 deletions ci_buildifier/action.yaml
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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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?

38 changes: 38 additions & 0 deletions test/ci_buildifier/BUILD
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",
],
)
31 changes: 31 additions & 0 deletions test/ci_buildifier/subfolder/subfolder/BUILD
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",
],
)
56 changes: 56 additions & 0 deletions test/ci_buildifier/subfolder/test.bzl
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)
Loading