Merge pull request #78 from bookingcom/mnaranjo/linters

Adding linters to CI

.github/workflows/linter.yaml

@@ -0,0 +1,41 @@
+name: Linters
+
+# Controls when the action will run.
+on:
+  # Triggers the workflow on push or pull request events but only for the main branch
+  push:
+    branches: [main]
+  pull_request:
+    branches: [main]
+  # Allows you to run this workflow manually from the Actions tab
+  workflow_dispatch:
+
+concurrency:
+  # Cancel previous actions from the same PR or branch except 'main' branch.
+  # See https://docs.github.com/en/actions/using-jobs/using-concurrency and https://docs.github.com/en/actions/learn-github-actions/contexts for more info.
+  group: concurrency-group::${{ github.workflow }}::${{ github.event.pull_request.number > 0 && format('pr-{0}', github.event.pull_request.number) || github.ref_name }}${{ github.ref_name == 'main' && format('::{0}', github.run_id) || ''}}
+  cancel-in-progress: ${{ github.ref_name != 'main' }}
+
+
+jobs:
+  check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: bazel-contrib/[email protected]
+        with:
+          # Avoid downloading Bazel every time.
+          bazelisk-cache: true
+          # enable a disk cache
+          disk-cache: true
+          # Share repository cache between workflows.
+          repository-cache: true
+          bazelrc: |
+            import %workspace%/.aspect/bazelrc/ci.bazelrc
+            import %workspace%/.github/workflows/ci.bazelrc
+          # keep a cache for MODULE.bazel repos
+          external-cache: true
+      - name: buildifier
+        run: bazel run //:buildifier.check
+      - name: gazelle
+        run: bazel run //:gazelle.check

BUILD.bazel

@@ -8,10 +8,26 @@ load("//bazeldnf:defs.bzl", "bazeldnf", "rpmtree", "tar2files")
 # gazelle:resolve go github.com/bazelbuild/buildtools/edit @com_github_bazelbuild_buildtools//edit:go_default_library
 gazelle(name = "gazelle")
 
+gazelle(
+    name = "gazelle.check",
+    mode = "diff",
+)
+
 buildifier(
     name = "buildifier",
 )
 
+buildifier(
+    name = "buildifier.check",
+    diff_command = "diff -u",
+    exclude_patterns = [
+        "./.git/*",
+        "./**/testdata/*",
+    ],
+    lint_mode = "warn",
+    mode = "diff",
+)
+
 bazeldnf(
     name = "bazeldnf",
 )

Makefile

@@ -11,7 +11,7 @@ test: gazelle e2e
 	bazelisk build //... && bazelisk test //...
 
 buildifier:
-	bazelisk run //:buildifier
+	bazelisk run //:buildifier.check
 
 gofmt:
 	gofmt -w pkg/.. cmd/..

WORKSPACE

@@ -46,10 +46,10 @@ http_archive(
     ],
 )
 
-load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")
 load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies")
+load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")
 load("//:build_deps.bzl", "bazeldnf_build_dependencies")
-load("//bazeldnf:deps.bzl", "bazeldnf_dependencies", "rpm")
+load("//bazeldnf:deps.bzl", "bazeldnf_dependencies")
 
 # gazelle:repository_macro build_deps.bzl%bazeldnf_build_dependencies
 bazeldnf_build_dependencies()

bazeldnf/toolchain.bzl

@@ -1,3 +1,5 @@
+"Wraps bazeldnf tool binary through a toolchain"
+
 def _bazeldnf_toolchain(ctx):
     return [
         platform_common.ToolchainInfo(

build_deps.bzl

@@ -1,9 +1,12 @@
+"Provides WORKSPACE build dependencies to build the bazeldnf binary"
+
 load(
     "@bazel_gazelle//:deps.bzl",
     _go_repository = "go_repository",
 )
 
 def bazeldnf_build_dependencies():
+    "dependencies to build the bazeldnf binary"
     _maybe(
         build_external = "external",
         name = "co_honnef_go_tools",

def.bzl

@@ -1,8 +1,11 @@
+"legacy API"
+
 load(
     "@bazeldnf//bazeldnf:defs.bzl",
     _bazeldnf = "bazeldnf",
 )
 
 def bazeldnf(*args, **kwargs):
+    # buildifier: disable=print
     print("import this method from @bazeldnf//bazeldnf:defs.bzl")
     _bazeldnf(*args, **kwargs)

deps.bzl

@@ -2,33 +2,39 @@
 
 load(
     "@bazeldnf//bazeldnf:defs.bzl",
-    _rpm = "rpm",
     _rpmtree = "rpmtree",
     _tar2files = "tar2files",
     _xattrs = "xattrs",
 )
 load(
     "@bazeldnf//bazeldnf:deps.bzl",
     _bazeldnf_dependencies = "bazeldnf_dependencies",
+    _rpm = "rpm",
 )
 
 def rpm(*args, **kwargs):
+    # buildifier: disable=print
     print("import rpm method from @bazeldnf//bazeldnf:defs.bzl")
     _rpm(*args, **kwargs)
 
 def rpmtree(*args, **kwargs):
+    # buildifier: disable=print
     print("import rpmtree method from @bazeldnf//bazeldnf:defs.bzl")
     _rpmtree(*args, **kwargs)
 
 def tar2files(*args, **kwargs):
+    # buildifier: disable=print
     print("import tar2files method from @bazeldnf//bazeldnf:defs.bzl")
     _tar2files(*args, **kwargs)
 
 def xattrs(*args, **kwargs):
+    # buildifier: disable=print
     print("import xattrs method from @bazeldnf//bazeldnf:defs.bzl")
     _xattrs(*args, **kwargs)
 
 def bazeldnf_dependencies():
     """Download bazeldnf dependencies"""
+
+    # buildifier: disable=print
     print("import bazeldnf_dependencies method from @bazeldnf//bazeldnf:deps.bzl")
     _bazeldnf_dependencies()

internal/bazeldnf.bzl

@@ -1,3 +1,5 @@
+"Provides a wrapper to run bazeldnf as a run target"
+
 load("@bazel_skylib//lib:shell.bzl", "shell")
 load("//bazeldnf:toolchain.bzl", "BAZELDNF_TOOLCHAIN")
 
@@ -6,16 +8,15 @@ def _bazeldnf_impl(ctx):
     out_file = ctx.actions.declare_file(ctx.label.name + ".bash")
     args = []
     if ctx.attr.command:
-        args += [ctx.attr.command]
+        args.append(ctx.attr.command)
     if ctx.attr.rulename:
-        args += ["--name", ctx.attr.rulename]
+        args.extend(["--name", ctx.attr.rulename])
     if ctx.attr.rpmtree:
-        args += ["--rpmtree", ctx.attr.rpmtree]
+        args.extend(["--rpmtree", ctx.attr.rpmtree])
     if ctx.file.tar:
-        args += ["--input", ctx.file.tar.path]
-        transitive_dependencies += [ctx.attr.tar.files]
-    for lib in ctx.attr.libs:
-        args += [lib]
+        args.extend(["--input", ctx.file.tar.path])
+        transitive_dependencies.extend(ctx.attr.tar.files)
+    args.extend(ctx.attr.libs)
 
     toolchain = ctx.toolchains[BAZELDNF_TOOLCHAIN]
 

internal/rpm.bzl

@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+"Exposes rpm files to a Bazel workspace"
+
 load("@bazel_tools//tools/build_defs/repo:utils.bzl", "update_attrs")
 
 _HTTP_FILE_BUILD = """
@@ -25,7 +27,6 @@ filegroup(
 def _rpm_impl(ctx):
     if ctx.attr.urls:
         downloaded_file_path = "downloaded"
-        download_path = ctx.path("rpm/" + downloaded_file_path)
         download_info = ctx.download(
             url = ctx.attr.urls,
             output = "rpm/" + downloaded_file_path,

internal/rpmtree.bzl

@@ -12,58 +12,66 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+"""Provide helpers to convert rpm files into a single tar file
+
+This file exposes rpmtree and tar2files to convert a group of
+rpm files into either a .tar or extract files from that tar to
+make available to bazel
+"""
+
 load("//bazeldnf:toolchain.bzl", "BAZELDNF_TOOLCHAIN")
 
 def _rpm2tar_impl(ctx):
-    rpms = []
-    for rpm in ctx.files.rpms:
-        rpms += ["--input", rpm.path]
+    args = ctx.actions.args()
 
     out = ctx.outputs.out
-    args = ["rpm2tar", "--output", out.path]
+    args.add_all(["rpm2tar", "--output", out])
 
     if ctx.attr.symlinks:
         symlinks = []
         for k, v in ctx.attr.symlinks.items():
-            symlinks += [k + "=" + v]
-        args += ["--symlinks", ",".join(symlinks)]
+            symlinks.append(k + "=" + v)
+        args.add_joined("--symlinks", symlinks, join_with = ",")
 
     if ctx.attr.capabilities:
         capabilities = []
         for k, v in ctx.attr.capabilities.items():
-            capabilities += [k + "=" + ":".join(v)]
-        args += ["--capabilities", ",".join(capabilities)]
+            capabilities.append(k + "=" + ":".join(v))
+        args.add_joined("--capabilities", capabilities, join_with = ",")
 
     if ctx.attr.selinux_labels:
         selinux_labels = []
         for k, v in ctx.attr.selinux_labels.items():
-            selinux_labels += [k + "=" + v]
-        args += ["--selinux-labels", ",".join(selinux_labels)]
+            selinux_labels.append(k + "=" + v)
+        args.add_joined("--selinux-labels", selinux_labels, join_with = ",")
 
-    args += rpms
+    for rpm in ctx.files.rpms:
+        args.add_all(["--input", rpm.path])
 
     ctx.actions.run(
         inputs = ctx.files.rpms,
         outputs = [out],
-        arguments = args,
+        arguments = [args],
         mnemonic = "Rpm2Tar",
         progress_message = "Converting %s to tar" % ctx.label.name,
         executable = ctx.toolchains[BAZELDNF_TOOLCHAIN]._tool,
     )
 
     return [DefaultInfo(files = depset([ctx.outputs.out]))]
 
+def _expand_path(files):
+    return [x.path for x in files]
+
 def _tar2files_impl(ctx):
-    out = ctx.outputs.out
-    files = []
-    for out in ctx.outputs.out:
-        files += [out.path]
+    args = ctx.actions.args()
+
+    args.add_all(["tar2files", "--file-prefix", ctx.attr.prefix, "--input", ctx.files.tar[0]])
+    args.add_all([ctx.outputs.out], map_each = _expand_path)
 
-    args = ["tar2files", "--file-prefix", ctx.attr.prefix, "--input", ctx.files.tar[0].path] + files
     ctx.actions.run(
         inputs = ctx.files.tar,
         outputs = ctx.outputs.out,
-        arguments = args,
+        arguments = [args],
         mnemonic = "Tar2Files",
         progress_message = "Extracting files",
         executable = ctx.toolchains[BAZELDNF_TOOLCHAIN]._tool,
@@ -97,31 +105,36 @@ _tar2files = rule(
     toolchains = [BAZELDNF_TOOLCHAIN],
 )
 
-def rpmtree(**kwargs):
-    kwargs.pop("files", None)
-    basename = kwargs["name"]
-    kwargs.pop("name", None)
-    tarname = basename + ".tar"
+def rpmtree(name, **kwargs):
+    """Creates a tar file from a list of rpm files."""
+    tarname = name + ".tar"
     _rpm2tar(
-        name = basename,
+        name = name,
         out = tarname,
         **kwargs
     )
 
-def tar2files(**kwargs):
-    files = kwargs["files"]
-    kwargs.pop("files", None)
-    basename = kwargs["name"]
-    kwargs.pop("name", None)
-    if files:
-        for k, v in files.items():
-            name = basename + k
-            files = []
-            for file in v:
-                files = files + [name + "/" + file]
-            _tar2files(
-                name = name,
-                prefix = k,
-                out = files,
-                **kwargs
-            )
+def tar2files(name, files = None, **kwargs):
+    """Extracts files from a tar file.
+
+    Args:
+        name: The name of the tar file to be processed.
+        files: A dictionary where each key-value pair represents a file to be extracted.
+               If not provided, the function will fail.
+        **kwargs: Additional keyword arguments to be passed to the _tar2files function.
+    """
+    if not files:
+        fail("files is a required attribute")
+
+    basename = name
+    for k, v in files.items():
+        name = basename + k
+        files = []
+        for file in v:
+            files.append(name + "/" + file)
+        _tar2files(
+            name = name,
+            prefix = k,
+            out = files,
+            **kwargs
+        )