[OpenWrt-Devel] base-files: execute package prerm script in a subshell

Message ID 20180824002122.25770-1-luaraneda@gmail.com
State New
Headers show
Series
  • [OpenWrt-Devel] base-files: execute package prerm script in a subshell
Related show

Commit Message

Luis Araneda Aug. 24, 2018, 12:21 a.m.
This fixes a problem that's causing an early return of
default_prerm() when the package prerm script has an
exit statement at the end, which is implemented as
"exit 0" by most of the packages that use prerm

With the new behavior, the execution of default_prerm()
will continue only if the prerm script returns 0,
otherwise the function will return the error code

Additionally, this makes the execution consistent with
the postinst script

Signed-off-by: Luis Araneda <luaraneda@gmail.com>
---
Compile and run tested on ipq40xx

---
 package/base-files/files/lib/functions.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Philip Prindeville Aug. 24, 2018, 1:30 a.m. | #1
LGTM


> On Aug 23, 2018, at 6:21 PM, Luis Araneda <luaraneda@gmail.com> wrote:
> 
> This fixes a problem that's causing an early return of
> default_prerm() when the package prerm script has an
> exit statement at the end, which is implemented as
> "exit 0" by most of the packages that use prerm
> 
> With the new behavior, the execution of default_prerm()
> will continue only if the prerm script returns 0,
> otherwise the function will return the error code
> 
> Additionally, this makes the execution consistent with
> the postinst script
> 
> Signed-off-by: Luis Araneda <luaraneda@gmail.com>
> ---
> Compile and run tested on ipq40xx
> 
> ---
> package/base-files/files/lib/functions.sh | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/package/base-files/files/lib/functions.sh b/package/base-files/files/lib/functions.sh
> index 318e91856b..579aa93dca 100755
> --- a/package/base-files/files/lib/functions.sh
> +++ b/package/base-files/files/lib/functions.sh
> @@ -153,9 +153,14 @@ config_list_foreach() {
> default_prerm() {
> 	local root="${IPKG_INSTROOT}"
> 	local name
> +	local ret
> 
> 	name=$(basename ${1%.*})
> -	[ -f "$root/usr/lib/opkg/info/${name}.prerm-pkg" ] && . "$root/usr/lib/opkg/info/${name}.prerm-pkg"
> +	if [ -f "$root/usr/lib/opkg/info/${name}.prerm-pkg" ]; then
> +		( . "$root/usr/lib/opkg/info/${name}.prerm-pkg" )
> +		ret=$?
> +		[ $ret -ne 0 ] && return $ret
> +	fi
> 
> 	local shell="$(which bash)"
> 	for i in `cat "$root/usr/lib/opkg/info/${name}.list" | grep "^/etc/init.d/"`; do
> -- 
> 2.18.0
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Luis Araneda Aug. 26, 2018, 3:31 a.m. | #2
Hi,

On Thu, Aug 23, 2018 at 10:30 PM Philip Prindeville
<philipp_subx@redfish-solutions.com> wrote:
> [...]
> > On Aug 23, 2018, at 6:21 PM, Luis Araneda <luaraneda@gmail.com> wrote:
> > [...]

Someone point me to a Github pull-request [1] that already fixes this
problem, with some small differences. Especially, it continues to
execute the function if the package prerm script return with a
failure.
I'm requesting the opinion of other developers to know if we should
return or continue the execution of the default_prerm() function.

[1] https://github.com/openwrt/openwrt/pull/779

Thanks,

Luis Araneda.

Patch

diff --git a/package/base-files/files/lib/functions.sh b/package/base-files/files/lib/functions.sh
index 318e91856b..579aa93dca 100755
--- a/package/base-files/files/lib/functions.sh
+++ b/package/base-files/files/lib/functions.sh
@@ -153,9 +153,14 @@  config_list_foreach() {
 default_prerm() {
 	local root="${IPKG_INSTROOT}"
 	local name
+	local ret
 
 	name=$(basename ${1%.*})
-	[ -f "$root/usr/lib/opkg/info/${name}.prerm-pkg" ] && . "$root/usr/lib/opkg/info/${name}.prerm-pkg"
+	if [ -f "$root/usr/lib/opkg/info/${name}.prerm-pkg" ]; then
+		( . "$root/usr/lib/opkg/info/${name}.prerm-pkg" )
+		ret=$?
+		[ $ret -ne 0 ] && return $ret
+	fi
 
 	local shell="$(which bash)"
 	for i in `cat "$root/usr/lib/opkg/info/${name}.list" | grep "^/etc/init.d/"`; do