Polish the buildifier integration
diff --git a/buildifier/buildifier.py b/buildifier/buildifier.py
old mode 100644
new mode 100755
index 76398f3..04800f7
--- a/buildifier/buildifier.py
+++ b/buildifier/buildifier.py
@@ -1,37 +1,140 @@
#!/usr/bin/env python3
+# -*- coding: utf-8 -*-
+import fnmatch
+import html
import os.path
import re
import subprocess
import sys
regex = re.compile(
- r"(?P<filename>[^:]*):(?P<line>\d*):(?:(?P<column>\d*):)? (?P<message>.*)"
+ r"(?P<filename>[^:]*):(?P<line>\d*):(?:(?P<column>\d*):)? (?P<message_id>[^:]*): (?P<message>.*) \((?P<message_url>.*)\)"
)
-files = []
-for root, dirnames, filenames in os.walk("."):
- for filename in filenames:
- if filename in ["BUILD", "BUILD.bazel"] or filename.endswith(".bzl"):
- files.append(os.path.relpath(os.path.join(root, filename)))
-result = subprocess.run(
- ["buildifier", "--lint=warn"] + sorted(files),
- capture_output=True,
- universal_newlines=True,
-)
+def eprint(*args, **kwargs):
+ """
+ Print to stderr and flush (just in case).
+ """
+ print(*args, flush=True, file=sys.stderr, **kwargs)
-findings = []
-messages = []
-for line in result.stderr.splitlines():
- match = regex.match(line)
+def upload_output(output):
+ # Generate output usable by Buildkite's annotations.
+ eprint("--- :hammer_and_wrench: Printing raw output for debugging")
+ eprint(output)
+
+ eprint("+++ :buildkite: Uploading output via 'buildkite annotate'")
+ result = subprocess.run(
+ ["buildkite-agent", "annotate", "--style", "warning", "--context", "buildifier"],
+ input=output,
+ )
+ if result.returncode != 0:
+ eprint(
+ ":rotating_light: 'buildkite-agent annotate' failed with exit code {}".format(
+ result.returncode
+ )
+ )
+
+
+def get_file_url(filename, line):
+ commit = os.environ.get("BUILDKITE_COMMIT")
+ repo = os.environ.get("BUILDKITE_PULL_REQUEST_REPO", os.environ.get("BUILDKITE_REPO", None))
+ if not commit or not repo:
+ return None
+
+ # Example 1: https://github.com/bazelbuild/bazel.git
+ # Example 2: git://github.com/philwo/bazel.git
+ # Example 3: git@github.com:bazelbuild/bazel.git
+ match = re.match(r"(?:(?:git|https?)://|git@)(github.com[:/].*)\.git", repo)
if match:
- findings.append(match)
- else:
- messages.append(line)
+ return "https://{}/blob/{}/{}#L{}".format(
+ match[1].replace(":", "/"), commit, filename, line
+ )
-if not findings and not messages:
- sys.exit(0)
+ return None
-for finding in findings:
+
+def main(argv=None):
+ if argv is None:
+ argv = sys.argv[1:]
+
+ # Gather all files to process.
+ eprint("+++ :female-detective: Looking for BUILD, BUILD.bazel and *.bzl files")
+ files = []
+ build_bazel_found = False
+ for root, dirnames, filenames in os.walk("."):
+ for filename in filenames:
+ if fnmatch.fnmatch(filename, "BUILD.bazel"):
+ build_bazel_found = True
+ for pattern in ("BUILD", "BUILD.bazel", "*.bzl"):
+ if fnmatch.fnmatch(filename, pattern):
+ files.append(os.path.relpath(os.path.join(root, filename)))
+ if build_bazel_found:
+ eprint(
+ "Found BUILD.bazel files in the workspace, thus ignoring BUILD files without suffix."
+ )
+ files = [fname for fname in files if not fnmatch(os.path.basename(fname), "BUILD")]
+ if not files:
+ eprint("No files found, exiting.")
+ return 0
+
+ # Run buildifier.
+ eprint("+++ :bazel: Running buildifier")
+ result = subprocess.run(
+ ["buildifier", "--lint=warn"] + sorted(files), capture_output=True, universal_newlines=True
+ )
+
+ # If buildifier was happy, there's nothing left to do for us.
+ if result.returncode == 0:
+ eprint("+++ :tada: Buildifier found nothing to complain about")
+ return result.returncode
+
+ # Parse output.
+ eprint("+++ :gear: Parsing buildifier output")
+ findings = []
+ for line in result.stderr.splitlines():
+ # Skip empty lines.
+ line = line.strip()
+ if not line:
+ continue
+
+ # Try to parse as structured data.
+ match = regex.match(line)
+ if match:
+ findings.append(match)
+ else:
+ output = "##### :bazel: buildifier: error while parsing output\n"
+ output += "<pre><code>" + html.escape(result.stderr) + "</code></pre>"
+ if "BUILDKITE_JOB_ID" in os.environ:
+ output += "\n\nSee [job {job}](#{job})\n".format(job=os.environ["BUILDKITE_JOB_ID"])
+ upload_output(output)
+ return result.returncode
+
+ output = "##### :bazel: buildifier: found {} problems in your BUILD and *.bzl files\n".format(
+ len(findings)
+ )
+ output += "<pre><code>"
+ for finding in findings:
+ file_url = get_file_url(finding["filename"], finding["line"])
+ if file_url:
+ output += '<a href="{}">{}:{}</a>:'.format(
+ file_url, finding["filename"], finding["line"]
+ )
+ else:
+ output += "{}:{}:".format(finding["filename"], finding["line"])
+ if finding["column"]:
+ output += "{}:".format(finding["column"])
+ output += ' <a href="{}">{}</a>: {}\n'.format(
+ finding["message_url"], finding["message_id"], finding["message"]
+ )
+ output = output.strip() + "</pre></code>"
+ upload_output(output)
+
+ # Preserve buildifier's exit code.
+ return result.returncode
+
+
+if __name__ == "__main__":
+ sys.exit(main())
diff --git a/buildkite/README.md b/buildkite/README.md
index 8b4328a..bf8d5ad 100644
--- a/buildkite/README.md
+++ b/buildkite/README.md
@@ -121,11 +121,10 @@
For each pipeline you can enable [Buildifier](https://github.com/bazelbuild/buildtools/tree/master/buildifier) to check whether all BUILD, BUILD.bazel and .bzl files comply with the standard formatting convention. Simply add the following code to the top of the particular pipeline Yaml configuration (either locally in `.bazelci/presubmit.yml` or in https://github.com/bazelbuild/continuous-integration/tree/master/buildkite/pipelines):
-```
+```yaml
---
-buildifier: 1
+buildifier: true
[...]
```
As a consequence, every future build for this pipeline will contain an additional "Buildifier" step that runs the latest version of Buildifier in "lint" mode.
-
diff --git a/buildkite/bazelci.py b/buildkite/bazelci.py
index a764abc..3ec1cfb 100644
--- a/buildkite/bazelci.py
+++ b/buildkite/bazelci.py
@@ -41,6 +41,11 @@
# Initialize the random number generator.
random.seed()
+CLOUD_PROJECT = (
+ "bazel-public"
+ if os.environ["BUILDKITE_ORGANIZATION_SLUG"] == "bazel-trusted"
+ else "bazel-untrusted"
+)
DOWNSTREAM_PROJECTS = {
"Android Testing": {
@@ -235,12 +240,6 @@
},
}
-CLOUD_PROJECT = (
- "bazel-public"
- if os.environ["BUILDKITE_ORGANIZATION_SLUG"] == "bazel-trusted"
- else "bazel-untrusted"
-)
-
# A map containing all supported platform names as keys, with the values being
# the platform name in a human readable format, and a the buildkite-agent's
# working directory.
@@ -326,9 +325,7 @@
},
}
-BUILDIFIER_INPUT_FILES = ["BUILD", "BUILD.bazel", "*.bzl"]
-
-BUILDIFIER_DOCKER_IMAGE = "gcr.io/bazel-untrusted/ubuntu1804:nojava"
+BUILDIFIER_DOCKER_IMAGE = f"gcr.io/{CLOUD_PROJECT}/buildkite:buildifier"
# The platform used for various steps (e.g. stuff that formerly ran on the "pipeline" workers).
DEFAULT_PLATFORM = "ubuntu1804"
@@ -686,7 +683,9 @@
]
)
except subprocess.CalledProcessError as e:
- raise BuildkiteException("Failed to download Bazel binary at %s, error message:\n%s" % (bazel_git_commit, str(e)))
+ raise BuildkiteException(
+ "Failed to download Bazel binary at %s, error message:\n%s" % (bazel_git_commit, str(e))
+ )
st = os.stat(bazel_binary_path)
os.chmod(bazel_binary_path, st.st_mode | stat.S_IEXEC)
return bazel_binary_path
@@ -967,9 +966,7 @@
print_expanded_group(":bazel: Clean")
try:
- execute_command(
- [bazel_binary] + common_startup_flags(platform) + ["clean", "--expunge"]
- )
+ execute_command([bazel_binary] + common_startup_flags(platform) + ["clean", "--expunge"])
except subprocess.CalledProcessError as e:
raise BuildkiteException("bazel clean failed with exit code {}".format(e.returncode))
@@ -1101,7 +1098,9 @@
def create_step(label, commands, platform=DEFAULT_PLATFORM):
host_platform = PLATFORMS[platform].get("host-platform", platform)
if "docker-image" in PLATFORMS[platform]:
- return create_docker_step(label, commands, PLATFORMS[platform]["docker-image"])
+ return create_docker_step(
+ label, image=PLATFORMS[platform]["docker-image"], commands=commands
+ )
else:
return {
"label": label,
@@ -1114,8 +1113,8 @@
}
-def create_docker_step(label, commands, docker_image):
- return {
+def create_docker_step(label, image, commands=None):
+ step = {
"label": label,
"command": commands,
"agents": {"kind": "docker", "os": "linux"},
@@ -1124,7 +1123,7 @@
"always-pull": True,
"debug": True,
"environment": ["BUILDKITE_ARTIFACT_UPLOAD_DESTINATION", "BUILDKITE_GS_ACL"],
- "image": docker_image,
+ "image": image,
"network": "host",
"privileged": True,
"propagate-environment": True,
@@ -1137,6 +1136,9 @@
}
},
}
+ if not step["command"]:
+ del step["command"]
+ return step
def print_project_pipeline(
@@ -1155,6 +1157,9 @@
pipeline_steps = []
+ if configs.get("buildifier"):
+ pipeline_steps.append(create_docker_step("Buildifier", image=BUILDIFIER_DOCKER_IMAGE))
+
# In Bazel Downstream Project pipelines, git_repository and project_name must be specified,
# and we should test the project at the last green commit.
git_commit = None
@@ -1176,10 +1181,6 @@
)
pipeline_steps.append(step)
- buildifier_step = get_buildifier_step_if_requested(configs)
- if buildifier_step:
- pipeline_steps.append(buildifier_step)
-
pipeline_slug = os.getenv("BUILDKITE_PIPELINE_SLUG")
all_downstream_pipeline_slugs = []
for _, config in DOWNSTREAM_PROJECTS.items():
@@ -1256,20 +1257,6 @@
)
-def get_buildifier_step_if_requested(configs):
- # There may be a "buildifier" entry in the config, but the value can be "false".
- if not configs.get("buildifier"):
- return None
-
- find_args = " -or ".join('-iname "{}"'.format(f) for f in BUILDIFIER_INPUT_FILES)
- # We need to escape the $ sign in order to not fail on Buildkite.
- # https://buildkite.com/docs/agent/v3/cli-pipeline#environment-variable-substitution
- command = ('buildifier --lint=warn $$(find . -type f \\( {} \\)) | grep -q "."').format(
- find_args
- )
- return create_docker_step("Buildifier", [command], BUILDIFIER_DOCKER_IMAGE)
-
-
def upload_project_pipeline_step(
project_name, git_repository, http_config, file_config, incompatible_flags
):
