luci-app-rustdesk-server: fixes firmware update causes configuration loss. - #8941
luci-app-rustdesk-server: fixes firmware update causes configuration loss.#8941smallprogram wants to merge 1 commit into
Conversation
7f8c227 to
bc36bd5
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 1 new commit; the diagnosis is right — uci-defaults scripts ship in the new rootfs and re-run on the first boot after sysupgrade, so the unguarded batch was resetting enabled/enabled_relay back to 0. Comments inline on the implementation.
Generated by Claude Code
Formality Check: FailedWe checked this pull request against the contribution guidelines. Here is what needs your attention: 🛑 CRITICAL ERRORS
Tip Do not close this pull request to make corrections. Instead, modify your existing commits (e.g. Something broken? Consider reporting an issue. |
191861f to
c33e042
Compare
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 2 new commits. The touch removal and the indentation change both address the earlier inline comments, and the guard itself is correct now.
The design question from the previous review is still open, though: the shipped conffile root/etc/config/rustdesk-server already contains the rustdesk-server section with enabled '0' and enabled_relay '0', so @rustdesk-server[0] resolves on a fresh install too and the guarded batch is unreachable on every path. That thread was resolved without a reply — if the script is being kept deliberately rather than deleted, the commit message is the place to say why.
Commit checks
- c33e042 "luci-app-rustdesk-server: fixes firmware update causes config loss" — the subject and body describe the sysupgrade fix, but this commit's diff contains only the re-indentation of the here-doc body; the actual fix landed in bc36bd5. Squashing the series (see inline) makes the message match the change.
Generated by Claude Code
| set rustdesk-server.@rustdesk-server[0].enabled='0' | ||
| set rustdesk-server.@rustdesk-server[0].enabled_relay='0' | ||
| commit rustdesk-server | ||
| EOF |
There was a problem hiding this comment.
nit: the re-indent in c33e042 moved the here-doc body but left the terminator at column 0, so the block now reads as unbalanced. It still works — <<- strips leading tabs from the delimiter line as well as the body — so this is cosmetic only, but indenting it to match the uci -q batch line it closes is what the rest of the block now does.
| EOF | |
| EOF |
Generated by Claude Code
| set rustdesk-server.@rustdesk-server[0].enabled='0' | ||
| set rustdesk-server.@rustdesk-server[0].enabled_relay='0' | ||
| commit rustdesk-server | ||
| if ! uci -q get rustdesk-server.@rustdesk-server[0] >/dev/null; then |
There was a problem hiding this comment.
nit: this guard is now spread over three commits for what is a single 7-line change — bc36bd5 introduces it together with a touch line, ebd811f deletes that touch line again, and c33e042 only re-indents the here-doc body. Neither of the last two stands on its own; they are review fixups to the first.
Squashing all three into one commit would make the series bisectable and would also clear the FormalityCheck / Git & Commits failure, since the offending Update applications/... subject on ebd811f disappears with the commit.
Generated by Claude Code
|
I think if you squash the commits, and add |
This fixes the issue where configuration settings were lost during a firmware upgrade Co-authored-by: OpenWrt AI review account <openwrt-ai@hauke-m.de> Signed-off-by: David Mandy <smallprogram@foxmail.com>
c33e042 to
5c4400c
Compare
Resubmitted the PR. #8946 |
Fixes #8935
This fixes the issue where configuration settings were lost during a firmware upgrade.
Signed-off-by: David Mandy smallprogramzhusir@gmail.com
Pull request details
Description
fixes firmware update causes configuration loss.
Tested on
OpenWrt version: OpenWrt 25.12.2
LuCI version: LuCI openwrt-25.12 branch
Web browser(s): Chrome
Checklist