From e3ef6326876dc40a8bb51c37468d1204f5156f40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Gon=C3=A7alves?= Date: Mon, 7 Sep 2026 14:16:24 -0300 Subject: [PATCH] fix: preserve package lists and surface profile generation failures Handle zero-byte removal files by matching awk inputs by filename. Propagate real git commit and diff failures from Make Profiles. Validate upstream KDE sections before appending, while allowing the legitimately empty Misc section. Add 46 regression cases and a read-only push/PR test workflow. Leave generated profiles untouched. --- .github/workflows/make-profiles.yml | 14 +- .github/workflows/test.yml | 36 ++++++ build-iso/build-iso.sh | 5 +- build-iso/tests/test_engine_empty_removals.py | 40 ++++++ build-iso/tests/test_kde_sections.py | 94 ++++++++++++++ build-iso/tests/test_make_profiles_commit.py | 120 ++++++++++++++++++ sources/editions/kde/special-commands.sh | 34 +++-- 7 files changed, 333 insertions(+), 10 deletions(-) create mode 100644 .github/workflows/test.yml create mode 100644 build-iso/tests/test_engine_empty_removals.py create mode 100644 build-iso/tests/test_kde_sections.py create mode 100644 build-iso/tests/test_make_profiles_commit.py diff --git a/.github/workflows/make-profiles.yml b/.github/workflows/make-profiles.yml index 8501f279..539f2f8f 100644 --- a/.github/workflows/make-profiles.yml +++ b/.github/workflows/make-profiles.yml @@ -128,11 +128,23 @@ jobs: rm -rf "$manjaroProfiles" "$commonBase" - name: Update github + shell: bash run: | + set -euo pipefail git add --all git config --local user.email "41898282+github-actions[bot]@users.noreply.github.com" git config --local user.name "github-actions[bot]" - if [ -n "$(git commit -m "new profile" -a | grep "nothing to commit")" ];then exit 0; fi + # A clean index is a no-op; a failed commit is not. Keep the commit + # outside a conditional/pipeline so errexit propagates its failure. + if git diff --cached --quiet; then + echo "No profile changes to commit." + exit 0 + else + status=$? + # git diff returns 1 for changes, but >1 for an actual error. + [[ "$status" -eq 1 ]] || exit "$status" + fi + git commit -m "new profile" - name: Push changes uses: ad-m/github-push-action@v1 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 00000000..4b405d60 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,36 @@ +name: Test profile tooling + +on: + push: + pull_request: + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + tests: + runs-on: ubuntu-24.04 + timeout-minutes: 10 + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + + - name: Check shell syntax + shell: bash + run: | + for script in build-iso/*.sh sources/editions/*/special-commands.sh; do + bash -n "$script" + done + + - name: Run the test suite + shell: bash + run: | + python3 -m venv .venv + .venv/bin/python -m pip install pytest + .venv/bin/python -m pytest -q -ra build-iso/tests/ diff --git a/build-iso/build-iso.sh b/build-iso/build-iso.sh index 6adbf2cf..a76bbf55 100755 --- a/build-iso/build-iso.sh +++ b/build-iso/build-iso.sh @@ -617,8 +617,11 @@ apply_profile_removals() { # the post-install step can remove, or a line nobody needs any more. # -v, not a trailing assignment: BEGIN runs before argument # assignments, and an empty `out` there is a fatal awk error. + # Match the input file, not record counters: with a zero-byte + # removal list, NR==FNR also holds throughout the package file and + # would silently replace it with an empty list. awk -v out="$target.new" 'BEGIN { printf "" > out } - NR==FNR { + FILENAME == ARGV[1] { sub(/#.*/, "") gsub(/^[ \t]+|[ \t]+$/, "") if ($0 != "") drop[$0] = 1 diff --git a/build-iso/tests/test_engine_empty_removals.py b/build-iso/tests/test_engine_empty_removals.py new file mode 100644 index 00000000..e00a391e --- /dev/null +++ b/build-iso/tests/test_engine_empty_removals.py @@ -0,0 +1,40 @@ +"""An empty opt-out file must never empty an edition's package list.""" + +import subprocess + +import pytest +from conftest import SCRIPTS + + +@pytest.mark.parametrize("kind", ["Root", "Live", "Mhwd", "Desktop"]) +@pytest.mark.parametrize("shared", [False, True], ids=["regular", "symlink"]) +@pytest.mark.parametrize("removals", ["", "\n \t\n", "# nothing to remove\n"], + ids=["zero-bytes", "whitespace", "comments"]) +def test_empty_removals_preserve_packages(tmp_path, kind, shared, removals): + profile = tmp_path / "edition" + profile.mkdir() + packages = "# selected packages\n\nbase\nfirefox\nvim >extra\nlibfoo++\n" + target = profile / f"Packages-{kind}" + shared_target = tmp_path / "shared-packages" + if shared: + shared_target.write_text(packages, encoding="utf-8") + target.symlink_to(shared_target) + else: + target.write_text(packages, encoding="utf-8") + (profile / f"{kind}-remove").write_text(removals, encoding="utf-8") + + proc = subprocess.run( + ["bash", "-c", 'source "$1"; apply_profile_removals', + "test", str(SCRIPTS / "build-iso.sh")], + env={"PATH": "/usr/bin:/bin", "PROFILE_PATH_EDITION": str(profile)}, + capture_output=True, text=True, check=False, + ) + + assert proc.returncode == 0, proc.stderr + assert target.read_text(encoding="utf-8") == packages + assert not target.is_symlink() + if shared: + assert shared_target.read_text(encoding="utf-8") == packages + staged = profile / "root-overlay/var/lib/packages-remove" / f"{kind}-remove" + assert staged.read_text(encoding="utf-8") == removals + assert not target.with_name(target.name + ".new").exists() diff --git a/build-iso/tests/test_kde_sections.py b/build-iso/tests/test_kde_sections.py new file mode 100644 index 00000000..b02c376b --- /dev/null +++ b/build-iso/tests/test_kde_sections.py @@ -0,0 +1,94 @@ +"""Upstream section changes must fail before modifying the generated profile.""" + +import subprocess + +import pytest +from conftest import SCRIPTS + +SCRIPT = SCRIPTS.parent / "sources/editions/kde/special-commands.sh" +SECTIONS = { + "## Printing": "cups\n", + "## Xorg Server and Graphics": "xorg-server\n", + "## Xorg Input Drivers": "xf86-input-libinput\nxf86-input-void\n", + "## Misc": "mesa-utils\n", +} +BASE = "# BigLinux desktop\nplasma-desktop\n" + + +def run_sections(tmp_path, source): + upstream = tmp_path / "manjaro-iso-profiles/manjaro/kde/Packages-Desktop" + upstream.parent.mkdir(parents=True) + upstream.write_text(source, encoding="utf-8") + generated = tmp_path / "biglinux/kde/Packages-Desktop" + generated.parent.mkdir(parents=True) + generated.write_text(BASE, encoding="utf-8") + proc = subprocess.run( + ["bash", str(SCRIPT)], cwd=tmp_path, env={"PATH": "/usr/bin:/bin"}, + capture_output=True, text=True, check=False, + ) + return proc, generated + + +def upstream_text(sections): + return "".join(f"{header}\n{packages}\n" for header, packages in sections.items()) + + +@pytest.mark.parametrize("final_newline", [False, True]) +def test_all_sections_are_copied_and_void_is_removed(tmp_path, final_newline): + source = upstream_text(SECTIONS) + if not final_newline: + source = source.rstrip("\n") + proc, generated = run_sections(tmp_path, source) + assert proc.returncode == 0, proc.stderr + expected = BASE + upstream_text(SECTIONS).replace("xf86-input-void\n", "") + assert generated.read_text(encoding="utf-8").rstrip("\n") == expected.rstrip("\n") + + +@pytest.mark.parametrize("header,problem", [ + (header, problem) + for header in SECTIONS + for problem in ["missing", "empty", "comments-only"] + if header != "## Misc" or problem == "missing" +]) +def test_invalid_sections_fail_without_partial_output(tmp_path, header, problem): + sections = SECTIONS.copy() + if problem == "missing": + del sections[header] + elif problem == "empty": + sections[header] = "" + else: + sections[header] = "# no packages here\n # another comment\n" + proc, generated = run_sections(tmp_path, upstream_text(sections)) + assert proc.returncode != 0 + assert header in proc.stderr + assert "Packages-Desktop" in proc.stderr + assert generated.read_text(encoding="utf-8") == BASE + + +@pytest.mark.parametrize("misc", ["", "# no miscellaneous packages\n"]) +def test_misc_may_be_empty_as_in_the_current_upstream_profile(tmp_path, misc): + sections = SECTIONS.copy() + sections["## Printing"] = ">extra manjaro-printer\n>extra gtk3-print-backends\n" + sections["## Misc"] = misc + proc, generated = run_sections(tmp_path, upstream_text(sections)) + assert proc.returncode == 0, proc.stderr + expected = BASE + upstream_text(sections).replace("xf86-input-void\n", "") + assert generated.read_text(encoding="utf-8").rstrip("\n") == expected.rstrip("\n") + + +def test_a_commented_reference_is_not_a_section_header(tmp_path): + source = upstream_text(SECTIONS).replace("## Printing\n", "# renamed ## Printing\n") + proc, generated = run_sections(tmp_path, source) + assert proc.returncode != 0 + assert generated.read_text(encoding="utf-8") == BASE + + +def test_whitespace_only_lines_end_a_section(tmp_path): + source = upstream_text(SECTIONS).replace("\n\n", "\n \t\n") + proc, generated = run_sections(tmp_path, source) + assert proc.returncode == 0, proc.stderr + lines = generated.read_text(encoding="utf-8").splitlines() + for header, packages in SECTIONS.items(): + assert lines.count(header) == 1 + for package in packages.splitlines(): + assert lines.count(package) == (0 if package == "xf86-input-void" else 1) diff --git a/build-iso/tests/test_make_profiles_commit.py b/build-iso/tests/test_make_profiles_commit.py new file mode 100644 index 00000000..c7d0a81e --- /dev/null +++ b/build-iso/tests/test_make_profiles_commit.py @@ -0,0 +1,120 @@ +"""Execute the real workflow commit step in an isolated Git repository.""" + +import os +import subprocess +import textwrap + +import pytest +from conftest import SCRIPTS + + +def commit_step(): + # Extract just this named literal run block, without adding a YAML dependency + # to the shell-script test suite. A moved/renamed step fails loudly here. + workflow = (SCRIPTS.parent / ".github/workflows/make-profiles.yml").read_text( + encoding="utf-8" + ) + step = workflow.split(" - name: Update github\n", 1)[1] + step = step.split("\n - name:", 1)[0] + return textwrap.dedent(step.split(" run: |\n", 1)[1]) + + +def git(repo, env, *args): + return subprocess.run( + ["git", "-C", str(repo), *args], env=env, + capture_output=True, text=True, check=True, + ).stdout.strip() + + +@pytest.fixture +def repository(exec_tmp_path): + repo = exec_tmp_path / "repo" + repo.mkdir() + env = { + "PATH": "/usr/bin:/bin", "HOME": str(exec_tmp_path), "LC_ALL": "C", + "GIT_CONFIG_NOSYSTEM": "1", "GIT_CONFIG_GLOBAL": os.devnull, + } + git(repo, env, "init", "-q") + git(repo, env, "config", "user.name", "Regression test") + git(repo, env, "config", "user.email", "test@example.invalid") + git(repo, env, "config", "commit.gpgsign", "false") + git(repo, env, "config", "core.hooksPath", str(repo / ".git/hooks")) + (repo / "Packages-Root").write_text("base\n", encoding="utf-8") + git(repo, env, "add", "--all") + git(repo, env, "commit", "-qm", "initial") + return repo, env + + +def run_step(repo, env): + return subprocess.run( + ["bash", "--noprofile", "--norc", "-e", "-o", "pipefail", "-c", commit_step()], + cwd=repo, env=env, capture_output=True, text=True, check=False, + ) + + +def test_no_changes_is_a_successful_noop(repository): + repo, env = repository + before = git(repo, env, "rev-parse", "HEAD") + proc = run_step(repo, env) + assert proc.returncode == 0, proc.stderr + assert git(repo, env, "rev-parse", "HEAD") == before + assert git(repo, env, "status", "--porcelain") == "" + + +@pytest.mark.parametrize("change", ["modify", "add", "delete"]) +def test_generated_changes_are_committed(repository, change): + repo, env = repository + before = git(repo, env, "rev-parse", "HEAD") + if change == "modify": + (repo / "Packages-Root").write_text("base\nfirefox\n", encoding="utf-8") + elif change == "add": + (repo / "Packages-Desktop").write_text("plasma-desktop\n", encoding="utf-8") + else: + (repo / "Packages-Root").unlink() + + proc = run_step(repo, env) + + assert proc.returncode == 0, proc.stderr + assert git(repo, env, "rev-parse", "HEAD") != before + assert git(repo, env, "log", "-1", "--format=%s") == "new profile" + assert git(repo, env, "status", "--porcelain") == "" + + +def test_a_real_commit_failure_is_not_reported_as_success(repository): + repo, env = repository + before = git(repo, env, "rev-parse", "HEAD") + (repo / "Packages-Root").write_text("base\nfirefox\n", encoding="utf-8") + hook = repo / ".git/hooks/pre-commit" + hook.write_text( + "#!/bin/sh\necho 'intentional pre-commit failure' >&2\nexit 23\n", + encoding="utf-8", + ) + hook.chmod(0o755) + + proc = run_step(repo, env) + + assert "intentional pre-commit failure" in proc.stderr + assert proc.returncode != 0 + assert git(repo, env, "rev-parse", "HEAD") == before + assert git(repo, env, "diff", "--cached", "--name-only") == "Packages-Root" + + +def test_a_diff_error_is_not_treated_as_a_change(repository): + repo, env = repository + before = git(repo, env, "rev-parse", "HEAD") + (repo / "Packages-Root").write_text("base\nfirefox\n", encoding="utf-8") + bindir = repo.parent / "bin" + bindir.mkdir() + shim = bindir / "git" + shim.write_text( + '#!/bin/sh\nif [ "$1" = diff ]; then\n' + " echo 'intentional diff failure' >&2\n exit 128\nfi\n" + 'exec /usr/bin/git "$@"\n', encoding="utf-8", + ) + shim.chmod(0o755) + + proc = run_step(repo, {**env, "PATH": f"{bindir}:{env['PATH']}"}) + + assert proc.returncode == 128 + assert "intentional diff failure" in proc.stderr + assert git(repo, env, "rev-parse", "HEAD") == before diff --git a/sources/editions/kde/special-commands.sh b/sources/editions/kde/special-commands.sh index e2c3fc5a..e9c2361d 100644 --- a/sources/editions/kde/special-commands.sh +++ b/sources/editions/kde/special-commands.sh @@ -13,17 +13,35 @@ set -euo pipefail # the printing stack come from, so look here before concluding a package is # missing from Desktop-add. # -# `sed -n '/## Section/,/^$/p'` prints from the section header down to the first -# blank line, which is how upstream separates its sections. +# Sections run from an exact header down to the first blank line (including +# whitespace-only lines). Validate every section before appending any of them: +# a renamed header must not silently produce an incomplete desktop profile. upstreamDesktop=manjaro-iso-profiles/manjaro/kde/Packages-Desktop generatedDesktop=biglinux/kde/Packages-Desktop -{ - sed -n '/## Printing/,/^$/p' "$upstreamDesktop" - sed -n '/## Xorg Server and Graphics/,/^$/p' "$upstreamDesktop" - sed -n '/## Xorg Input Drivers/,/^$/p' "$upstreamDesktop" - sed -n '/## Misc/,/^$/p' "$upstreamDesktop" -} >> "$generatedDesktop" +sections=( + "## Printing" + "## Xorg Server and Graphics" + "## Xorg Input Drivers" + "## Misc" +) +sectionContents=() +for section in "${sections[@]}"; do + content=$(sed -n "/^${section}[[:space:]]*$/,/^[[:space:]]*$/p" "$upstreamDesktop") + if [[ -z "$content" ]]; then + printf 'ERROR: missing section "%s" in %s\n' "$section" "$upstreamDesktop" >&2 + exit 1 + fi + # Upstream legitimately leaves Misc empty. The three essential sections + # must still carry package entries, not just a header and comments. + if [[ "$section" != "## Misc" ]] && ! grep -q '^[[:space:]]*[^#[:space:]]' <<< "$content"; then + printf 'ERROR: no packages in section "%s" in %s\n' "$section" "$upstreamDesktop" >&2 + exit 1 + fi + sectionContents+=("$content") +done +# Command substitution strips trailing newlines; restore the section separator. +printf '%s\n\n' "${sectionContents[@]}" >> "$generatedDesktop" # Came in with "## Xorg Input Drivers" above and is not wanted: the void driver # claims input devices and nothing uses it. Anchored so it cannot match a