oph5spdtst: add package - #30529
oph5spdtst: add package#30529AzulEterno wants to merge 2 commits into
Conversation
38704f0 to
ed93798
Compare
| if [ -z "$${IPKG_INSTROOT}" ]; then | ||
| rm -f /tmp/oph5spdtst/payload.bin /tmp/oph5spdtst/payload.bin.tmp | ||
| rm -f /www/luci-static/resources/oph5spdtst/downloading \ | ||
| /www/luci-static/resources/oph5spdtst/downloading.tmp |
There was a problem hiding this comment.
Shouldn't this be in the luci-app- package?
There was a problem hiding this comment.
There will later be a luci-app-oph5spdtst package coming (Currently simply only for install framed web + Configuration plane in web ui.). The intention here is to split test payload generation logic + pure test web resource from luci web since openwrt devices might have small/slow flash memory or other types of web service. Maybe I should couple this harder with luci since it requires luci path? I can integrate this logic into luci app if you say so.
| $(INSTALL_DIR) $(1)/etc/init.d | ||
| $(INSTALL_BIN) ./files/oph5spdtst.init $(1)/etc/init.d/oph5spdtst | ||
|
|
||
| $(INSTALL_DIR) $(1)/www/luci-static/resources/oph5spdtst |
openwrt-ai
left a comment
There was a problem hiding this comment.
Commit checks
ed93798"oph5spdtst: add package" — body has blank lines inserted mid-sentence ("... payload service from the" / "immutable v0.2.0 source release."); reflow it into one paragraph.
Generated by Claude Code
| stop() { | ||
| remove_payloads | ||
| } |
There was a problem hiding this comment.
In flash mode $FILE_SRC_LINK_PATH is the real 5 MiB payload, not a symlink, and remove_payloads deletes it unconditionally — so every shutdown (STOP=10 runs the K link) and every restart wipes it and start() rewrites 5 MiB to flash on the next boot. That defeats use_flash_stored_payload and adds avoidable flash wear. Load the config in stop() and skip the flash file, or only unlink when [ -L "$FILE_SRC_LINK_PATH" ].
Generated by Claude Code
There was a problem hiding this comment.
| PKG_SOURCE:=v$(PKG_VERSION).tar.gz | ||
| PKG_SOURCE_URL:=https://github.com/AzulEterno/oph5spdtst/archive/refs/tags/ |
There was a problem hiding this comment.
v0.2.0.tar.gz is a generic filename in the shared dl/ directory and collides with any other package fetching a v<version> GitHub tag archive. The feed's convention is $(PKG_NAME)-$(PKG_VERSION).tar.gz plus the codeload URL (244 Makefiles use codeload.github.com vs 10 using archive/refs/tags); the tarball bytes are the same, so PKG_HASH stays valid.
| PKG_SOURCE:=v$(PKG_VERSION).tar.gz | |
| PKG_SOURCE_URL:=https://github.com/AzulEterno/oph5spdtst/archive/refs/tags/ | |
| PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz | |
| PKG_SOURCE_URL:=https://codeload.github.com/AzulEterno/oph5spdtst/tar.gz/v$(PKG_VERSION)? |
Generated by Claude Code
There was a problem hiding this comment.
| PKG_LICENSE:=Apache-2.0 MIT | ||
| PKG_LICENSE_FILES:=src/LICENSE src/www/luci-static/resources/oph5spdtst/License.md | ||
|
|
||
| PKG_BUILD_DIR:=$(BUILD_DIR)/$(PKG_NAME)-$(PKG_VERSION) |
There was a problem hiding this comment.
There was a problem hiding this comment.
| START=99 | ||
| STOP=10 |
There was a problem hiding this comment.
Init scripts in this feed are expected to be procd-based: add USE_PROCD=1 and use start_service() / stop_service(). Even for a one-shot that buys you service_triggers, so editing /etc/config/oph5spdtst re-runs the payload setup instead of requiring a manual restart.
Generated by Claude Code
There was a problem hiding this comment.
|
|
||
| mkdir -p "$dir" || return 1 | ||
| rm -f "$tmp" | ||
| dd if=/dev/urandom of="$tmp" bs="$FILE_SIZE" count=1 >/dev/null 2>&1 || { |
There was a problem hiding this comment.
nit: this is one 5 MiB read, and a short read from /dev/urandom still exits 0, so a truncated payload gets mv'd into place and only fails payload_is_valid on the next start. Use bs=1M count=5, or verify the size before the mv.
Generated by Claude Code
There was a problem hiding this comment.
| create_payload "$SPDTST_FILE_PATH" "$SPDTST_TMP_PATH" "$SPDTST_FILE_DIR" || return 1 | ||
| fi | ||
|
|
||
| ln -s "$SPDTST_FILE_PATH" "$FILE_SRC_LINK_PATH" |
There was a problem hiding this comment.
With uhttpd's option no_symlinks 1 the request path goes through realpath() and is then rejected by the /* check whether found path is within docroot */ test in uhttpd's file.c, so this link resolves outside /www and the download 404s. Default config is unaffected — is that a documented limitation, or should the payload live under the docroot (bind mount / real file) instead?
Generated by Claude Code
ed93798 to
7b7cf5c
Compare
Add the payload service and UCI configuration for the oph5spdtst web frontend. Web assets are installed by the separate LuCI package. Signed-off-by: Azul Eterno <jay2236419527@gmail.com>
7b7cf5c to
28b6767
Compare
| if ! payload_is_valid "$FILE_SRC_LINK_PATH"; then | ||
| create_payload "$FILE_SRC_LINK_PATH" "$FILE_SRC_TMP_PATH" "$FILE_SRC_DIR" | ||
| fi |
There was a problem hiding this comment.
create_payload's exit status is discarded, so a failed flash write leaves start_service returning 0 with no payload in place; the tmp branch at oph5spdtst.init:64 propagates it.
| if ! payload_is_valid "$FILE_SRC_LINK_PATH"; then | |
| create_payload "$FILE_SRC_LINK_PATH" "$FILE_SRC_TMP_PATH" "$FILE_SRC_DIR" | |
| fi | |
| if ! payload_is_valid "$FILE_SRC_LINK_PATH"; then | |
| create_payload "$FILE_SRC_LINK_PATH" "$FILE_SRC_TMP_PATH" "$FILE_SRC_DIR" || return 1 | |
| fi |
Generated by Claude Code
There was a problem hiding this comment.
| FILE_SIZE_MB=5 | ||
| FILE_SIZE_BYTES=5242880 |
There was a problem hiding this comment.
nit: two constants that have to be kept in sync by hand — change one and payload_is_valid rejects every payload forever, so each start rewrites the file. Derive the byte count instead.
| FILE_SIZE_MB=5 | |
| FILE_SIZE_BYTES=5242880 | |
| FILE_SIZE_MB=5 | |
| FILE_SIZE_BYTES=$((FILE_SIZE_MB * 1024 * 1024)) |
Generated by Claude Code
There was a problem hiding this comment.
| endef | ||
|
|
||
| define Package/oph5spdtst/description | ||
| Provides a payload service for the oph5spdtst web frontend. |
There was a problem hiding this comment.
nit: the PR body still states the package "installs the web assets, UCI configuration, and payload service from the immutable v0.2.0 release" — neither the web assets nor the tarball download exist in this revision. Please bring the PR description in line with this description block.
Generated by Claude Code
There was a problem hiding this comment.
Add the payload service and UCI configuration for the oph5spdtst web frontend. Web assets are installed by the separate LuCI package. Signed-off-by: Azul Eterno <jay2236419527@gmail.com>
openwrt-ai
left a comment
There was a problem hiding this comment.
Commit checks
111a660"oph5spdtst: add package" — repeats28b6767's full "add package" message but only tweaks two lines inoph5spdtst.init; squash it into28b6767so the package is added in a single commit.
Generated by Claude Code
Add oph5spdtst as an HTML5 speed-test payload service for OpenWrt. This base package installs the service and UCI configuration only; web assets are installed by the separate
luci-app-oph5spdtstpackage.Package details
Maintainer: @AzulEterno
Provides the payload service used by the oph5spdtst local HTML5 speed-test frontend.
Testing
Reviewed
CONTRIBUTING.md. This PR contains no patches.