Fix Codex-review findings: Makefile packaging, git archive main hardcode
Independent Codex review of the app-id rename found real gaps our own
review missed:
- Makefile: app_name was derived from the checkout directory's own
basename, not the app id in appinfo/info.xml. Our actual working
checkout is still named "occweb" (matches the repo name), so
building from it would have produced an "occweb"-named/rooted
archive under the new "extended_occweb" id. Fixed by deriving
app_name from info.xml via xpath (same technique already used for
VERSION), and added an ensure-app-name-symlink step so the
Makefile's inherited "../$(app_name)" references (used throughout
the source/appstore tar exclude patterns) keep resolving correctly
regardless of what the checkout directory is actually named.
Verified end to end: `make appstore` now produces
extended_occweb.tar.gz with extended_occweb/ as its single top-level
folder, matching the App Store's stated requirement.
- Makefile: sign target had "occweb" hardcoded into both the
certificate path and the archive filename - broken independent of
the app_name fix above. Now derived from the same variables.
- Makefile: version target pushed the release tag to the "origin"
remote, which in this repo is the stale upstream
(fanategorius/occweb), not somewhere we want to push. Changed to
"fork" (dillardblom/occweb), matching how every tag has actually
been pushed this whole session.
- INSTALL-AIO.md / notes/deploy-nc01.md: the deploy snippet did
`git archive main` unconditionally, silently deploying the main
branch even if an operator had checked out a specific tag or
maintenance branch as instructed elsewhere. Changed to `git archive
HEAD`.
- README.md: the compatibility matrix's rollback advice
("git checkout v0.3.4") doesn't actually work as a rollback anymore
post-rename - that tag still declares the old occweb id, so checking
it out under an extended_occweb-named install just breaks. Added a
caveat explaining what a real rollback requires; full migration docs
are still deferred to when a test instance exists (tracked
separately, not written up here).
This commit is contained in:
+1
-1
@@ -20,7 +20,7 @@ The AIO container has no working git access to GitHub, so deploy a tarball inste
|
||||
```bash
|
||||
# 1. On your machine: clean checkout of the branch/tag you want, dev files stripped
|
||||
STAGE=$(mktemp -d)
|
||||
git archive main | tar -x -C "$STAGE"
|
||||
git archive HEAD | tar -x -C "$STAGE"
|
||||
rm -rf "$STAGE"/{tests,.travis.yml,phpunit.xml,phpunit.integration.xml,composer.json,composer.lock,Makefile}
|
||||
tar -C "$STAGE" -cf /tmp/occweb-deploy.tar .
|
||||
|
||||
|
||||
@@ -39,7 +39,7 @@
|
||||
# "build": "node node_modules/gulp-cli/bin/gulp.js"
|
||||
# },
|
||||
|
||||
app_name=$(notdir $(CURDIR))
|
||||
app_name=$(shell xpath -q -e "//info/id/text()" appinfo/info.xml)
|
||||
build_tools_directory=$(CURDIR)/build/tools
|
||||
source_build_directory=$(CURDIR)/build/artifacts/source
|
||||
source_package_name=$(source_build_directory)/$(app_name)
|
||||
@@ -110,9 +110,21 @@ dist:
|
||||
make source
|
||||
make appstore
|
||||
|
||||
# The source/appstore targets below reference ../$(app_name) throughout,
|
||||
# inherited from the standard Nextcloud app Makefile template, which
|
||||
# assumes CURDIR's own basename equals the app id. That's not true here
|
||||
# (this checkout is "occweb", the app id is "extended_occweb"), so make
|
||||
# ../$(app_name) resolve to this checkout regardless of what it's actually
|
||||
# named, rather than rewriting every exclude pattern below.
|
||||
.PHONY: ensure-app-name-symlink
|
||||
ensure-app-name-symlink:
|
||||
@if [ "$(notdir $(CURDIR))" != "$(app_name)" ]; then \
|
||||
ln -sfn $(CURDIR) $(CURDIR)/../$(app_name); \
|
||||
fi
|
||||
|
||||
# Builds the source package
|
||||
.PHONY: source
|
||||
source:
|
||||
source: ensure-app-name-symlink
|
||||
rm -rf $(source_build_directory)
|
||||
mkdir -p $(source_build_directory)
|
||||
tar \
|
||||
@@ -126,7 +138,7 @@ source:
|
||||
|
||||
# Builds the source package for the app store, ignores php and js tests
|
||||
.PHONY: appstore
|
||||
appstore:
|
||||
appstore: ensure-app-name-symlink
|
||||
rm -rf $(appstore_build_directory)
|
||||
mkdir -p $(appstore_build_directory)
|
||||
tar \
|
||||
@@ -160,7 +172,7 @@ test: composer
|
||||
|
||||
.PHONY: sign
|
||||
sign:
|
||||
@openssl dgst -sha512 -sign ~/.nextcloud/certificates/occweb.key build/artifacts/appstore/occweb.tar.gz |openssl base64
|
||||
@openssl dgst -sha512 -sign ~/.nextcloud/certificates/$(app_name).key $(appstore_package_name).tar.gz |openssl base64
|
||||
|
||||
VERSION := $(shell xpath -q -e "//info/version/text()" appinfo/info.xml)
|
||||
|
||||
@@ -172,7 +184,7 @@ show-version:
|
||||
version:
|
||||
@echo "Creating version v$(VERSION)"
|
||||
@git tag v$(VERSION)
|
||||
@git push origin v$(VERSION)
|
||||
@git push fork v$(VERSION)
|
||||
@make dist
|
||||
@echo "\nRelease Signature: \n"
|
||||
@make sign
|
||||
|
||||
@@ -150,8 +150,14 @@ was verified, so a rollback target is obvious if a newer major causes problems.
|
||||
| 0.4.1 | `v0.4.1-nc34` | 34.0.4 | ✅ Verified (nc01, 2026-09-26) - code review fixes: correct jQuery `.fail()` signature, added a missing `.fail()` handler on the initial terminal-load request |
|
||||
| 0.4.1 | `v0.4.1-nc34` | 33.0.8 | ⚠️ Same caveat as 0.3.5/0.4.0 - not re-verified, no NC33 test environment available. Also unresolved: `js/jquery.js` now loads unconditionally on every supported version, even NC30-33 where core still provides jQuery - flagged in code review, needs an NC33 environment to confirm it's harmless there. |
|
||||
|
||||
If a future major breaks something, `git checkout v0.3.4` is the last version confirmed
|
||||
working on NC33 and older.
|
||||
If a future major breaks something, `v0.3.4` is the last version confirmed working on NC33
|
||||
and older - but a plain `git checkout v0.3.4` is **not** a working rollback by itself. That
|
||||
tag predates the app id rename (it still declares `<id>occweb</id>`), so checking it out
|
||||
into an `extended_occweb`-named install just produces a broken id/directory mismatch. A
|
||||
real rollback means disabling/removing `extended_occweb` and deploying `v0.3.4` fresh under
|
||||
an `occweb`-named directory instead, same as the original install steps above. This is one
|
||||
of the items still deferred to when a proper test instance exists (see the code review
|
||||
notes in git history around 0.4.1) - not yet written up in full.
|
||||
|
||||
## TODOs:
|
||||
See [open issues](https://github.com/dillardblom/occweb/issues)
|
||||
|
||||
Reference in New Issue
Block a user