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