From c28d389e0e3b6a59aa8a193b8df8da565cfe7f97 Mon Sep 17 00:00:00 2001 From: Ayaz Salikhov Date: Wed, 26 Aug 2026 14:04:58 +0000 Subject: [PATCH] build: Refactor generate.py to make packaging_config part of config (#8115) --- .github/scripts/strategy-matrix/generate.py | 101 ++++++++++++-------- .github/scripts/strategy-matrix/linux.json | 35 ++----- .github/workflows/reusable-package.yml | 4 +- package/README.md | 49 +++++----- 4 files changed, 98 insertions(+), 91 deletions(-) diff --git a/.github/scripts/strategy-matrix/generate.py b/.github/scripts/strategy-matrix/generate.py index 7a3b7a8cf5..65671dbd11 100755 --- a/.github/scripts/strategy-matrix/generate.py +++ b/.github/scripts/strategy-matrix/generate.py @@ -23,6 +23,19 @@ _SANITIZER_SUFFIX: dict[str, str] = { } +def config_name( + distro: str, + compiler: str, + build_type: str, + arch: str, + suffix: str = "", + sanitizer: str = "", +) -> str: + """Name a config. Its artifacts are named after it, so packaging reuses this.""" + parts = [s for s in [suffix, _SANITIZER_SUFFIX.get(sanitizer, "")] if s] + return "-".join([f"{distro}-{compiler}-{build_type.lower()}-{arch}", *parts]) + + def get_cmake_args(build_type: str, extra_args: str) -> str: """Get the full list of CMake arguments for a config.""" args = _BASE_CMAKE_ARGS.copy() @@ -37,17 +50,27 @@ def get_cmake_args(build_type: str, extra_args: str) -> str: # Every config must declare 'minimal'. Minimal configs form the reduced matrix -# built for pull requests by default; the full matrix adds the rest. Packaging -# configs declare it too, but packaging is gated in the workflow, not by it. +# built for pull requests by default; the full matrix adds the rest. # -# Configs may also opt into 'benchmark' to smoke-run the benchmarks. Note that -# the flag applies to every entry a config expands into, so only set it on -# configs that expand to a single combination. +# Configs may also opt into 'benchmark' to smoke-run the benchmarks, or carry a +# 'package' map to be packaged as well. Note that either applies to every entry +# a config expands into, so only set them on configs that expand to a single +# combination. + + +@dataclasses.dataclass +class PackageConfig: + """The 'package' map of a config whose binaries are also packaged.""" + + type: str # "deb" or "rpm"; has to match what the image provides + # The packaging container image: a vanilla distro image, not the nix image + # the config itself builds in. + image: str @dataclasses.dataclass class LinuxConfig: - """One entry in linux.json's 'configs' or 'package_configs' arrays.""" + """One entry in a linux.json 'configs' array.""" compiler: list[str] build_type: list[str] @@ -57,9 +80,11 @@ class LinuxConfig: sanitizers: list[str] = dataclasses.field(default_factory=list) suffix: str = "" extra_cmake_args: str = "" - # The two below are only used by package_configs entries. - image: str = "" - package_type: str = "" # "deb" or "rpm"; has to match what image provides + package: PackageConfig | None = None # set to also package this config + + def __post_init__(self) -> None: + if isinstance(self.package, dict): + self.package = PackageConfig(**self.package) @dataclasses.dataclass @@ -68,22 +93,16 @@ class LinuxFile: image_tag: str configs: dict[str, list[LinuxConfig]] # distro → configs - package_configs: dict[str, list[LinuxConfig]] # distro → packaging configs @classmethod def load(cls, path: Path) -> "LinuxFile": data = json.loads(path.read_text()) - - def parse(section: dict) -> dict[str, list[LinuxConfig]]: - return { - distro: [LinuxConfig(**c) for c in cfgs] - for distro, cfgs in section.items() - } - return cls( image_tag=data["image_tag"], - configs=parse(data["configs"]), - package_configs=parse(data.get("package_configs", {})), + configs={ + distro: [LinuxConfig(**c) for c in cfgs] + for distro, cfgs in data["configs"].items() + }, ) @@ -199,13 +218,9 @@ def expand_linux_matrix(linux: LinuxFile, minimal: bool) -> list[MatrixEntry]: effective_sanitizers, effective_archs.items(), ): - name = f"{distro}-{compiler}-{build_type.lower()}-{arch}" - suffix_parts = [ - s for s in [cfg.suffix, _SANITIZER_SUFFIX.get(sanitizer, "")] if s - ] - if suffix_parts: - name += "-" + "-".join(suffix_parts) - + name = config_name( + distro, compiler, build_type, arch, cfg.suffix, sanitizer + ) entries.append( MatrixEntry( config_name=name, @@ -225,27 +240,33 @@ def expand_linux_matrix(linux: LinuxFile, minimal: bool) -> list[MatrixEntry]: def expand_linux_packaging(linux: LinuxFile) -> list[PackagingEntry]: - """Generate the packaging matrix from a LinuxFile's package_configs section. + """Generate the packaging matrix from the configs that carry a 'package' map. - Packaging uses vanilla distro images (debian:bookworm, almalinux:9) instead of - the nix-based build images, because deb/rpm tooling (debhelper, rpm-build) - is taken from the distro's archive rather than from nixpkgs. Each config - entry carries its own 'image'. + Packaging consumes the binaries that config's build job uploaded, so the + artifact names come from the same config name, and a packaged config is one + that passes -Dvalidator_keys=ON. - The artifact names must match what the build job uploads: one artifact per - binary, each named after the build config. + Packaging itself runs in vanilla distro images (debian:trixie, almalinux:10) + instead of the nix-based build images, because deb/rpm tooling (debhelper, + rpm-build) is taken from the distro's archive rather than from nixpkgs. """ entries = [] - for distro, configs in linux.package_configs.items(): + for distro, configs in linux.configs.items(): for cfg in configs: - for compiler, build_type in itertools.product(cfg.compiler, cfg.build_type): - config_name = f"{distro}-{compiler}-{build_type.lower()}-amd64" + if cfg.package is None: + continue + for compiler, build_type, arch in itertools.product( + cfg.compiler, cfg.build_type, cfg.arch + ): + # The packaging workflow hardcodes an amd64 runner. + assert arch == "amd64", f"cannot package {distro} on {arch}" + name = config_name(distro, compiler, build_type, arch, cfg.suffix) entries.append( PackagingEntry( - xrpld_artifact_name=f"xrpld-{config_name}", - validator_keys_artifact_name=f"validator-keys-{config_name}", - image=cfg.image, - package_type=cfg.package_type, + xrpld_artifact_name=f"xrpld-{name}", + validator_keys_artifact_name=f"validator-keys-{name}", + image=cfg.package.image, + package_type=cfg.package.type, ) ) diff --git a/.github/scripts/strategy-matrix/linux.json b/.github/scripts/strategy-matrix/linux.json index 14d1c725d7..731536a748 100644 --- a/.github/scripts/strategy-matrix/linux.json +++ b/.github/scripts/strategy-matrix/linux.json @@ -71,7 +71,11 @@ "build_type": ["Release"], "arch": ["amd64"], "minimal": false, - "extra_cmake_args": "-Dvalidator_keys=ON" + "extra_cmake_args": "-Dvalidator_keys=ON", + "package": { + "type": "deb", + "image": "ghcr.io/xrplf/xrpld/packaging-debian:sha-45e4b88" + } } ], @@ -81,30 +85,11 @@ "build_type": ["Release"], "arch": ["amd64"], "minimal": false, - "extra_cmake_args": "-Dvalidator_keys=ON" - } - ] - }, - "package_configs": { - "debian": [ - { - "compiler": ["gcc"], - "build_type": ["Release"], - "arch": ["amd64"], - "minimal": false, - "image": "ghcr.io/xrplf/xrpld/packaging-debian:sha-45e4b88", - "package_type": "deb" - } - ], - - "rhel": [ - { - "compiler": ["gcc"], - "build_type": ["Release"], - "arch": ["amd64"], - "minimal": false, - "image": "ghcr.io/xrplf/xrpld/packaging-rhel:sha-45e4b88", - "package_type": "rpm" + "extra_cmake_args": "-Dvalidator_keys=ON", + "package": { + "type": "rpm", + "image": "ghcr.io/xrplf/xrpld/packaging-rhel:sha-45e4b88" + } } ] } diff --git a/.github/workflows/reusable-package.yml b/.github/workflows/reusable-package.yml index 4d1968b93c..aa9183be37 100644 --- a/.github/workflows/reusable-package.yml +++ b/.github/workflows/reusable-package.yml @@ -1,7 +1,7 @@ # Build Linux packages from the pre-built xrpld and validator-keys artifacts: # -# - one job per distro, taken from "package_configs" in linux.json -# - each entry names its container image and the format it builds there +# - one job per config that carries a "package" map in linux.json +# - that map names the container image and the format it builds there # - with 'publish: true' a job also uploads what it built # (see package/publish_pkg.py) # diff --git a/package/README.md b/package/README.md index bacd79efe5..8295a8a38e 100644 --- a/package/README.md +++ b/package/README.md @@ -23,16 +23,16 @@ package/ ## Prerequisites -Packaging targets and their container images are declared in -[`.github/scripts/strategy-matrix/linux.json`](../.github/scripts/strategy-matrix/linux.json) -under `package_configs`, one entry per distro. Today only `linux/amd64` is -emitted. Each entry pins its full container image in an `image` field; to move -to a new image, edit that field and both CI and local builds pick it up. The -entry also declares the format that image builds in a `package_type` field, -which CI passes to `build_pkg.py` as `--package-type`; the two have to stay in -step. +Packaging is declared on the build configs themselves, in +[`.github/scripts/strategy-matrix/linux.json`](../.github/scripts/strategy-matrix/linux.json): +a config that is also packaged carries a `package` map, so its binaries and its +packaging job cannot drift apart. Today only `linux/amd64` is emitted. The map +pins the full container image in `image` — edit that field to move to a new +image and both CI and local builds pick it up — and names the format that image +builds in `type`, which CI passes to `build_pkg.py` as `--package-type`; the two +have to stay in step. -| Package type | Image (`package_configs.[].image` in `linux.json`) | Tools required | +| Package type | Image (`configs.[].package.image` in `linux.json`) | Tools required | | ------------ | ---------------------------------------------------------- | --------------------------------------------------- | | RPM | `ghcr.io/xrplf/xrpld/packaging-rhel:sha-` | `rpmbuild`, `rpmsign` | | DEB | `ghcr.io/xrplf/xrpld/packaging-debian:sha-` | `dpkg-buildpackage`, debhelper with compat level 13 | @@ -50,19 +50,20 @@ To print the full packaging matrix (artifact names and images) for the current Caller workflows (`on-pr.yml`, `on-tag.yml`, `on-trigger.yml`) call `reusable-package.yml`. That workflow generates its own packaging matrix from -`package_configs` in `linux.json` (via `generate.py --packaging`) and fans out -one job per distro. Each job downloads the pre-built `xrpld` and `validator-keys` -binary artifacts and runs in that distro's container, building the format its -`package_type` declares. The packaging script derives the package version from -the downloaded binary's `xrpld --version` output; no CMake configure or build -step is needed inside the packaging job. +the configs that carry a `package` map (via `generate.py --packaging`) and fans +out one job per distro. Each job downloads the pre-built `xrpld` and +`validator-keys` binary artifacts and runs in that distro's container, building +the format `package.type` declares. The packaging script derives the package +version from the downloaded binary's `xrpld --version` output; no CMake +configure or build step is needed inside the packaging job. -The binaries come from the `debian` and `rhel` build configurations in -`linux.json`'s `configs` section, which pass `-Dvalidator_keys=ON` so that the +The binaries come from the `debian` and `rhel` build configs themselves — the +ones carrying the `package` map — which pass `-Dvalidator_keys=ON` so that the build job produces `validator-keys` next to `xrpld` and uploads it as the -`validator-keys-` artifact. The packaging entry for a distro names -both artifacts (`xrpld_artifact_name` and `validator_keys_artifact_name`), so a -packaged configuration must keep `-Dvalidator_keys=ON`. +`validator-keys-` artifact. The packaging matrix names both +artifacts (`xrpld_artifact_name` and `validator_keys_artifact_name`) after that +same config, so a packaged config must keep `-Dvalidator_keys=ON`. Those configs +are not `minimal`, so `on-pr.yml` only packages once a PR runs the full matrix. `validator-keys` is fetched from an exact commit pinned in [`cmake/XrplValidatorKeys.cmake`](../cmake/XrplValidatorKeys.cmake), so a given @@ -75,10 +76,10 @@ With `xrpld` and `validator-keys` binaries already built at `build/xrpld` and The image tag is derived from `linux.json` so you don't need to hardcode a SHA. ```bash -# From the repo root. Each distro's container image is the `image` field of its -# package_configs entry in linux.json. Example for the rpm-producing image (use -# .package_configs.debian[0].image and --package-type deb for the other one): -IMAGE=$(jq -r '.package_configs.rhel[0].image' .github/scripts/strategy-matrix/linux.json) +# From the repo root. Each distro's container image is the `package.image` field +# of its config in linux.json. Example for the rpm-producing image (use +# .configs.debian[0].package.image and --package-type deb for the other one): +IMAGE=$(jq -r '.configs.rhel[0].package.image' .github/scripts/strategy-matrix/linux.json) PKG_RELEASE=1