openwrt / luci

LuCI - OpenWrt Configuration Interface
Apache License 2.0
6.38k stars 2.53k forks source link

luci_statistics: opkg install causes error due to uci-defaults script #722

Closed hnyman closed 6 years ago

hnyman commented 8 years ago

Using opkg to install packages that have uci-defaults scripts may produce errors. I ran into that with luci-app-statistics when installing from *.ipk file, so I am reporting this here, but the problem is likely related to also other packages.

The error looks like failure at the new feature to execute uci-defaults already at the opkg install stage. At least I have not seen the same message before the change by @dangowrt in https://git.lede-project.org/?p=source.git;a=commit;h=9b9c78e071469313566473984fb79f54c1881c10 and https://dev.openwrt.org/changeset/49340

Error from opkg install of luci-app-statistics package in console:

root@myrouter:/tmp# opkg install luci-app-statistics_git-16.133.26819-b213573-1_all.ipk
Installing luci-app-statistics (git-16.133.26819-b213573-1) to root...
Configuring luci-app-statistics.
//usr/lib/opkg/info/luci-app-statistics.postinst: .: line 130: can't open './40_luci-statistics'
uci: Entry not found
uci: Entry not found

40_luci-statistics is the package's uci-defaults script. I tried testing with some other packages, but was unable to produce the error. So I am not quite sure if the problem is more related to Luci and postinst scripts that it generates for the luci-* packages, or to the general packages installation routines.

My guess is that the routines defined in the Luci toplevel makefile (e.g. in https://github.com/openwrt/luci/blob/master/luci.mk#L180 ) run first and remove the uci-defaults file, and then the new routine tries to run the file, but does not find it as it is already deleted by postinst-pkg. Possible? (I am not really familiar in which order the different scripts related to opkg install do run...)

Possibly the new addition should check first that the uci-defaults file still exists in the filesystem, not only in the control file list, or pipe the possible errors to stderr?

Scripts from the build system:

perus@ub1604:/$ cat build_dir/target-mips_34kc_musl-1.1.14/luci-app-statistics/ipkg-all/luci-app-statistics/CONTROL/postinst
#!/bin/sh
[ "${IPKG_NO_SCRIPT}" = "1" ] && exit 0
. ${IPKG_INSTROOT}/lib/functions.sh
default_postinst $0 $@

perus@ub1604:/$ cat build_dir/target-mips_34kc_musl-1.1.14/luci-app-statistics/ipkg-all/luci-app-statistics/CONTROL/postinst-pkg 
[ -n "${IPKG_INSTROOT}" ] || {
    (. /etc/uci-defaults/40_luci-statistics) && rm -f /etc/uci-defaults/40_luci-statistics
    exit 0
}

The scripts from the router:

root@OpenWrt:~# cat /usr/lib/opkg/info/luci-app-statistics.postinst
#!/bin/sh
[ "${IPKG_NO_SCRIPT}" = "1" ] && exit 0
. ${IPKG_INSTROOT}/lib/functions.sh
default_postinst $0 $@

root@OpenWrt:~# cat /usr/lib/opkg/info/luci-app-statistics.postinst-pkg
[ -n "${IPKG_INSTROOT}" ] || {
        (. /etc/uci-defaults/40_luci-statistics) && rm -f /etc/uci-defaults/40_luci-statistics
        exit 0
}

EDIT: I noticed actually a difference between LEDE and Openwrt. In LEDE there is only one error message, while in Openwrt trunk r49377 there are two error messages:

root@OpenWrt:/tmp# opkg install luci-app-statistics_git-16.133.26819-b213573-1_all.ipk
Upgrading luci-app-statistics on root from git-16.131.69935-650c031-1 to git-16.133.26819-b213573-1...
Configuring luci-app-statistics.
//usr/lib/opkg/info/luci-app-statistics.postinst: .: line 130: can't open './40_luci-statistics'
uci: Entry not found
uci: Entry not found
//usr/lib/opkg/info/luci-app-statistics.postinst: .: line 130: can't open './40_luci-statistics'
uci: Entry not found
uci: Entry not found
hnyman commented 8 years ago

This error has now been reported also for luci-theme-bootstrap and luci-app-ddns: http://lists.infradead.org/pipermail/lede-dev/2016-May/000872.html http://lists.infradead.org/pipermail/lede-dev/2016-May/000873.html

So I guess that this concerns all Luci apps. The same error may also pop up for other apps that have similar package-specific postinst routines to that run uci-defaults.

Possibly the new addition should check first that the uci-defaults file still exists in the filesystem, not only in the control file list, or pipe the possible errors to stderr?

jow- commented 8 years ago

I staged a fix in https://git.lede-project.org/?p=lede/jow/staging.git;a=commitdiff;h=48cd7b0b852fa85677f0417492c4f1d60df6f598

hnyman commented 8 years ago

Thanks, your fix in LEDE trunk seems to work.

hnyman commented 7 years ago

@jow- I think that something has broken opkg install again.

When I install a Luci app that has config files, e.g. luci-app-statistics, it overwrites the old config file, does not warn about that and produces two "uci: Entry not found" errors.

Installing a normal package warns correctly about the config file conflict. See below:

root@lede:/tmp# opkg install luci-app-statistics_git-16.322.70636-36e695d-1_all.ipk
Installing luci-app-statistics (git-16.322.70636-36e695d-1) to root...
Configuring luci-app-statistics.
uci: Entry not found
uci: Entry not found

root@lede:/tmp# opkg install --force-reinstall adblock
Removing package adblock from root...
Not deleting modified conffile /etc/config/adblock.
Not deleting modified conffile /etc/adblock/adblock.whitelist.
Installing adblock (1.5.3-1) to root...
Downloading http://downloads.lede-project.org/snapshots/packages/arm_cortex-a15_neon-vfpv4/packages/adblock_1.5.3-1_all.ipk.
Configuring adblock.
adblock[2236] warn : adblock installation finished successfully, 'opkg' currently locked by package installer
Collected errors:
 * resolve_conffiles: Existing conffile /etc/config/adblock is different from the conffile in the new package. The new conffile will be placed at /etc/config/adblock-opkg.
 * resolve_conffiles: Existing conffile /etc/adblock/adblock.whitelist is different from the conffile in the new package. The new conffile will be placed at /etc/adblock/adblock.whitelist-opkg.

But there may be similar uci errors for normal packages.

root@OpenWrt2:~#  opkg install bcp38
Installing bcp38 (5-1) to root...
Downloading http://downloads.lede-project.org/snapshots/packages/mips_24kc/packages/bcp38_5-1_mips_24kc.ipk.
Configuring bcp38.
uci: Entry not found
uci: Entry not found
Collected errors:
 * resolve_conffiles: Existing conffile /etc/config/bcp38 is different from the conffile in the new package. The new conffile will be placed at /etc/config/bcp38-opkg.

I have not tried to debug this, yet.

I noticed this first a few weeks ago, but I can't pinpoint any exact date.

This manifests in both ar71xx and arm15... build, so it is not about architecture.

danielfdickinson commented 7 years ago

@hnyman @jow- It appears when commit 39ff053890a1519e11def32336154bebde2519e4 was done, the conffiles were not moved to individual packages.

danielfdickinson commented 7 years ago

@hnyman I've provisionally assigned you and I to the task ; if you won't be able to work on it, let me know and I'll leave just me.

hnyman commented 7 years ago

Nice catch. I can add the four conffiles definitions that were lost in that commit, to the specific Makefiles. Hopefully that is the correct reasoning.

(I have actually noticed already earlier that a manual install of Luci statistics package seems to have overwritten the existing config, but have never really looked for the root cause.)

hnyman commented 7 years ago

It appears when commit 39ff053 was done, the conffiles were not moved to individual packages.

I added the four missing conffiles sections with https://github.com/openwrt/luci/commit/dad185e2c9b57f458fdc8701f401e6956eaaa406

That has naturally not fixed the uci: Entry not found errors.

danielfdickinson commented 7 years ago

@hnyman but at least the config shouldn't be overwritten now... The uci errors I'm wondering if are trunk related to the changes to default_postinst and/or default_preinst.

hnyman commented 7 years ago

at least the config shouldn't be overwritten now...

Yep. Thanks for digging that up.

The uci errors I'm wondering if are trunk related to the changes to default_postinst and/or default_preinst.

Likely they are, and Jow fixed that issue already once in May. Something broke it again in October/November(?).

danielfdickinson commented 7 years ago

I believe there are two offenders. In luci.mk postinst definition I'm trying this fix:

diff --git a/luci.mk b/luci.mk
index 69aecaa..218a3e9 100644
--- a/luci.mk
+++ b/luci.mk
@@ -188,8 +188,8 @@ endef

 ifneq ($(LUCI_DEFAULTS),)
 define Package/$(PKG_NAME)/postinst
-[ -n "$${IPKG_INSTROOT}" ] || {$(foreach script,$(LUCI_DEFAULTS),
-       (. /etc/uci-defaults/$(script)) && rm -f /etc/uci-defaults/$(script))
+{$(foreach script,$(LUCI_DEFAULTS),
+       (. $${IPKG_INSTROOT}/etc/uci-defaults/$(script)) && rm -f $${IPKG_INSTROOT}/etc/uci-defaults/$(script))
        exit 0
 }
 endef

There is also the following issue:

diff --git a/applications/luci-app-statistics/root/etc/uci-defaults/40_luci-statistics b/applications/luci-app-statistics/root/etc/uci-defaults/40_luci-statistics
index 28e3529..0c3dc2e 100755
--- a/applications/luci-app-statistics/root/etc/uci-defaults/40_luci-statistics
+++ b/applications/luci-app-statistics/root/etc/uci-defaults/40_luci-statistics
@@ -1,8 +1,11 @@
 #!/bin/sh

+if [ -n "$(uci get ucitrack.@luci_statistics[-1] 2>/dev/null)" ]; then
+       uci delete ucitrack.@luci_statistics[-1]
+fi
+
 # register commit handler
 uci -q batch <<-EOF >/dev/null
-       delete ucitrack.@luci_statistics[-1]
        add ucitrack luci_statistics
        set ucitrack.@luci_statistics[-1].init=luci_statistics
        commit ucitrack

basically the 'entry not found' comes from uci get and uci delete on a fresh install (because no ucitrack entry).

danielfdickinson commented 7 years ago

Another possibility is that the luci.mk essential duplicates the commits you mentions and the execution and rm -f are really redundant.

danielfdickinson commented 7 years ago

On further review the latter explanation appears to show why the luci.mk is wrong: here is a fix I'll test:

ifneq ($(LUCI_DEFAULTS),)
define Package/$(PKG_NAME)/postinst

[ -n "$${IPKG_INSTROOT}" ] || ! grep -q uci-defaults $${IPKG_INSTROOT}/etc/functions.sh && {$(foreach script,$(LUCI_DEFAULTS),
        ([ -r /etc/uci-defaults/$(script) ] && . /etc/uci-defaults/$(script)) && rm -f /etc/uci-defaults/$(script))
        exit 0  
}
endef
endif
danielfdickinson commented 7 years ago

Note that if we don't care about backward compatibility with older OpenWrt releases the whole postinst could just be dropped from luci.mk

psyborg55 commented 7 years ago

add -q option to silence error messages printed to syslog. i've just tested and it works with luci-app-mwan3

danielfdickinson commented 7 years ago

@hnyman @jow- I removed myself from OpenWrt organization before unassigning myself from Issues etc; could you remove me? (I've appreciated you two, and hope I run across you elsewhere, but I'm moving on to other projects for a whole variety reasons, not only the issues I had that were not helpful for OpenWrt/LEDE0.

jow- commented 6 years ago

I assume this is fixed by now, if not please reopen.

white-gecko commented 4 years ago

I just updated luci-mod-network and luci-theme-bootstrap through the luci interface and received:

Upgrading luci-mod-network on root from git-20.057.55219-13dd17f-1 to git-20.081.71863-12f0dac-1...
Downloading http://downloads.openwrt.org/releases/19.07.2/packages/mips_24kc/luci/luci-mod-network_git-20.081.71863-12f0dac-1_all.ipk
Removing obsolete file /usr/lib/lua/luci/view/admin_network/diagnostics.htm.
Configuring luci-mod-network.

Errors

uci: Entry not found
Upgrading luci-theme-bootstrap on root from git-20.057.55219-13dd17f-1 to git-20.081.71863-12f0dac-1...
Downloading http://downloads.openwrt.org/releases/19.07.2/packages/mips_24kc/luci/luci-theme-bootstrap_git-20.081.71863-12f0dac-1_all.ipk
Configuring luci-theme-bootstrap.

Errors

uci: Entry not found