diff --git a/.github/workflows/container-build-and-upload.yml b/.github/workflows/container-build-and-upload.yml index 15d82ca..d52475d 100644 --- a/.github/workflows/container-build-and-upload.yml +++ b/.github/workflows/container-build-and-upload.yml @@ -48,7 +48,7 @@ jobs: run: | set -euo pipefail - source_dir="${GITHUB_WORKSPACE}/.github/dummy-package" + source_dir="${GITHUB_WORKSPACE}/test/dummy-package" output_root="${GITHUB_WORKSPACE}/build/dummy-package" for distro in noble resolute trixie; do @@ -81,6 +81,22 @@ jobs: # DEBIAN IMAGES docker push ghcr.io/${{env.QCOM_ORG_NAME}}/${{env.IMAGE_NAME}}:trixie + # Runs on its own runner (no state shared with build-deb-arm64), so it + # rebuilds the trixie image itself before exercising --extra-repo-priority. + test-extra-repo-pinning: + needs: build-deb-arm64 + permissions: + contents: read + runs-on: ubuntu-24.04-arm + steps: + - name: Checkout repository + uses: actions/checkout@v6 + with: + persist-credentials: false + + - name: Run extra-repo pin-priority regression test + run: ./test/pinning/run-pinning-test.sh + build-rpm-arm64: permissions: contents: read diff --git a/AGENTS.md b/AGENTS.md index e747e2b..38eb6ef 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,6 +61,12 @@ docker_deb_build.py -d --rebuild # Pass an additional APT repo at build time docker_deb_build.py -s -o -d \ -e "deb [arch=arm64 signed-by=/etc/apt/keyrings/qsc-deb-releases.asc] https://... main" + +# Pin an --extra-repo's priority so its package wins even when its version +# number is lower than another source's (positionally paired with -e, one +# --extra-repo-priority per -e, in the same order) +docker_deb_build.py -s -o -d \ + -e "deb [trusted=yes] https://... main" --extra-repo-priority 1001 ``` ## When Editing Dockerfiles diff --git a/README.md b/README.md index 437a97c..0634a07 100644 --- a/README.md +++ b/README.md @@ -135,6 +135,26 @@ docker_deb_build.py \ --host-tmp-dir /var/tmp/sbuild ``` +### Pinning an extra APT repo + +If a package from `--extra-repo` needs to win dependency resolution even +when a different source offers a numerically higher version (for example, a +downstream-patched package losing to a newer upstream security release), +pass `--extra-repo-priority` with an APT pin priority. It pairs positionally +with `--extra-repo`: the Nth priority applies to the Nth `--extra-repo`, so +if used it must be specified once per `--extra-repo`, in the same order. A +priority above 1000 lets APT install a package even though it means picking +a lower version number than what's otherwise available. + +```bash +docker_deb_build.py \ + --source-dir pkg-example \ + --output-dir build \ + --distro trixie \ + --extra-repo "deb [trusted=yes] https://deb.example.com/qcom trixie main" \ + --extra-repo-priority 1001 +``` + ### Docker Images To add a new suite, copy an existing suite Dockerfile in `Dockerfiles/` and adapt it for the new release. diff --git a/docker_deb_build.py b/docker_deb_build.py index 96e7aec..e3687c4 100755 --- a/docker_deb_build.py +++ b/docker_deb_build.py @@ -18,11 +18,15 @@ import platform import shutil import urllib.request +import re +import base64 import glob import grp import pwd import getpass +from urllib.parse import urlparse + from color_logger import logger # Docker image name template @@ -109,7 +113,19 @@ def parse_arguments() -> argparse.Namespace: action='append', default=[], help="Additional APT repository to include. Can be specified multiple times. Example: 'deb [arch=arm64 trusted=yes] http://pkg.qualcomm.com noble/stable main'") - + + parser.add_argument("--extra-repo-priority", + type=int, + action='append', + default=[], + help="APT pin priority (Pin-Priority) for the --extra-repo at the same position. " + "Pairs positionally with --extra-repo: the Nth priority applies to the Nth --extra-repo, " + "so if provided it must be specified once per --extra-repo, in the same order. " + "A priority above 1000 will make APT prefer that repo's package even when its version " + "number is lower than what's available elsewhere (e.g. to keep a downstream-patched " + "package from being replaced by a newer upstream security release). Example: " + "-e '...' --extra-repo-priority 1001") + parser.add_argument("-p", "--extra-package", type=str, action='append', @@ -138,6 +154,8 @@ def parse_arguments() -> argparse.Namespace: raise Exception("--run-lintian cannot be used with --rebuild mode") if args.extra_repo: raise Exception("--extra-repo cannot be used with --rebuild mode") + if args.extra_repo_priority: + raise Exception("--extra-repo-priority cannot be used with --rebuild mode") if args.extra_package: raise Exception("--extra-package cannot be used with --rebuild mode") if args.skip_gbp: @@ -153,6 +171,11 @@ def parse_arguments() -> argparse.Namespace: args.output_dir = ".." if args.distro is None: raise Exception("--distro is required in build mode (when --rebuild is not used)") + if args.extra_repo_priority and len(args.extra_repo_priority) != len(args.extra_repo): + raise Exception( + "--extra-repo-priority must be specified once per --extra-repo, in the same order " + f"(got {len(args.extra_repo)} --extra-repo but {len(args.extra_repo_priority)} --extra-repo-priority)." + ) return args def check_docker_dependencies(timeout: int = 20) -> bool: @@ -372,7 +395,71 @@ def make_source_pkg_cmd(sbuild_cmd: str) -> str: ) -def build_package_in_docker(image_name: str, source_dir: str, output_dir: str, distro: str, run_lintian: bool, extra_repo: str, extra_package: str, skip_gbp: bool, host_tmp_dir: str = None) -> bool: +def _extra_repo_host(repo_line: str) -> str: + """ + Extract the hostname from the first http(s) URL found in an --extra-repo + 'deb ...' line. Used to generate an APT pin matching that repo by origin. + """ + match = re.search(r'https?://[^\s\]]+', repo_line) + if not match: + raise Exception(f"Could not find an http(s) URL in --extra-repo value to pin: {repo_line!r}") + host = urlparse(match.group(0)).hostname + if not host: + raise Exception(f"Could not determine hostname from --extra-repo URL to pin: {match.group(0)!r}") + return host + + +def build_apt_pin_commands(extra_repo: list[str], extra_repo_priority: list[int]) -> str: + """ + Build --chroot-setup-commands options that pin each --extra-repo to its + paired --extra-repo-priority, by writing an APT preferences file into the + chroot before build-deps are resolved. + + Pinning is done by origin (hostname): APT's 'Pin: origin' matches by + hostname only and ignores the port, so this is only distinct per-host, + not per-port. + + Two --extra-repo entries on the same host can only be pinned to the same + priority: APT has no reliable way to tell them apart on hostname alone + (and neither suite nor the Release file's Origin/Label are guaranteed to + be distinctive - e.g. a repo built for the same suite as the base mirror + it's meant to override, or a generic Artifactory instance that doesn't + set a repo-specific Origin). Rather than silently picking one priority + over the other (APT itself resolves conflicting same-origin pins by + filename order, not by priority or recency - confirmed empirically), we + raise here so the conflict is visible instead of silently wrong. + + Each preferences file is written via a base64-encoded payload instead of + an inline heredoc/echo with raw quotes: this string gets embedded once + directly in a 'bash -c' command, and a second time inside a + double-quoted --git-builder="..." string for quilt+gbp packages. Raw '"' + or newlines here would break that second, double-quoted nesting; base64's + alphabet has no shell metacharacters, so it survives both layers + unescaped. + """ + host_priorities = {} + for repo, priority in zip(extra_repo, extra_repo_priority): + host = _extra_repo_host(repo) + if host in host_priorities and host_priorities[host] != priority: + raise Exception( + f"--extra-repo entries for host {host!r} request conflicting priorities " + f"({host_priorities[host]} and {priority}): APT pins by hostname only, so " + "these two repos can't be told apart and must share the same " + "--extra-repo-priority." + ) + host_priorities[host] = priority + + snippets = [] + for idx, (repo, priority) in enumerate(zip(extra_repo, extra_repo_priority)): + host = _extra_repo_host(repo) + content = f'Package: *\nPin: origin "{host}"\nPin-Priority: {priority}\n' + encoded = base64.b64encode(content.encode()).decode() + pref_path = f"/etc/apt/preferences.d/90-extra-repo-{idx}.pref" + snippets.append(f"--chroot-setup-commands='echo {encoded} | base64 -d > {pref_path}'") + return " ".join(snippets) + + +def build_package_in_docker(image_name: str, source_dir: str, output_dir: str, distro: str, run_lintian: bool, extra_repo: list[str], extra_repo_priority: list[int], extra_package: list[str], skip_gbp: bool, host_tmp_dir: str = None) -> bool: """ Build the debian package inside the given docker image. source_dir: path to the debian package source (mounted into the container) @@ -380,6 +467,7 @@ def build_package_in_docker(image_name: str, source_dir: str, output_dir: str, d distro: target distribution string (e.g. 'noble') run_lintian: whether to run lintian on the built package extra_repo: list of additional APT repositories to include + extra_repo_priority: list of APT pin priorities, paired positionally with extra_repo host_tmp_dir: host directory to bind-mount as container /tmp; if None, no host directory is mounted Returns True on success, False on failure. """ @@ -394,12 +482,13 @@ def build_package_in_docker(image_name: str, source_dir: str, output_dir: str, d # Build the gbp command # The --git-builder value is a single string passed to gbp extra_repo_option = " ".join(f"--extra-repository='{repo}'" for repo in extra_repo) if extra_repo else "" + extra_repo_pin_option = build_apt_pin_commands(extra_repo, extra_repo_priority) extra_package_option = " ".join(f"--extra-package='{pkg}'" for pkg in extra_package) if extra_package else "" lintian_option = '--no-run-lintian' if not run_lintian else "" # --no-clean-source: skip dpkg-buildpackage --clean on host (avoids build-dep check outside chroot) # --chroot-mode=unshare: force using the mmdebstrap tarball chroot path for all supported suites. # --build-dep-resolver=aptitude: use non-default resolver that will accept alternate build-dependencies (Build-Depends: new-name | old-name) - sbuild_cmd = f"sbuild --chroot-mode=unshare --build-dep-resolver=aptitude --no-clean-source --build-dir=/workspace/output --host=arm64 --build=arm64 --dist={distro} {lintian_option} {extra_repo_option} {extra_package_option}" + sbuild_cmd = f"sbuild --chroot-mode=unshare --build-dep-resolver=aptitude --no-clean-source --build-dir=/workspace/output --host=arm64 --build=arm64 --dist={distro} {lintian_option} {extra_repo_option} {extra_repo_pin_option} {extra_package_option}" # Ensure git inside the container treats the mounted checkout as safe git_safe_cmd = "git config --global --add safe.directory /workspace/src" @@ -600,6 +689,7 @@ def main() -> None: args.distro, args.run_lintian, args.extra_repo, + args.extra_repo_priority, args.extra_package, args.skip_gbp, args.host_tmp_dir, diff --git a/.github/dummy-package/README.md b/test/dummy-package/README.md similarity index 100% rename from .github/dummy-package/README.md rename to test/dummy-package/README.md diff --git a/.github/dummy-package/debian/changelog b/test/dummy-package/debian/changelog similarity index 100% rename from .github/dummy-package/debian/changelog rename to test/dummy-package/debian/changelog diff --git a/.github/dummy-package/debian/control b/test/dummy-package/debian/control similarity index 100% rename from .github/dummy-package/debian/control rename to test/dummy-package/debian/control diff --git a/.github/dummy-package/debian/copyright b/test/dummy-package/debian/copyright similarity index 100% rename from .github/dummy-package/debian/copyright rename to test/dummy-package/debian/copyright diff --git a/.github/dummy-package/debian/install b/test/dummy-package/debian/install similarity index 100% rename from .github/dummy-package/debian/install rename to test/dummy-package/debian/install diff --git a/.github/dummy-package/debian/rules b/test/dummy-package/debian/rules similarity index 100% rename from .github/dummy-package/debian/rules rename to test/dummy-package/debian/rules diff --git a/.github/dummy-package/debian/source/format b/test/dummy-package/debian/source/format similarity index 100% rename from .github/dummy-package/debian/source/format rename to test/dummy-package/debian/source/format diff --git a/.github/dummy-package/dummy.txt b/test/dummy-package/dummy.txt similarity index 100% rename from .github/dummy-package/dummy.txt rename to test/dummy-package/dummy.txt diff --git a/test/pinning/package/debian/changelog b/test/pinning/package/debian/changelog new file mode 100644 index 0000000..d54547a --- /dev/null +++ b/test/pinning/package/debian/changelog @@ -0,0 +1,5 @@ +qcom-ci-pin-test (0.1.0) unstable; urgency=medium + + * Initial CI extra-repo pin-priority regression test package. + + -- Qualcomm Linux CI Wed, 09 Sep 2026 12:00:00 -0700 diff --git a/test/pinning/package/debian/control b/test/pinning/package/debian/control new file mode 100644 index 0000000..1ed9448 --- /dev/null +++ b/test/pinning/package/debian/control @@ -0,0 +1,16 @@ +Source: qcom-ci-pin-test +Section: misc +Priority: optional +Maintainer: Qualcomm Linux CI +Build-Depends: debhelper-compat (= 13), libqcomdummy-dev +Standards-Version: 4.6.2 +Rules-Requires-Root: no + +Package: qcom-ci-pin-test +Architecture: all +Depends: ${misc:Depends} +Description: CI regression test for docker-pkg-build --extra-repo-priority + Build-Depends on a synthetic libqcomdummy-dev published at conflicting + versions by two extra repos; debian/rules fails unless the pinned + version won dependency resolution. Reproduces and verifies the fix for + qualcomm-linux/docker-pkg-build#48. diff --git a/test/pinning/package/debian/copyright b/test/pinning/package/debian/copyright new file mode 100644 index 0000000..389b547 --- /dev/null +++ b/test/pinning/package/debian/copyright @@ -0,0 +1,26 @@ +Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: +* Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. +* Redistributions in binary form must reproduce the above + copyright notice, this list of conditions and the following + disclaimer in the documentation and/or other materials provided + with the distribution. +* Neither the name of the copyright holder nor the names of its + contributors may be used to endorse or promote products derived + from this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED "AS IS" AND ANY EXPRESS OR IMPLIED +WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT +ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS +BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR +BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, +WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE +OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN +IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. \ No newline at end of file diff --git a/test/pinning/package/debian/rules b/test/pinning/package/debian/rules new file mode 100755 index 0000000..2e1a8f9 --- /dev/null +++ b/test/pinning/package/debian/rules @@ -0,0 +1,18 @@ +#!/usr/bin/make -f + +# Hardcoded rather than parametrized: docker_deb_build.py has no mechanism to +# pass env vars into the sbuild chroot, and this test package only ever needs +# to check for one specific outcome. See test/pinning/run-pinning-test.sh for +# how the two conflicting versions are published and which run is expected +# to fail vs. succeed. +EXPECTED_LIBQCOMDUMMY_VERSION = 1.0-1+qcom1 + +%: + dh $@ + +override_dh_auto_configure: + dh_auto_configure + installed_ver="$$(dpkg-query -W -f='$${Version}' libqcomdummy-dev)"; \ + echo "Resolved libqcomdummy-dev version: $$installed_ver"; \ + [ "$$installed_ver" = "$(EXPECTED_LIBQCOMDUMMY_VERSION)" ] || \ + { echo "ERROR: expected $(EXPECTED_LIBQCOMDUMMY_VERSION), got $$installed_ver"; exit 1; } diff --git a/test/pinning/package/debian/source/format b/test/pinning/package/debian/source/format new file mode 100644 index 0000000..89ae9db --- /dev/null +++ b/test/pinning/package/debian/source/format @@ -0,0 +1 @@ +3.0 (native) diff --git a/test/pinning/run-pinning-test.sh b/test/pinning/run-pinning-test.sh new file mode 100755 index 0000000..68db69b --- /dev/null +++ b/test/pinning/run-pinning-test.sh @@ -0,0 +1,131 @@ +#!/usr/bin/env bash +# +# Regression test for --extra-repo-priority (qualcomm-linux/docker-pkg-build#48). +# +# Publishes two conflicting versions of a synthetic libqcomdummy-dev package +# via two separate flat APT repos (served by two throwaway nginx containers +# on the docker default bridge network, so each gets its own IP with no +# manual network setup), then builds test/pinning/package (which +# Build-Depends on libqcomdummy-dev and fails unless the "qcom" version won +# dependency resolution): +# +# 1. Without --extra-repo-priority: expect the build to FAIL, because APT +# picks the higher-numbered "upstream" version - this proves the bug +# from #48 is real and reproducible. +# 2. With --extra-repo-priority pinning the "qcom" repo ahead of the +# "upstream" one: expect the build to SUCCEED - this proves the fix. +# +# The two repos must be reachable at genuinely different hostnames/IPs, not +# just different ports on one host: APT's `Pin: origin ""` matches by +# hostname only and ignores the port, so same-host-different-port repos +# can't be told apart by the pin mechanism --extra-repo-priority actually +# uses (confirmed empirically during development of this test). Using two +# separate containers sidesteps this for free, since each gets its own +# Docker-assigned IP on the bridge network. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)" +PIN_TEST_PACKAGE="${SCRIPT_DIR}/package" + +UPSTREAM_VERSION="1.0-2" +QCOM_VERSION="1.0-1+qcom1" +REPO_UPSTREAM_CONTAINER="pin-test-repo-upstream-$$" +REPO_QCOM_CONTAINER="pin-test-repo-qcom-$$" + +WORK_DIR="$(mktemp -d)" + +cleanup() { + docker stop "$REPO_UPSTREAM_CONTAINER" "$REPO_QCOM_CONTAINER" >/dev/null 2>&1 || true + rm -rf "$WORK_DIR" +} +trap cleanup EXIT + +echo "== Installing equivs (used to fabricate synthetic .deb packages) ==" +sudo apt-get update -qq +sudo apt-get install -y -qq equivs dpkg-dev + +build_equivs_deb() { + local version="$1" out_dir="$2" + local ctrl="${WORK_DIR}/equivs-control-${version}" + cat > "$ctrl" < +Architecture: all +Description: Synthetic dummy library for docker-pkg-build CI pin-priority test + Fabricated via equivs; used only to exercise APT version/pin resolution + for the --extra-repo-priority regression test. +EOF + (cd "$out_dir" && equivs-build "$ctrl" >"${WORK_DIR}/equivs-build-${version}.log" 2>&1) \ + || { cat "${WORK_DIR}/equivs-build-${version}.log"; exit 1; } +} + +echo "== Fabricating two conflicting libqcomdummy-dev versions ==" +REPO_UPSTREAM_DIR="${WORK_DIR}/repo-upstream" +REPO_QCOM_DIR="${WORK_DIR}/repo-qcom" +mkdir -p "$REPO_UPSTREAM_DIR" "$REPO_QCOM_DIR" + +build_equivs_deb "$UPSTREAM_VERSION" "$REPO_UPSTREAM_DIR" +build_equivs_deb "$QCOM_VERSION" "$REPO_QCOM_DIR" + +(cd "$REPO_UPSTREAM_DIR" && dpkg-scanpackages . /dev/null > Packages 2>/dev/null) +(cd "$REPO_QCOM_DIR" && dpkg-scanpackages . /dev/null > Packages 2>/dev/null) + +echo "== Serving each repo from its own throwaway container (own IP on the default bridge network) ==" +docker run -d --rm --name "$REPO_UPSTREAM_CONTAINER" \ + -v "${REPO_UPSTREAM_DIR}:/usr/share/nginx/html:ro" nginx:alpine >/dev/null +docker run -d --rm --name "$REPO_QCOM_CONTAINER" \ + -v "${REPO_QCOM_DIR}:/usr/share/nginx/html:ro" nginx:alpine >/dev/null +sleep 1 + +UPSTREAM_IP="$(docker inspect -f '{{.NetworkSettings.Networks.bridge.IPAddress}}' "$REPO_UPSTREAM_CONTAINER")" +QCOM_IP="$(docker inspect -f '{{.NetworkSettings.Networks.bridge.IPAddress}}' "$REPO_QCOM_CONTAINER")" +echo "repo-upstream at ${UPSTREAM_IP}, repo-qcom at ${QCOM_IP}" + +UPSTREAM_REPO_LINE="deb [trusted=yes] http://${UPSTREAM_IP}/ ./" +QCOM_REPO_LINE="deb [trusted=yes] http://${QCOM_IP}/ ./" + +echo "== Self-check: confirming both repos are reachable the same way sbuild will reach them ==" +for ip in "$UPSTREAM_IP" "$QCOM_IP"; do + if ! docker run --rm curlimages/curl:latest -sf --max-time 5 "http://${ip}/Packages" >/dev/null; then + echo "ERROR: test repo at http://${ip}/Packages is not reachable from a container on the default bridge network." >&2 + exit 1 + fi +done +echo "Both repos reachable." + +echo "== Rebuilding the trixie builder image ==" +"${REPO_ROOT}/docker_deb_build.py" --rebuild --distro trixie --no-update-check + +run_build() { + local out_dir="$1" + shift + mkdir -p "$out_dir" + "${REPO_ROOT}/docker_deb_build.py" \ + --source-dir "$PIN_TEST_PACKAGE" \ + --output-dir "$out_dir" \ + --distro trixie \ + --no-update-check \ + -e "$UPSTREAM_REPO_LINE" \ + -e "$QCOM_REPO_LINE" \ + "$@" +} + +echo "== Run 1/2: without --extra-repo-priority (expected to FAIL - reproduces the bug) ==" +if run_build "${WORK_DIR}/output-unpinned"; then + echo "ERROR: build unexpectedly succeeded without pinning." >&2 + echo "This means the synthetic repos aren't actually conflicting the way this test expects." >&2 + exit 1 +fi +echo "Confirmed: build failed as expected (upstream version won without pinning)." + +echo "== Run 2/2: with --extra-repo-priority pinning the qcom repo (expected to SUCCEED - verifies the fix) ==" +if ! run_build "${WORK_DIR}/output-pinned" --extra-repo-priority 500 --extra-repo-priority 1001; then + echo "ERROR: build unexpectedly failed with pinning applied." >&2 + exit 1 +fi +echo "Confirmed: build succeeded (qcom-patched version won with pinning)." + +echo "== PASS: --extra-repo-priority regression test passed =="