From 4a1e3319e9a24a37e4e6b4306e7bfc6f58720be1 Mon Sep 17 00:00:00 2001 From: Dominik Roos Date: Mon, 30 Oct 2023 18:25:49 +0100 Subject: [PATCH] build: drop @cgrindel_bazel_starlib//updatesrc Refactor the remaining targets that depended on @cgrindel_bazel_starlib//updatesrc. The repository has now been completely migrated to @aspect_bazel_lib//lib:write_source_files. During the refactor, we dropped the functionality to write the source files in rules_openapi. Instead, the caller side can decide how to write the source files. This makes the rules more composable and easier to use. The functionality has been moved to a macro in private/mgmtapi/api.bzl, which is used inside of scionproto/scion. --- .bazelignore | 1 + BUILD.bazel | 19 ++++---- Makefile | 1 - WORKSPACE | 6 --- control/mgmtapi/BUILD.bazel | 3 +- daemon/mgmtapi/BUILD.bazel | 3 +- dispatcher/mgmtapi/BUILD.bazel | 3 +- gateway/mgmtapi/BUILD.bazel | 3 +- private/ca/api/BUILD.bazel | 2 +- private/mgmtapi/api.bzl | 58 ++++++++++++++++++++++++ private/mgmtapi/cppki/api/BUILD.bazel | 3 +- private/mgmtapi/health/api/BUILD.bazel | 3 +- private/mgmtapi/segments/api/BUILD.bazel | 3 +- router/mgmtapi/BUILD.bazel | 3 +- rules_openapi/defs.bzl | 46 ++++++------------- rules_openapi/dependencies.bzl | 12 ----- rules_openapi/internal/bundle.bzl | 11 ++++- rules_openapi/internal/generate.bzl | 45 +++++++++--------- rules_openapi/internal/header.bzl | 45 ------------------ rules_openapi/rules_starlib.patch | 13 ------ spec/BUILD.bazel | 16 +++++++ tools/lint/go_embed.bzl | 7 --- 22 files changed, 143 insertions(+), 163 deletions(-) create mode 100644 private/mgmtapi/api.bzl delete mode 100644 rules_openapi/internal/header.bzl delete mode 100644 rules_openapi/rules_starlib.patch diff --git a/.bazelignore b/.bazelignore index 19dbe95779..1c4373a08f 100644 --- a/.bazelignore +++ b/.bazelignore @@ -1,4 +1,5 @@ bin +doc/_build docker/_build rules_openapi/tools/node_modules tools/lint/logctxcheck/testdata/src diff --git a/BUILD.bazel b/BUILD.bazel index 267914fd4b..fcc01787bd 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -7,7 +7,6 @@ load("//tools/lint:write_source_files.bzl", "write_source_files") load("//tools/lint/python:flake8_config.bzl", "flake8_lint_config") load("//:nogo.bzl", "nogo_deps") load("@com_github_bazelbuild_buildtools//buildifier:def.bzl", "buildifier") -load("@cgrindel_bazel_starlib//updatesrc:defs.bzl", "updatesrc_update_all") # gazelle:prefix github.com/scionproto/scion # gazelle:map_kind go_library go_library //tools/lint:go.bzl @@ -233,19 +232,21 @@ buildifier( mode = "check", ) -# Runs all update_src targets in this Workspace. Currently, generating the -# OpenAPI specs is the last target that depends on update_src. Eventually, -# this should be transitioned to write_all_source_files below. -updatesrc_update_all( - name = "update_all", -) - # Runs all write_source_files targets in this Workspace. To update the list run # bazel run @com_github_bazelbuild_buildtools//buildozer -- --root_dir $PWD "add additional_update_targets $( bazel query 'filter("^.*[^\d]$", kind(_write_source_file, //...)) except //:write_all_source_files' | tr '\n' ' ')" //:write_all_source_files write_source_files( name = "write_all_source_files", additional_update_targets = [ - "//doc/_build/_static/command:write_files", + "//control/mgmtapi:write_files", + "//daemon/mgmtapi:write_files", + "//dispatcher/mgmtapi:write_files", "//doc/command:write_files", + "//gateway/mgmtapi:write_files", + "//private/ca/api:write_files", + "//private/mgmtapi/cppki/api:write_files", + "//private/mgmtapi/health/api:write_files", + "//private/mgmtapi/segments/api:write_files", + "//router/mgmtapi:write_files", + "//spec:write_files", ], ) diff --git a/Makefile b/Makefile index 2cb2419407..e5a3ae2264 100644 --- a/Makefile +++ b/Makefile @@ -70,7 +70,6 @@ antlr: write_all_source_files: bazel run //:write_all_source_files - bazel run //:update_all .PHONY: lint lint-bazel lint-bazel-buildifier lint-doc lint-doc-mdlint lint-go lint-go-bazel lint-go-gazelle lint-go-golangci lint-go-semgrep lint-openapi lint-openapi-spectral lint-protobuf lint-protobuf-buf diff --git a/WORKSPACE b/WORKSPACE index 491241f5f5..c36638c2e6 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -297,9 +297,3 @@ rules_openapi_dependencies() load("//rules_openapi:install.bzl", "rules_openapi_install_yarn_dependencies") rules_openapi_install_yarn_dependencies() - -# TODO(lukedirtwalker): can that be integrated in the rules_openapi_dependencies -# call above somehow? -load("@cgrindel_bazel_starlib//:deps.bzl", "bazel_starlib_dependencies") - -bazel_starlib_dependencies() diff --git a/control/mgmtapi/BUILD.bazel b/control/mgmtapi/BUILD.bazel index 69012989b4..c2e82115a4 100644 --- a/control/mgmtapi/BUILD.bazel +++ b/control/mgmtapi/BUILD.bazel @@ -1,5 +1,6 @@ load("//tools/lint:go.bzl", "go_library", "go_test") -load("@com_github_scionproto_scion//rules_openapi:defs.bzl", "openapi_build_docs", "openapi_generate_go") +load("@com_github_scionproto_scion//rules_openapi:defs.bzl", "openapi_build_docs") +load("//private/mgmtapi:api.bzl", "openapi_generate_go") openapi_build_docs( name = "doc", diff --git a/daemon/mgmtapi/BUILD.bazel b/daemon/mgmtapi/BUILD.bazel index 33f3f3ca05..e9814e69ec 100644 --- a/daemon/mgmtapi/BUILD.bazel +++ b/daemon/mgmtapi/BUILD.bazel @@ -1,5 +1,6 @@ load("//tools/lint:go.bzl", "go_library") -load("@com_github_scionproto_scion//rules_openapi:defs.bzl", "openapi_build_docs", "openapi_generate_go") +load("@com_github_scionproto_scion//rules_openapi:defs.bzl", "openapi_build_docs") +load("//private/mgmtapi:api.bzl", "openapi_generate_go") openapi_build_docs( name = "doc", diff --git a/dispatcher/mgmtapi/BUILD.bazel b/dispatcher/mgmtapi/BUILD.bazel index d63ed1b5d8..6fe00d67b0 100644 --- a/dispatcher/mgmtapi/BUILD.bazel +++ b/dispatcher/mgmtapi/BUILD.bazel @@ -1,5 +1,6 @@ load("//tools/lint:go.bzl", "go_library") -load("//rules_openapi:defs.bzl", "openapi_build_docs", "openapi_generate_go") +load("//rules_openapi:defs.bzl", "openapi_build_docs") +load("//private/mgmtapi:api.bzl", "openapi_generate_go") openapi_build_docs( name = "doc", diff --git a/gateway/mgmtapi/BUILD.bazel b/gateway/mgmtapi/BUILD.bazel index 22076e1293..49e5d6d877 100644 --- a/gateway/mgmtapi/BUILD.bazel +++ b/gateway/mgmtapi/BUILD.bazel @@ -1,5 +1,6 @@ load("//tools/lint:go.bzl", "go_library") -load("@com_github_scionproto_scion//rules_openapi:defs.bzl", "openapi_build_docs", "openapi_generate_go") +load("@com_github_scionproto_scion//rules_openapi:defs.bzl", "openapi_build_docs") +load("//private/mgmtapi:api.bzl", "openapi_generate_go") openapi_build_docs( name = "doc", diff --git a/private/ca/api/BUILD.bazel b/private/ca/api/BUILD.bazel index f23d09bbc9..2d17595466 100644 --- a/private/ca/api/BUILD.bazel +++ b/private/ca/api/BUILD.bazel @@ -1,5 +1,5 @@ load("//tools/lint:go.bzl", "go_library") -load("@com_github_scionproto_scion//rules_openapi:defs.bzl", "openapi_generate_go") +load("//private/mgmtapi:api.bzl", "openapi_generate_go") openapi_generate_go( name = "api_generated", diff --git a/private/mgmtapi/api.bzl b/private/mgmtapi/api.bzl new file mode 100644 index 0000000000..ec6b033bff --- /dev/null +++ b/private/mgmtapi/api.bzl @@ -0,0 +1,58 @@ +"""OpenAPI Macros +Macros for generating Go code from OpenAPI specs. +""" + +load("//tools/lint:write_source_files.bzl", "write_source_files") +load("//rules_openapi:defs.bzl", _openapi_generate_go = "openapi_generate_go") + +def openapi_generate_go( + name, + client = True, + server = True, + spec = True, + types = True, + **kwargs): + """ + Generates Go code from an OpenAPI spec. + + This macro creates two additional rules: + - {{name}}_files: A filegroup with the generated files. + - write_files: A write_source_files rule that writes the generated files to + the source directory. + + Args: + name: The name of the rule. + client: Whether to generate a client. + server: Whether to generate a server. + spec: Whether to generate a spec. + types: Whether to generate types. + **kwargs: Ad. + """ + + _openapi_generate_go( + name = name + "_gen", + out_client = "client.bzl.gen.go" if client else None, + out_server = "server.bzl.gen.go" if server else None, + out_spec = "spec.bzl.gen.go" if spec else None, + out_types = "types.bzl.gen.go" if types else None, + **kwargs + ) + + out_files = [] + write_files = {} + for typ, gen in {"client": client, "server": server, "spec": spec, "types": types}.items(): + if not gen: + continue + src = typ + ".bzl.gen.go" + out_files.append(src) + write_files[typ + ".gen.go"] = src + + native.filegroup( + name = name, + srcs = out_files, + ) + + write_source_files( + name = "write_files", + files = write_files, + ) diff --git a/private/mgmtapi/cppki/api/BUILD.bazel b/private/mgmtapi/cppki/api/BUILD.bazel index 01cc4fe911..8e0d5d1f33 100644 --- a/private/mgmtapi/cppki/api/BUILD.bazel +++ b/private/mgmtapi/cppki/api/BUILD.bazel @@ -1,10 +1,9 @@ load("//tools/lint:go.bzl", "go_library", "go_test") -load("@com_github_scionproto_scion//rules_openapi:defs.bzl", "openapi_generate_go") +load("//private/mgmtapi:api.bzl", "openapi_generate_go") openapi_generate_go( name = "api_generated", src = "//spec:cppki", - server = True, spec = False, ) diff --git a/private/mgmtapi/health/api/BUILD.bazel b/private/mgmtapi/health/api/BUILD.bazel index a547133556..da5148b4b4 100644 --- a/private/mgmtapi/health/api/BUILD.bazel +++ b/private/mgmtapi/health/api/BUILD.bazel @@ -1,10 +1,9 @@ load("//tools/lint:go.bzl", "go_library") -load("@com_github_scionproto_scion//rules_openapi:defs.bzl", "openapi_generate_go") +load("//private/mgmtapi:api.bzl", "openapi_generate_go") openapi_generate_go( name = "api_generated", src = "//spec:health", - server = True, spec = False, ) diff --git a/private/mgmtapi/segments/api/BUILD.bazel b/private/mgmtapi/segments/api/BUILD.bazel index 37c6e33fde..ef2a43e8df 100644 --- a/private/mgmtapi/segments/api/BUILD.bazel +++ b/private/mgmtapi/segments/api/BUILD.bazel @@ -1,10 +1,9 @@ load("//tools/lint:go.bzl", "go_library", "go_test") -load("@com_github_scionproto_scion//rules_openapi:defs.bzl", "openapi_generate_go") +load("//private/mgmtapi:api.bzl", "openapi_generate_go") openapi_generate_go( name = "api_generated", src = "//spec:segments", - server = True, spec = False, ) diff --git a/router/mgmtapi/BUILD.bazel b/router/mgmtapi/BUILD.bazel index f1d74c2a54..c57c59a4e0 100644 --- a/router/mgmtapi/BUILD.bazel +++ b/router/mgmtapi/BUILD.bazel @@ -1,5 +1,6 @@ load("//tools/lint:go.bzl", "go_library", "go_test") -load("@com_github_scionproto_scion//rules_openapi:defs.bzl", "openapi_build_docs", "openapi_generate_go") +load("@com_github_scionproto_scion//rules_openapi:defs.bzl", "openapi_build_docs") +load("//private/mgmtapi:api.bzl", "openapi_generate_go") openapi_build_docs( name = "doc", diff --git a/rules_openapi/defs.bzl b/rules_openapi/defs.bzl index fc537789c3..f3a65e6804 100644 --- a/rules_openapi/defs.bzl +++ b/rules_openapi/defs.bzl @@ -1,8 +1,6 @@ load("//rules_openapi/internal:generate.bzl", _openapi_generate_go = "openapi_generate_go") load("//rules_openapi/internal:bundle.bzl", _openapi_bundle = "openapi_bundle") load("//rules_openapi/internal:docs.bzl", _openapi_build_docs = "openapi_build_docs") -load("//rules_openapi/internal:header.bzl", _header = "header") -load("@cgrindel_bazel_starlib//updatesrc:defs.bzl", "updatesrc_update") def openapi_bundle( name, @@ -10,22 +8,22 @@ def openapi_bundle( entrypoint, visibility = None): _openapi_bundle( - name = name, + name = name + "-no-header", + out = name + "-no-header.bzl.gen.yml", srcs = srcs, entrypoint = entrypoint, - visibility = visibility, - ) - - _header_target = name + "-add-header" - _header( - name = _header_target, - srcs = [":" + name], - header = "# GENERATED FILE DO NOT EDIT", ) - - updatesrc_update( - name = name + "-update", - deps = [":" + _header_target], + native.genrule( + name = name, + srcs = [name + "-no-header"], + outs = [name + ".bzl.gen.yml"], + cmd = "$(location @com_github_scionproto_scion//rules_openapi/internal:header.sh) \ + '$(location " + name + "-no-header)' \ + '$(location " + name + ".bzl.gen.yml)' \ + '# GENERATED FILE DO NOT EDIT' \ + ", + tools = ["@com_github_scionproto_scion//rules_openapi/internal:header.sh"], + visibility = visibility, ) def openapi_generate_go( @@ -36,24 +34,6 @@ def openapi_generate_go( **kwargs ) - generate = { - "types": kwargs.get("types", True), - "server": kwargs.get("server", True), - "client": kwargs.get("client", True), - "spec": kwargs.get("spec", True), - } - - srcs = [] - for k, v in generate.items(): - if not v: - continue - srcs.append(k + ".gen.go") - updatesrc_update( - name = name + "-update", - srcs = srcs, - outs = [":" + name], - ) - def openapi_build_docs( name, src, diff --git a/rules_openapi/dependencies.bzl b/rules_openapi/dependencies.bzl index ebb2315805..47a02d3b3d 100644 --- a/rules_openapi/dependencies.bzl +++ b/rules_openapi/dependencies.bzl @@ -8,15 +8,3 @@ def rules_openapi_dependencies(): sha256 = "b32a4713b45095e9e1921a7fcb1adf584bc05959f3336e7351bcf77f015a2d7c", urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/4.1.0/rules_nodejs-4.1.0.tar.gz"], ) - - maybe( - http_archive, - name = "cgrindel_bazel_starlib", - sha256 = "163a45d949fdb96b328bb44fe56976c610c6728c77118c6cd999f26cedca97eb", - strip_prefix = "bazel-starlib-0.2.1", - urls = [ - "http://github.com/cgrindel/bazel-starlib/archive/v0.2.1.tar.gz", - ], - patches = ["@com_github_scionproto_scion//rules_openapi:rules_starlib.patch"], - patch_args = ["-p1"], - ) diff --git a/rules_openapi/internal/bundle.bzl b/rules_openapi/internal/bundle.bzl index 6021eb1c86..f0bd3e0978 100644 --- a/rules_openapi/internal/bundle.bzl +++ b/rules_openapi/internal/bundle.bzl @@ -1,8 +1,11 @@ load("@bazel_skylib//lib:shell.bzl", "shell") def _openapi_bundle_impl(ctx): - prefix = ctx.label.name - out_file = ctx.actions.declare_file(prefix + ".gen.yml") + if ctx.outputs.out: + out_file = ctx.outputs.out + else: + out_file = ctx.actions.declare_file(ctx.label.name + ".bzl.gen.yml") + cmd = "{bin} bundle --output {out} {entrypoint}".format( bin = ctx.executable._openapi_cli.path, out = shell.quote(out_file.path), @@ -29,6 +32,10 @@ openapi_bundle = rule( doc = "All files that are referenced in the entrypoint file", allow_files = [".yml"], ), + "out": attr.output( + doc = "The bundled open API specification file", + mandatory = False, + ), "entrypoint": attr.label( doc = "The main source to generate files from", allow_single_file = [".yml"], diff --git a/rules_openapi/internal/generate.bzl b/rules_openapi/internal/generate.bzl index 4987523d1d..8b2cda5f2c 100644 --- a/rules_openapi/internal/generate.bzl +++ b/rules_openapi/internal/generate.bzl @@ -2,16 +2,15 @@ load("@bazel_skylib//lib:shell.bzl", "shell") def _openapi_generate_go(ctx): generate = { - "types": ctx.attr.types, - "server": ctx.attr.server, - "client": ctx.attr.client, - "spec": ctx.attr.spec, + "types": ctx.outputs.out_types, + "server": ctx.outputs.out_server, + "client": ctx.outputs.out_client, + "spec": ctx.outputs.out_spec, } out_files = [] - for k, v in generate.items(): - if not v: + for k, out_file in generate.items(): + if not out_file: continue - out_file = ctx.actions.declare_file(k + ".gen.go") generate_kind = k if generate_kind == "server": generate_kind = "chi-server" @@ -69,26 +68,10 @@ openapi_generate_go = rule( doc = "The Go package the generated code should live in.", default = "api", ), - "types": attr.bool( - doc = "Whether the types file should be generated", - default = True, - ), "types_excludes": attr.label( doc = "The file containing the schema list to exclude during the types generation.", allow_single_file = True, ), - "server": attr.bool( - doc = "Whether the server code should be generated", - default = True, - ), - "client": attr.bool( - doc = "Whehter the client code should be generated", - default = True, - ), - "spec": attr.bool( - doc = "Whether the spec code should be generated", - default = True, - ), "templates": attr.label_list( doc = """Folder containing Go templates to be used during code generation. Note that the template file names need to match the ones used by the @@ -102,5 +85,21 @@ openapi_generate_go = rule( executable = True, cfg = "target", ), + "out_types": attr.output( + doc = "The generated types file.", + mandatory = False, + ), + "out_server": attr.output( + doc = "The generated server file.", + mandatory = False, + ), + "out_client": attr.output( + doc = "The generated client file.", + mandatory = False, + ), + "out_spec": attr.output( + doc = "The generated spec file.", + mandatory = False, + ), }, ) diff --git a/rules_openapi/internal/header.bzl b/rules_openapi/internal/header.bzl deleted file mode 100644 index f061dad1eb..0000000000 --- a/rules_openapi/internal/header.bzl +++ /dev/null @@ -1,45 +0,0 @@ -# This file was copied from https://github.com/cgrindel/rules_updatesrc/tree/main/examples/simple/header -load( - "@cgrindel_bazel_starlib//updatesrc:defs.bzl", - "UpdateSrcsInfo", - "update_srcs", -) - -def _header_impl(ctx): - outs = [] - updsrcs = [] - for src in ctx.files.srcs: - out = ctx.actions.declare_file(src.basename + "_with_header") - outs.append(out) - updsrcs.append(update_srcs.create(src = src, out = out)) - ctx.actions.run( - outputs = [out], - inputs = [src], - executable = ctx.executable._header_tool, - arguments = [src.path, out.path, ctx.attr.header], - ) - - return [ - DefaultInfo(files = depset(outs)), - UpdateSrcsInfo(update_srcs = depset(updsrcs)), - ] - -header = rule( - implementation = _header_impl, - attrs = { - "srcs": attr.label_list( - allow_files = True, - mandatory = True, - ), - "header": attr.string( - mandatory = True, - ), - "_header_tool": attr.label( - default = "@com_github_scionproto_scion//rules_openapi/internal:header.sh", - executable = True, - cfg = "host", - allow_files = True, - ), - }, - doc = "Adds a header to the src files.", -) diff --git a/rules_openapi/rules_starlib.patch b/rules_openapi/rules_starlib.patch deleted file mode 100644 index 3ea25836da..0000000000 --- a/rules_openapi/rules_starlib.patch +++ /dev/null @@ -1,13 +0,0 @@ -diff --git a/updatesrc/private/updatesrc_update.bzl b/updatesrc/private/updatesrc_update.bzl -index c0cbfc1..a8d24ca 100644 ---- a/updatesrc/private/updatesrc_update.bzl -+++ b/updatesrc/private/updatesrc_update.bzl -@@ -49,7 +49,7 @@ if [[ ! -z "${BUILD_WORKSPACE_DIRECTORY}" ]]; then - cd "${BUILD_WORKSPACE_DIRECTORY}" - fi - """ + "\n".join([ -- "cp -f $(readlink \"${{runfiles_dir}}/{out}\") {src}".format( -+ "cp -f $(readlink \"${{runfiles_dir}}/{out}\") {src}\nchmod 0444 {src}".format( - src = updsrc.src.short_path, - out = updsrc.out.short_path, - ) diff --git a/spec/BUILD.bazel b/spec/BUILD.bazel index 49b6a8e3bf..88f9cea64a 100644 --- a/spec/BUILD.bazel +++ b/spec/BUILD.bazel @@ -1,4 +1,5 @@ load("@com_github_scionproto_scion//rules_openapi:defs.bzl", "openapi_bundle") +load("//tools/lint:write_source_files.bzl", "write_source_files") openapi_bundle( name = "control", @@ -100,3 +101,18 @@ openapi_bundle( entrypoint = "//spec/health:spec.yml", visibility = ["//visibility:public"], ) + +write_source_files( + name = "write_files", + files = { + "control.gen.yml": ":control", + "ca.gen.yml": ":ca", + "dispatcher.gen.yml": ":dispatcher", + "daemon.gen.yml": ":daemon", + "gateway.gen.yml": ":gateway", + "router.gen.yml": ":router", + "segments.gen.yml": ":segments", + "cppki.gen.yml": ":cppki", + "health.gen.yml": ":health", + }, +) diff --git a/tools/lint/go_embed.bzl b/tools/lint/go_embed.bzl index 98e10f8bda..9e47b4ebba 100644 --- a/tools/lint/go_embed.bzl +++ b/tools/lint/go_embed.bzl @@ -1,5 +1,4 @@ load("@io_bazel_rules_go//go:def.bzl", _go_embed_data = "go_embed_data") -load("@cgrindel_bazel_starlib//updatesrc:defs.bzl", "updatesrc_update") load(":go_fmt.bzl", _go_fmt = "go_fmt") def go_embed_data( @@ -22,9 +21,3 @@ def go_embed_data( name = fmt_name, src = ":" + name, ) - - updatesrc_update( - name = name + "-update", - srcs = [out_src], - outs = [":" + fmt_name], - )