maintainer-feedback requested: [Bug 280275] ports-mgmt/pkg: Wrong exit code in periodic script 490.status-pkg-changes

Bugzilla Automation <bugzilla@FreeBSD.org> has asked freebsd-pkg (Nobody)
<pkg@FreeBSD.org> for maintainer-feedback:
Bug 280275: ports-mgmt/pkg: Wrong exit code in periodic script
490.status-pkg-changes
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=280275



--- Description ---
I'm using the following periodic configuration, which should notify me when
packages have changed:

daily_show_success="NO"
daily_status_pkg_changes_enable="YES"

The script /usr/local/etc/periodic/daily/490.status-pkg-changes is run, but due
to a bug in the list_pkgs function, the return code is not set correctly.

Here's the function:

11	list_pkgs() {
12	    local pkgargs="$1"
13	    local bak_file="$2"
14	    local rc
15
16	    bak=/var/backups
17
18	    [ -r $bak/$bak_file ] && mv -f $bak/$bak_file $bak/${bak_file}2
19	    ${pkgcmd} ${pkgargs} info > $bak/$bak_file
20	    rc=$?
21
22	    cmp -sz $bak/$bak_file $bak/${bak_file}2
23	    if [ $? -eq 1 ]; then
24		diff -U 0 $bak/${bak_file}2 $bak/${bak_file} | \
25		    grep '^[-+][^-+]' | sort -k 1.2
26	    fi
27
28	    return $rc
29	}


On line 19, the return code, $rc, is set to the exit code of "pkg info", which
is usually 0 (success), whether packages have changed or not.

The comparison on line 22 however doesn't change $rc, even though it should, to
signal that packages have changed.

Otherwise, the overall exit code of the periodic script is 0 and won't get
picked up when $daily_show_success is disabled.

Additionally, although not responsible for the specific problem: In
list_pkgs_all() on line 51 and in each following loop, $rc is explicitly set to
1 after invocation of list_pkgs(). $rc being overwritten there multiple times
is certainly not correct.

Of course, all of this builds on the assumption that 1) no package changes =
success 2) package changes = info.

So, to fix both of these issues, what about this diff? I also attached it with
hopefully correct formatting.

Kind regards,
Andre

--- /usr/local/etc/periodic/daily/490.status-pkg-changes	2024-05-23
03:05:49.000000000 +0200
+++ 490.status-pkg-changes	2024-07-14 14:50:22.429206000 +0200
@@ -20,7 +20,8 @@
     rc=$?

     cmp -sz $bak/$bak_file $bak/${bak_file}2
-    if [ $? -eq 1 ]; then
+    rc=$?
+    if [ $rc -eq 1 ]; then
	diff -U 0 $bak/${bak_file}2 $bak/${bak_file} | \
	    grep '^[-+][^-+]' | sort -k 1.2
     fi
@@ -32,6 +33,7 @@
     local rc
     local jails
     local bak_file
+    local list_rc
     local rc=0

     : ${daily_status_pkg_changes_chroots=$pkg_chroots}
@@ -48,7 +50,8 @@
     fi

     list_pkgs '' pkg.bak
-    [ $? -ne 0 ] && rc=1
+    list_rc=$?
+    [ $list_rc -gt $rc ] && rc=$list_rc

     for c in $daily_status_pkg_changes_chroots ; do
	echo
@@ -56,7 +59,8 @@
	bak_file="pkg.chroot-$(echo $c | tr -C a-zA-Z0-9.- _).bak"

	list_pkgs "-c $c" $bak_file
-	[ $? -ne 0 ] && rc=1
+	list_rc=$?
+	[ $list_rc -gt $rc ] && rc=$list_rc
     done

     case $daily_status_pkg_changes_jails in
@@ -77,7 +81,8 @@
	bak_file="pkg.jail-$(echo $j | tr -C a-zA-Z0-9.- _).bak"

	list_pkgs "-j $j" $bak_file
-	[ $? -ne 0 ] && rc=1
+	list_rc=$?
+	[ $list_rc -gt $rc ] && rc=$list_rc
     done

     return $rc