ci: define a proper regression strategy for Makefile release packaging #8845

Activity

1. vitormattos

   added this to the Next Major (36) milestone on Sep 29, 2026
2. github-project-automation

   added this to LibreSign Roadmapon Sep 29, 2026
3. maia-andre commented on Oct 2, 2026

   

   maia-andre

   on Oct 2, 2026

   Contributor

   More actions

   I'd like to take this one.

   I'll start with the investigation the issue asks for: how Talk, Deck, Polls and Mail exercise their Makefile targets in CI, and whether a make --dry-run/execution-based contract check inside an existing build or release job can catch the GITHUB_ACTIONS=true regression without a dedicated workflow. I'll post the comparison and a recommendation here before writing code.
4. maia-andre commented on Oct 4, 2026

   

   maia-andre

   on Oct 4, 2026

   Contributor

   More actions

   I went through Talk, Deck, Polls and Mail (default branches as of today) and through what was tried in #8841.

   How the four apps exercise their Makefiles

   App	Packaging entry point	Runs on PRs?	CI-only logic in the Makefile?
   Talk	make appstore version=… from the templated appstore-build-publish.yml	No, release only	No. Signing is gated only by the key file (occ integrity:sign-app against ../../occ)
   Deck	krankerl package, with before_cmds = ['make release']	No, release only	No
   Polls	make package from its tag-push workflows (publish_release/beta/alpha.yml) and the template	No, tags only	No (copy list in sync_list.txt)
   Mail	krankerl package	Yes: package.yml builds the real tarball on every PR and uploads it	No

   None of them tests Makefile behavior, and none needs to: their packaging is copy + tar, with signing gated only by the key file. LibreSign's appstore recipe is different: when the key exists and GITHUB_ACTIONS=true, it installs a Nextcloud instance, enables the app, downloads the binaries for both architectures and signs the setup files. That CI-only branch is where the regression lived, and there is no upstream pattern to copy for it.

   Mail's approach (package on every PR) would not catch this regression here: PRs, especially from forks, have no signing key, so the recipe never reaches the CI-only branch. Today the real path only runs in nightly-release.yml (push to stable*) and at release time, both after merge.

   make --dry-run

   The condition is evaluated by the shell, so make -n can only print the expanded recipe:

   current syntax: if [ -f …/libresign.key ] && [ "true" = "true" ]; then \
   old syntax:     if [ -f …/libresign.key ] && [ "" = "true" ]; then \
   

   It shows the difference, but asserting it means matching expanded recipe text (what the reverted tests/ci/test-release-makefile.sh did), not observing whether the setup ran. A change in quoting or in the shape of the if breaks the test without changing behavior, and a wrong condition that keeps the same text passes. checkmake has the same limit, as the issue already notes.

   Recommendation

   Give the CI-only setup its own target and test it by execution with a stubbed occ:

   1. Move the Nextcloud setup block into a target (e.g. appstore-nextcloud-setup) that appstore calls, with the server path as an overridable variable (nextcloud_dir ?= $(CURDIR)/../nextcloud). occ can already be overridden from the command line. appstore keeps its current behavior and output.

   2. A small shell test creates a temp dir with a dummy key and a fake occ that logs its arguments, then runs the target twice:

      • GITHUB_ACTIONS=true: asserts that maintenance:install and then app:enable --force libresign were invoked;
      • GITHUB_ACTIONS unset: asserts that occ was never invoked.

      No network, no build, no Nextcloud server; it runs in seconds and fails with the old ${GITHUB_ACTIONS:-} syntax, because the fake occ is never called.

   3. Expose it as make test-release-packaging, so the workflow step is a single command and the YAML stays thin, and document which behaviors are covered and where they run (in the developer docs, or next to the target if you prefer).

   The open point is where to run it. release-metadata.yml already triggers on Makefile changes and already calls make -s print-release-changelog, so it looks like the natural place, but #8841 added a step there and then reverted it ("keep release metadata workflow unchanged"). If that workflow should stay limited to the release-tool contract, which existing job would you prefer? The alternative I see is a step in an existing lint/PHP quality job.

   If this direction works for you, I'll send it as one PR (target extraction, test, docs) with no change to the packaged output.
5. vitormattos commented on Oct 8, 2026

   

   vitormattos

   on Oct 8, 2026

   MemberAuthor

   More actions

   @maia-andre Thanks for the detailed investigation and the comparison with Talk, Deck, Polls and Mail.

   After reviewing the current Makefile, our release workflows, the shared LibreCode workflow templates and their local patches, I'd like to refine the implementation direction.

   The goal of this issue is to prevent regressions like the one that broke release packaging, without turning the Makefile into a release framework or building an extensive test environment around it.

   The original problem was caused by shell-style variable expansion (${GITHUB_ACTIONS:-}) being interpreted by GNU Make before reaching the shell. The hotfix corrected the expression, but we still lack automated regression coverage for that behavior.

   The difficulty of testing the current appstore recipe comes from having package assembly, Nextcloud initialization and signing operations inside one large recipe.

   We should introduce a small testing boundary without redesigning the entire release process.

   1. Preserve the existing public interface

   These commands must remain unchanged from a consumer's perspective:

   • make appstore
   • make verify-appstore-package

   The resulting archive must remain at build/artifacts/libresign.tar.gz.

   Preserve the existing file inclusions and exclusions, changelog handling, unsigned packaging behavior, setup integrity metadata, support for both aarch64 and x86_64, signing order and final package structure.

   The official publishing workflow already uses make-signs-app: true and require-setup-signatures: true. These contracts must remain compatible.

   We do not need to introduce Krankerl or change the shared publishing action for this issue.

   2. Make the smallest useful extraction

   Keep package staging, file copying, exclusions and tarball generation in the existing Makefile.

   Extract only the Nextcloud initialization block into an internal target, for example _appstore-nextcloud-setup.

   This target should handle the behavior currently guarded by the signing key and GITHUB_ACTIONS condition:

   • Check whether initialization is required.
   • Prepare the Nextcloud data directory and app link.
   • Execute occ maintenance:install.
   • Execute the Nextcloud version check.
   • Execute occ app:enable --force libresign.
   • Propagate failures from required operations.

   The existing appstore recipe must invoke this internal target at the same point in the execution sequence.

   Make the Nextcloud location and occ command configurable where needed for testing, while preserving the existing defaults.

   Keep the signing block in appstore unless extracting a small part becomes strictly necessary. We are not trying to refactor every release operation.

   GNU Make does not provide truly private targets, but using an internal naming convention and excluding the target from make help is sufficient. Do not expose additional release stages as supported public commands.

   Prefer keeping this implementation in the Makefile rather than introducing a separate release script. This is also important for compatibility with the existing manual release recovery mechanism, which can replace the Makefile without replacing additional scripts.

   3. Add behavioral regression tests with Bats-core

   We already use Bats-core in .devcontainer/tests/, so please reuse it instead of introducing another testing framework.

   Add focused tests under tests/ci/.

   The tests should execute the real GNU Make target, with external commands stubbed where necessary.

   Required scenarios:

   1. Key present and GITHUB_ACTIONS=true: Nextcloud initialization runs, including maintenance:install and app:enable --force libresign.
   2. Key present and GITHUB_ACTIONS unset: Nextcloud initialization is skipped.
   3. Key absent and GITHUB_ACTIONS=true: Nextcloud initialization is skipped.
   4. Command failure: A failure during required Nextcloud initialization stops the operation and returns a non-zero status.
   5. Integration with appstore: Verify that the actual appstore entry point reaches the extracted initialization logic when its conditions are satisfied.

   The fifth case matters. Testing the internal target alone is insufficient because a future change could disconnect it from appstore while the isolated tests continue passing.

   Please avoid matching Makefile source strings or expanded command text. Assertions should be based on executed commands, results and observable behavior.

   The tests must not:

   • Download dependencies or access the network.
   • Install a real Nextcloud instance.
   • Require real signing keys.
   • Construct a large fake application or duplicate the package assembly logic.
   • Modify the developer's existing environment.

   Use temporary directories and minimal stubs, with cleanup after each test.

   4. Demonstrate that the original regression is detected

   Before considering the tests complete, demonstrate that temporarily restoring the original ${GITHUB_ACTIONS:-} expression causes the relevant positive test to fail.

   Then restore the corrected expression and demonstrate that the same test passes.

   This is an important acceptance criterion. We need evidence that the test actually detects the regression that motivated this issue, rather than merely passing against the new implementation.

   Do not commit the intentionally broken implementation.

   5. Validate actual packaging before merge

   We should also validate unsigned package generation on relevant pull requests.

   This should use the actual project files and build artifacts prepared by CI, not an artificial fixture containing copies of the project's directory structure.

   The packaging validation should:

   • Execute the real unsigned packaging path.
   • Verify that the resulting tarball exists and is readable.
   • Check required files, directory structure and exclusions.
   • Reuse verify-appstore-package where applicable.
   • Fail when required artifacts are missing.

   Reuse an existing CI build surface where dependencies are already installed. Do not introduce a separate dependency installation pipeline just to run this check.

   The fast Bats regression tests can run from release-metadata.yml, which already validates release-related Makefile behavior.

   Ensure the relevant CI jobs run when the Makefile, associated tests or packaging-related files change. In particular, changing only a test file must not silently skip its execution.

   Keep workflow YAML limited to invoking existing commands and collecting results. Release logic belongs in the project, not in GitHub Actions conditionals.

   6. Keep nightly and official release workflows compatible

   The generic nightly workflow is maintained in LibreCodeCoop/.github.

   Its LibreSign-specific behavior belongs exclusively in LibreSign/libresign/.github/workflows/nightly-release.yml.patch.

   Do not introduce LibreSign-specific conditions into the shared nightly template.

   We identified an existing divergence between the local nightly patch and the materialized nightly-release.yml. That reconciliation is a separate task and must not expand this PR.

   For this issue, preserve compatibility with the currently expected LibreSign release behavior:

   • The Makefile performs LibreSign-specific setup and application signing when the required key and environment are available.
   • Setup integrity metadata must be present in signed releases.
   • The final archive remains compatible with both nightly publication and official App Store releases.
   • No additional generic signing step should be introduced into the LibreSign-specific flow.

   Do not modify the nightly template, its patch or the shared release publishing action as part of this PR.

   If the implementation reveals a concrete incompatibility with those contracts, document it rather than silently changing the workflows.

   7. Keep the implementation focused

   The expected changes are limited to:

   • A small Makefile extraction for Nextcloud initialization.
   • Any minimal variable configuration needed for isolated testing.
   • Bats regression tests.
   • Integration into existing CI checks.
   • Actual unsigned package validation before merge.
   • Short developer documentation explaining the covered behavior and how to run the tests.

   Please do not:

   • Rewrite the complete appstore recipe.
   • Introduce Krankerl or another packaging framework.
   • Introduce a generic release orchestration script.
   • Create several new public Make targets.
   • Move signing or packaging logic into workflow YAML.
   • Change public Makefile entry points or artifact paths.
   • Change the existing signing implementation.
   • Refactor unrelated Makefile targets.
   • Include nightly workflow synchronization fixes in this PR.

   8. Acceptance criteria and PR evidence

   The PR should demonstrate:

   • Bats tests pass locally and in CI without network access or external downloads.
   • The original variable-expansion regression is detected.
   • appstore still invokes Nextcloud initialization when required.
   • Initialization is skipped under the existing conditions.
   • Initialization errors propagate correctly.
   • Real unsigned packaging succeeds in CI and produces the expected tarball.
   • Existing signed release behavior and artifact contracts are preserved.
   • No additional public release interface or unnecessary infrastructure is introduced.

   Please include the test results and a concise explanation of the changes in the PR description.

   Use focused, coherent commits with DCO sign-off.

   The intended outcome is a small, maintainable regression-testing boundary around the existing packaging behavior. We want to detect this class of failure before merging, without turning the Makefile or its test suite into a larger system.

   👍React with 👍1Reacted by André Maia