packaging: Take the version tag for file names from the command line.

At the request of the reviewers, no-longer auto-generate a version tag for file
names. This is to avoid the need for a bazel wrapper script or other acrobatics.
It happens that we also rarely need the versionned names; normally only when
building publishables from the CI pipeline; so an extra argument on the
command line isn't inconvenient.

We now rely on the pkg_deb support for configurable file names and have added
similar support in openwrt packaging rules. versioning.bzl, tools/bazel, and
the file copies in the Makefile have been replaced by a simple symlink of all
assets to a convenient location.

In the absence of directives, the version tag is "dev".
The version strings embeded in packages and binaries is always generated, as
before.

.bazelrc

@@ -16,6 +16,7 @@ build --java_runtime_version=remotejdk_11
 build --incompatible_default_to_explicit_init_py
 # Enable resolution of cc toolchain by go toolchain
 build --incompatible_enable_cc_toolchain_resolution
+build --flag_alias=file_name_version=//:file_name_version
 
 # include one of "--define gotags=sqlite_mattn" or "--define gotags=sqlite_modernc"
 # cannot be in common, because query chokes on it.

.buildkite/pipeline.yml

@@ -25,12 +25,16 @@ steps:
   - wait
   - label: "Package :debian:"
     command:
+      - make git-version
       - make dist-deb
       - cd deb;
       - tar -chaf scion-deb-amd64.tar.gz *_amd64.deb
       - tar -chaf scion-deb-arm64.tar.gz *_arm64.deb
       - tar -chaf scion-deb-i386.tar.gz *_i386.deb
       - tar -chaf scion-deb-armel.tar.gz *_armel.deb
+      - make dist-openwrt BFLAGS="--file_name_version=$(cat bazel/bin/dist/git-version)"
+      - cd openwrt
+      - tar -chaf scion-openwrt-x86_64.tar.gz *_x86_64.ipk
     artifact_paths:
       - "deb/*.tar.gz"
     plugins:

.gitignore

@@ -39,7 +39,6 @@ __pycache__
 .installstamp
 *.d
 compile_commands.json
-versioning.bzl
 
 # Environment files
 #########################

BUILD.bazel

@@ -1,4 +1,5 @@
 load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
+load("@bazel_skylib//rules:common_settings.bzl", "string_flag")
 load("@bazel_gazelle//:def.bzl", "gazelle")
 load("@rules_pkg//:pkg.bzl", "pkg_tar")
 load("@io_bazel_rules_go//go:def.bzl", "nogo")
@@ -156,6 +157,17 @@ flake8_lint_config(
     ],
 )
 
+# Optional version string to produce versioned file names. End deliverables, such as installable
+# packages will have a name derived from that string.
+# The flag is to be used when producing publishable assets (so, typically by the CI build).
+# The rest of the time the assets will have an unversioned name. The version tags embedded
+# in binaries and package manifests are always set. Regardless of this flag.
+string_flag(
+    name = "file_name_version",
+    build_setting_default = "dev",
+    visibility = ["//visibility:public"],
+)
+
 # Add a build flag to enable bundling the management API documentation with the
 # binaries. This can be enabled by passing --//:mgmtapi_bundle_doc=true when
 # invoking bazel

Makefile

@@ -1,4 +1,4 @@
-.PHONY: all build build-dev dist-deb antlr clean docker-images gazelle go.mod licenses mocks protobuf scion-topo test test-integration write_all_source_files
+.PHONY: all build build-dev dist-deb antlr clean docker-images gazelle go.mod licenses mocks protobuf scion-topo test test-integration write_all_source_files git-version
 
 build-dev:
 	rm -f bin/*
@@ -12,33 +12,29 @@ build:
 	tar -kxf bazel-bin/scion.tar -C bin
 
 dist-deb:
-	bazel build //dist:deb_all
+	bazel build //dist:deb_all $(BFLAGS)
 	mkdir -p deb; rm -rf deb/*
-	@ # Bazel cannot include the version in the filename, if we want to set it automatically from the git tag.
-	@ # Extract the version from the .deb "control" manifest and expand the "__" in the filename to "_<version>_".
-	@ #   See e.g. https://en.wikipedia.org/wiki/Deb_(file_format)#Control_archive
-	@for f in `bazel cquery //dist:deb_all --output=files 2>/dev/null`; do \
-		if [ -f "$$f" ]; then \
-			bf=`basename $$f`; \
-			v="$$(ar p $$f control.tar.gz | tar -xz --to-stdout ./control | sed -n 's/Version: //p')"; \
-			bfv=$${bf%%__*}_$${v}_$${bf#*__}; \
-			cp -v "$$f" deb/$$bfv; \
-		fi \
-	done
+	@ # These artefacts have unique names but varied locations. Link them somewhere convenient.
+	@ mkdir -p deb
+	@ cd deb ; ln -sfv ../bazel-out/*/bin/dist/*.deb .
 
 dist-openwrt:
-	bazel build //dist:openwrt_all
-	mkdir -p openwrt; rm -rf openwrt/*
-	@ for f in `bazel cquery //dist:openwrt_all --output=files 2>/dev/null`; do \
-		cp -v "$$f" openwrt; \
-	done
+	bazel build //dist:openwrt_all $(BFLAGS)
+	@ # These artefacts have unique names but varied locations. Link them somewhere convenient.
+	@ mkdir -p openwrt
+	@ cd openwrt ; ln -sfv ../bazel-out/*/bin/dist/*.ipk .
 
 dist-openwrt-testing:
-	bazel build //dist:openwrt_testing_all
-	mkdir -p openwrt-testing; rm -rf openwrt-testing/*
-	@ for f in `bazel cquery //dist:openwrt_testing_all --output=files 2>/dev/null`; do \
-		cp -v "$$f" openwrt-testing; \
-	done
+	bazel build //dist:openwrt_testing_all $(BFLAGS)
+	@ # These artefacts have unique names but varied locations. Link them somewhere convenient.
+	@ mkdir -p openwrt
+	@ cd openwrt ; ln -sfv ../bazel-out/*/bin/dist/*.ipk .
+
+# Git-version, the data file. Not the tool. (We remove the leading 'v').
+# This is helpful to invoke make with the proper file_name_version string:
+# $(cat bazel-bin/dist/git-version)
+git-version:
+	bazel build //dist:git_version
 
 # all: performs the code-generation steps and then builds; the generated code
 # is git controlled, and therefore this is only necessary when changing the
@@ -55,6 +51,7 @@ clean:
 scrub:
 	bazel clean --expunge
 	rm -f bin/*
+	rm -f openwrt/*
 
 test:
 	bazel test --config=unit_all

dist/openwrt/ipk.bzl

@@ -1,16 +1,16 @@
-load("@@//:versioning.bzl", "STRIPPED_GIT_VERSION")
-
 # This build file is layered onto the openwrt build tree which is
 # imported as an external dependency.
 # When reading, remember that:
 # * This used in the context of external/openwrt_<target>_SDK/.
 # * The "command" script is *not* sandboxed.
 
+load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
+
 # All this replicates the standard openwrt recipe to add and build a new package (one which
 # code is already compiled). Two hacks in-there:
 #
 # * Because the openwrt src tree is full of dangling or circular symlinks, we can't run the
-#   packaging setps in a sandbox (bazel can't clone the tree); we have to do it in-place.
+#   packaging steps in a sandbox (bazel can't clone the tree); we have to do it in-place.
 # * Because we aren't sandboxed and the Makefiles aren't re-entrant, we can't build more than one
 #   package at a time. To achieve that we serialize the packaging of each component by arbitrarily
 #   depending on the previous one. For this to work, we have to declare the deps as inputs
@@ -25,15 +25,18 @@ def _ipk_impl(ctx):
     in_configs = ctx.files.configs
     in_configsroot = ctx.file.configsroot
     sdk_feeds_file = ctx.file._sdk_feeds_file
-    tag, count, commit = STRIPPED_GIT_VERSION.split("-", 2)
-    dirty = "dirty" if "-" in commit else ""
-    fileversion = (tag + "-" + count + "-dirty") if dirty else (tag + "-" + count)
+    version_file = ctx.file.version_file
+    file_name_version_str = ctx.attr.file_name_version[BuildSettingInfo].value
+
+    # Figure the version string for the file name.
+    tag, count, commit, dirty = (file_name_version_str.split("-") + ["", "", ""])[:4]
+    vers_name = (tag + "-" + count + "-dirty") if dirty else ((tag + "-" + count) if count else tag)
 
     out_file = ctx.actions.declare_file(
         "bin/packages/%s/scion/%s_%s_%s.ipk" % (
             target_arch,
             pkg_name,
-            fileversion,
+            vers_name,
             target_arch,
         ),
     )
@@ -64,18 +67,16 @@ def _ipk_impl(ctx):
             "no-sandbox": "1",
             "no-cache": "1",
         },
-        inputs = in_execs + in_initds + in_configs + [makefile, sdk_feeds_file] + in_deps,
+        inputs = in_execs + in_initds + in_configs + [makefile, sdk_feeds_file, version_file] + in_deps,
         outputs = [out_file],
         progress_message = "Packaging %{input} to %{output}",
         arguments = [
-            ctx.file._sdk_feeds_file.path,
-            pkg_name,
-            makefile.path,
-            out_file.path,
-            tag,
-            count,
-            dirty,
-            target_arch,
+            sdk_feeds_file.path,  # 1
+            pkg_name,  # 2
+            makefile.path,  # 3
+            out_file.path,  # 4
+            version_file.path,  # 5
+            target_arch,  # 6
         ],
         command = "&&".join([
             r"PATH=/bin:/sbin:/usr/bin:/usr/sbin",
@@ -90,10 +91,11 @@ def _ipk_impl(ctx):
             r"scripts/feeds update scion",
             r"scripts/feeds install -a -p scion",
             r"make IB=1 defconfig",  # IB=1 bypasses various unnecessary prerequisites
-            r'pkgrel=${6}${7+"-dirty$(date +%s)"}',
+            r"IFS='-' read tag count commit dirty < ${execroot_abspath}/$5",
+            r'pkgrel=${count}${dirty+"-dirty$(date +%s)"}',
             r"make package/feeds/scion/${2}/compile EXECROOT=${execroot_abspath}" +
-            ' PKG_VERSION="${5}" PKG_RELEASE="${pkgrel}"',
-            r"cp bin/packages/${8}/scion/${2}_${5}-${pkgrel}_${8}.ipk ${execroot_abspath}/${4}",
+             ' PKG_VERSION="${tag}" PKG_RELEASE="${pkgrel}"',
+            r"cp bin/packages/${6}/scion/${2}_${tag}-${pkgrel}_${6}.ipk ${execroot_abspath}/${4}",
         ]),
     )
 
@@ -150,13 +152,14 @@ _ipk_pkg = rule(
             mandatory = True,
             doc = "A base name for the resulting package (e.g. 'router')",
         ),
-        "version": attr.string(
-            default = "1.0",
-            doc = "A version string for the package",
+        "file_name_version": attr.label(
+            mandatory = True,
+            doc = "The version string for the package file name",
         ),
-        "release": attr.string(
-            default = "1",
-            doc = "A release number string for the package",
+        "version_file": attr.label(
+            mandatory = True,
+            allow_single_file = True,
+            doc = "A file containing the version string for the package manifest",
         ),
     },
 )
@@ -176,5 +179,7 @@ def ipk_pkg(name, **kwargs):
         name = name,
         target_arch = target_arch,
         target_compatible_with = ["@@//dist/openwrt:musl_libc"],
+        file_name_version = "@@//:file_name_version",
+        version_file = "@@//dist:git_version",
         **kwargs
     )

dist/package.bzl

@@ -1,12 +1,35 @@
 load("@rules_pkg//pkg:pkg.bzl", "pkg_deb", "pkg_tar")
-load("@@//:versioning.bzl", "STRIPPED_GIT_VERSION")
+load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
+load("@rules_pkg//pkg:providers.bzl", "PackageVariablesInfo")
 
 SCION_PKG_HOMEPAGE = "https://github.com/scionproto/scion"
 SCION_PKG_MAINTAINER = "SCION Contributors"
 SCION_PKG_LICENSE = "Apache 2.0"
 SCION_PKG_PRIORITY = "optional"
 SCION_PKG_SECTION = "net"
 
+def _name_elems_impl(ctx):
+    values = {}
+    values["file_name_version"] = ctx.attr.file_name_version[BuildSettingInfo].value
+    values["package"] = ctx.attr.package
+    values["architecture"] = ctx.attr.architecture
+    return PackageVariablesInfo(values = values)
+
+name_elems = rule(
+    implementation = _name_elems_impl,
+    attrs = {
+        "file_name_version": attr.label(
+            doc = "Placeholder for our file name version string cmd line arg.",
+        ),
+        "package": attr.string(
+            doc = "Placeholder for our file name package name string.",
+        ),
+        "architecture": attr.string(
+            doc = "Placeholder for our file name architecture string.",
+        ),
+    }
+)
+
 def scion_pkg_deb(name, executables = {}, systemds = [], configs = [], **kwargs):
     """
     The package content, the _data_ arg for the pkg_deb rule, is assembled from:
@@ -60,10 +83,20 @@ def scion_pkg_deb(name, executables = {}, systemds = [], configs = [], **kwargs)
             # "@platforms//cpu:ppc": "ppc64",
             # "@platforms//cpu:ppc64le": "ppc64le",
         })
+
+    name_elems(
+        name = "package_file_naming_" + name,
+        file_name_version = "@@//:file_name_version",
+        architecture = kwargs["architecture"],
+        package = kwargs["package"],
+    )
+
     pkg_deb(
         name = name,
         data = data,
         target_compatible_with = ["@platforms//os:linux"],
+        package_file_name ="{package}_{file_name_version}_{architecture}.deb",
+        package_variables = ":package_file_naming_" + name,
         **kwargs
     )
 
@@ -119,6 +152,37 @@ copy_file = rule(
     },
 )
 
+
+# A copy file implmentation that derives its output from its
+# (configuable) input. This is used to bring files made by an external dependency build
+# into the local build.
+def _copy_in_impl(ctx):
+    src_path = ctx.file.src.path
+    dst_name = ctx.file.src.basename
+    out_file = ctx.actions.declare_file(dst_name)
+    ctx.actions.run_shell(
+        inputs = [ctx.file.src],
+        outputs = [out_file],
+        arguments = [
+            src_path,
+            out_file.path,
+        ],
+        command = "cp -f $1 $2",
+    )
+    return DefaultInfo(files = depset([out_file]))
+
+_copy_in = rule(
+    implementation = _copy_in_impl,
+    executable = False,
+    attrs = {
+        "src": attr.label(
+            mandatory = True,
+            allow_single_file = True,
+            doc = "The label of the file to copy in."
+        ),
+    }
+)
+
 def scion_pkg_ipk(name, package, **kwargs):
     """
     The package labeled @openwrt_<target_arch>_SDK//:<name> is built and copied to
@@ -131,19 +195,15 @@ def scion_pkg_ipk(name, package, **kwargs):
     target_arch is the specific target cpu as understood by the openwrt toolchain. It is mapped
     from the cpu as is understood by bazel plaform (as in --platforms=[...]) for which we build.
     """
-    tag, count, commit = STRIPPED_GIT_VERSION.split("-", 2)
-    dirty = "dirty" if "-" in commit else ""
-    version = (tag + "-" + count + "-" + dirty) if dirty else (tag + "-" + count)
-    copy_file(
+    _copy_in(
         name = name,
+
         # The final target and file names cannot be evaluated before action time. So we have to pass
         # the entire unresolved select expression. There may be ways around this, but just as ugly.
         src = select({
             "@platforms//cpu:x86_64": "@openwrt_x86_64_SDK//:" + name,
         }),
-        out = select({
-            "@platforms//cpu:x86_64": package + "_" + version + "_x86_64.ipk",
-        }),
+
         **kwargs
     )