Skip to content

Commit a58125e

Browse files
oc-tmuellerclaude
andauthored
build: drop the appstore target in favour of dist (#634)
Closes #622. The Makefile had two packaging targets. `dist` copies an explicit allowlist (`all_src`) and is what ships: `release.yml` points `artifact-glob` at `build/dist/richdocuments.tar.gz` and the reusable release workflow runs `make dist`. `appstore` rsynced the whole working tree and subtracted a hand-maintained exclude list, writing to `build/richdocuments.tar.gz`, which nothing reads. Only the denylist rotted. It now lets through 19 tracked root entries that are not part of `all_src`: .eslintrc.json .github .php-cs-fixer.dist.php .prettierrc.json .tsconfig.json .well-known AGENTS.md CHANGELOG.md CLAUDE.md CODE_OF_CONDUCT.md SECURITY.md SUPPORT.md package.json pnpm-lock.yaml pnpm-workspace.yaml sonar-project.properties src tsconfig.json vite.config.ts It also excluded `vendor/bin` and `vendor-bin` but never `vendor` or `node_modules`, so after any build it packaged those too. Five of its excludes name files that no longer exist at all (`.drone.star`, `.php_cs.dist`, `.scrutinizer.yml`, `nbproject`, `screenshots`), and it called `occ integrity:sign-app` unconditionally, so it failed outright without release certificates present. Deleting it rather than extending the exclude list: an allowlist cannot silently pick up new root files the way this one just did - `pnpm-workspace.yaml` joined the list days after the issue was filed. `activity`, `oauth2` and `customgroups` already have no such target, and the reusable release workflow now asserts that artifacts are free of development files, so an appstore-style tarball would fail that gate anyway. Removed with it, all dead once the target is gone: `project_dir`, `sign_dir`, `appstore_dir`, `source_dir`, `package_name`, and a duplicate `occ=` assignment that the later `occ=$(CURDIR)/../../occ` already overrode - the earlier one pointed at `../core/occ`, which for an app at `apps/<app>/` resolves to `apps/core/occ` and never existed. Also dropped the two references to `$(bower_deps)`, a variable no longer defined anywhere since the bower rules were removed in #623. The released artifact is unchanged, verified rather than argued: `make dist` in `owncloudci/php:8.3` produces a tarball whose 171 entries are byte-identical to the one built from the parent commit, and `make help` output is identical too. Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 90e6cd8 commit a58125e

2 files changed

Lines changed: 3 additions & 40 deletions

File tree

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ make clean
9999

100100
- **Tests need a core checkout:** `make test-php-unit` resolves PHPUnit at `../../lib/composer/bin/phpunit`, so the app must be checked out as `apps/richdocuments` inside an ownCloud Server tree. It cannot be run from a standalone clone. The same holds for `make test-js`: the scripts in `js/` expect the globals the server puts on the page, so `tests/js/karma.config.cjs` loads jQuery, jQuery UI and `OC` from the surrounding core checkout (`core/js/core.json`, `core/vendor/`, `core/js/tests/specHelper.js`). Core's node dependencies have to be installed there once (`make` in the core root), which is what creates the `core/vendor` symlink.
101101
- **JavaScript unit tests only cover `js/`:** `tests/js/` runs the classic frontend. The Vue connector in `src/` has no unit tests yet; it needs a separate vitest setup, as used by `owncloud/web-extensions`.
102-
- **`make appstore` is release-only:** it unconditionally calls `occ integrity:sign-app` and needs a signing key and certificate in `~/.owncloud/certificates/`. Use `make dist` for a local build; `make dist` skips signing when no certificate is present.
102+
- **`make dist` is the only packaging target:** it copies an explicit allowlist (`all_src`) into `build/dist/` and is what the release workflow publishes. It calls `occ integrity:sign-app` only when a signing key and certificate are present in `~/.owncloud/certificates/`, and skips signing otherwise, so a local build needs no certificates.
103103
- **WOPI dependency:** Requires a running Collabora Online server that the ownCloud server can reach, and that can reach the ownCloud server in turn.
104104
- **Dual frontend:** Has both a classic frontend (`js/`) and an ownCloud Web connector (`src/`, built with Vite into `js/web/`). Frontend changes usually need to be made in both places.
105105
- **Generated frontend bundle is committed:** regenerate `js/web/richdocuments.js` with `pnpm build` and commit the result; never hand-edit it.

‎Makefile‎

Lines changed: 2 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,13 @@ COMPOSER_BIN := $(shell command -v composer 2> /dev/null)
66
PNPM ?= npx --yes pnpm@10
77

88
app_name=richdocuments
9-
project_dir=$(CURDIR)/../$(app_name)
109
doc_files=README.md
1110
src_dirs=appinfo assets css img js l10n lib templates
1211
src_files=admin.php settings.php
1312
all_src=$(src_dirs) $(src_files) $(doc_files)
1413
build_dir=$(CURDIR)/build
1514
dist_dir=$(build_dir)/dist
16-
sign_dir=$(build_dir)/sign
17-
appstore_dir=$(build_dir)/appstore
18-
source_dir=$(build_dir)/source
19-
package_name=$(app_name)
2015
cert_dir=$(HOME)/.owncloud/certificates
21-
occ=$(CURDIR)/../core/occ
2216

2317
# composer
2418
composer_deps=vendor
@@ -27,37 +21,6 @@ acceptance_test_deps=vendor-bin/behat/vendor
2721
# node
2822
nodejs_deps=node_modules
2923

30-
appstore:
31-
mkdir -p $(sign_dir)
32-
rsync -a \
33-
--exclude=.git \
34-
--exclude=.phan \
35-
--exclude=build \
36-
--exclude=.drone.star \
37-
--exclude=.gitignore \
38-
--exclude=.php_cs.cache \
39-
--exclude=.php_cs.dist \
40-
--exclude=.scrutinizer.yml \
41-
--exclude=CONTRIBUTING.md \
42-
--exclude=composer.json \
43-
--exclude=composer.lock \
44-
--exclude=l10n/.gitkeep \
45-
--exclude=l10n/.tx \
46-
--exclude=l10n/no-php \
47-
--exclude=Makefile \
48-
--exclude=nbproject \
49-
--exclude=screenshots \
50-
--exclude=phpcs.xml \
51-
--exclude=phpstan.neon \
52-
--exclude=phpunit*xml \
53-
--exclude=tests \
54-
--exclude=vendor/bin \
55-
--exclude=vendor-bin \
56-
$(project_dir) $(sign_dir)
57-
@echo "Signing…"
58-
$(occ) integrity:sign-app --privateKey=$(cert_dir)/$(app_name).key --certificate=$(cert_dir)/$(app_name).crt --path=$(sign_dir)/$(app_name)
59-
tar -czf $(build_dir)/$(app_name).tar.gz -C $(sign_dir) $(app_name)
60-
6124
# signing
6225
occ=$(CURDIR)/../../occ
6326
private_key=$(HOME)/.owncloud/certificates/$(app_name).key
@@ -97,7 +60,7 @@ $(nodejs_deps): package.json pnpm-lock.yaml
9760
#
9861
# dist
9962
#
100-
$(dist_dir)/$(app_name): $(composer_deps) $(bower_deps)
63+
$(dist_dir)/$(app_name): $(composer_deps)
10164
rm -Rf $@; mkdir -p $@
10265
cp -R $(all_src) $@
10366
rm -Rf $@/l10n/.gitkeep
@@ -124,7 +87,7 @@ clean-build:
12487

12588
.PHONY: clean-deps
12689
clean-deps:
127-
rm -Rf $(nodejs_deps) $(bower_deps)
90+
rm -Rf $(nodejs_deps)
12891
rm -Rf vendor
12992
rm -Rf vendor-bin/**/vendor vendor-bin/**/composer.lock
13093

0 commit comments

Comments
 (0)