On Wed, Jan 05, 2022 at 05:36:42PM -0800, Andrew Hewus Fresh wrote:
> After getting fw_update(8) into a state where it could get some testing,
> I found that the man page indicated that -v should indicate different
> levels of verbosity and I currently only had one.
>
> This was useful as I didn't really like the output anyway.
>
> Now one -v prints out an additional line when it's doing something to
> the firmware. Two add a progress bar and mentions "detecting". Three
> provide a bit more debugging mostly from ftp(1).
If you want multiple levels, I'd treat VERBOSE as an integer. Basically
use `((VERBOSE))' as "non-zero?", `((VERBOSE > 1))', etc.
I went through your patch (does not apply anymore after the other diffs
were committed) and did the mechanical conversion whilst introducing
your levels and messages.
`doas ./fw_update -n -v[[v]v]' shows different output for me, but I have
not tested it thoroughly.
> Also a couple extra small improvements, hiding errors from killing the
> ftp subprocess which could happen if it exits before we do, just
> using `firmware_devicename "$_d"` instead of `echo "${...}"`, and using
> a normal [=] comparison instead of [[=]] because we don't want pattern
> matching there.
I'd do that separately, the verbosity changes are big and noisy already.
Feel free to take over and commit any part of this diff, I just put it
here to demonstrate the shell bits.
Index: fw_update.sh
===================================================================
RCS file: /cvs/src/usr.sbin/fw_update/fw_update.sh,v
retrieving revision 1.30
diff -u -p -r1.30 fw_update.sh
--- fw_update.sh 12 Jan 2022 02:21:15 -0000 1.30
+++ fw_update.sh 16 Jan 2022 16:27:09 -0000
@@ -35,7 +35,7 @@ FWURL=http://firmware.openbsd.org/firmwa
FWPUB_KEY=${DESTDIR}/etc/signify/openbsd-${VERSION}-fw.pub
DRYRUN=false
-VERBOSE=false
+integer VERBOSE=0
DELETE=false
DOWNLOAD=true
INSTALL=true
@@ -69,18 +69,19 @@ tmpdir() {
fetch() {
local _src="${FWURL}/${1##*/}" _dst=$1 _user=_file _exit _error=''
+ local _flags=-VM
+
+ ((VERBOSE > 1)) && _flags=-vm
# The installer uses a limited doas(1) as a tiny su(1)
set -o monitor # make sure ftp gets its own process group
(
- flags=-VM
- "$VERBOSE" && flags=-vm
if [ -x /usr/bin/su ]; then
exec /usr/bin/su -s /bin/ksh "$_user" -c \
- "/usr/bin/ftp -N '${0##/}' -D 'Get/Verify' $flags -o-
'$_src'" > "$_dst"
+ "/usr/bin/ftp -N '${0##/}' -D 'Get/Verify' $_flags -o-
'$_src'" > "$_dst"
else
exec /usr/bin/doas -u "$_user" \
- /usr/bin/ftp -N "${0##/}" -D 'Get/Verify' $flags -o-
"$_src" > "$_dst"
+ /usr/bin/ftp -N "${0##/}" -D 'Get/Verify' $_flags -o-
"$_src" > "$_dst"
fi
) & FTPPID=$!
set +o monitor
@@ -216,9 +217,9 @@ detect_firmware() {
add_firmware () {
local _f="${1##*/}" _pkgname
FWPKGTMP="$( tmpdir "${DESTDIR}/var/db/pkg/.firmware" )"
- local flags=-VM
- "$VERBOSE" && flags=-vm
- ftp -N "${0##/}" -D "Install" "$flags" -o- "file:${1}" |
+ local _flags=-VM
+ ((VERBOSE > 1)) && _flags=-Vm
+ ftp -N "${0##/}" -D "Install" "$_flags" -o- "file:${1}" |
tar -s ",^\+,${FWPKGTMP}/+," \
-s ",^firmware,${DESTDIR}/etc/firmware," \
-C / -zxphf - "+*" "firmware/*"
@@ -249,7 +250,7 @@ delete_firmware() {
local _cwd _pkg="$1" _pkgdir="${DESTDIR}/var/db/pkg"
# TODO: Check hash for files before deleting
- "$VERBOSE" && echo "Uninstalling $_pkg"
+ ((VERBOSE > 2)) && echo "Uninstall $_pkg ..."
_cwd="${_pkgdir}/$_pkg"
if [ ! -e "$_cwd/+CONTENTS" ] ||
@@ -283,6 +284,8 @@ delete_firmware() {
rm -f "$_r"
fi
done
+
+ ((VERBOSE > 2)) && echo " done."
}
usage() {
@@ -300,7 +303,7 @@ do
F) OPT_F=true ;;
n) DRYRUN=true ;;
p) LOCALSRC="$OPTARG" ;;
- v) VERBOSE=true ;;
+ v) ((++VERBOSE)) ;;
:)
echo "${0##*/}: option requires an argument -- -$OPTARG" >&2
usage 2
@@ -354,6 +357,9 @@ set -sA devices -- "$@"
if "$DELETE"; then
[ "$OPT_F" ] && echo "Cannot use -F and -d" >&2 && usage 22
+ # Show the "Uninstall" message when just deleting not upgrading
+ ((VERBOSE > 1)) && VERBOSE=3
+
set -A installed
if [ "${devices[*]:-}" ]; then
"$ALL" && echo "Cannot use -a and devices/files" >&2 && usage 22
@@ -381,7 +387,7 @@ if "$DELETE"; then
if [ "${installed:-}" ]; then
for fw in "${installed[@]}"; do
if "$DRYRUN"; then
- echo "Delete $fw"
+ ((VERBOSE)) && echo "Delete $fw"
else
delete_firmware "$fw" || continue
fi
@@ -405,9 +411,9 @@ CFILE="$LOCALSRC/$CFILE"
if [ "${devices[*]:-}" ]; then
"$ALL" && echo "Cannot use -a and devices/files" >&2 && usage 22
else
- "$VERBOSE" && echo -n "Detecting firmware ..."
+ ((VERBOSE > 1)) && echo -n "Detect firmware ..."
set -sA devices -- $( detect_firmware )
- "$VERBOSE" &&
+ ((VERBOSE > 1)) &&
{ [ "${devices[*]:-}" ] && echo " found." || echo " done." ; }
fi
@@ -437,7 +443,7 @@ for f in "${devices[@]}"; do
if "$INSTALL" && [ "${installed[*]:-}" ]; then
for i in "${installed[@]}"; do
if [ "${f##*/}" = "$i.tgz" ]; then
- "$VERBOSE" && echo "Keep $i"
+ ((VERBOSE > 2)) && echo "Keep $i"
kept="$kept,$d"
continue 2
fi
@@ -447,11 +453,11 @@ for f in "${devices[@]}"; do
if [ -e "$f" ]; then
if "$DOWNLOAD"; then
if "$verify_existing" && ! "$DRYRUN"; then
- "$VERBOSE" && ! "$INSTALL" &&
+ ((VERBOSE > 1)) && ! "$INSTALL" &&
echo "Keep/Verify ${f##*/}"
verify "$f" || continue
else
- "$VERBOSE" && ! "$INSTALL" &&
+ ((VERBOSE > 1)) && ! "$INSTALL" &&
echo "Keep ${f##*/}"
fi
"$INSTALL" || kept="$kept,$d"
@@ -459,10 +465,14 @@ for f in "${devices[@]}"; do
fi
elif "$DOWNLOAD"; then
if "$DRYRUN"; then
- "$VERBOSE" && echo "Get/Verify ${f##*/}"
+ ((VERBOSE)) && echo "Get/Verify ${f##*/}"
else
- fetch "$f" || continue
- verify "$f" || continue
+ ((VERBOSE == 1)) && echo -n "Get/Verify ${##*/} ..."
+ fetch "$f" && verify "$f" || {
+ ((VERBOSE == 1)) && echo " failed."
+ continue
+ }
+ ((VERBOSE == 1)) && ! "$INSTALL" && echo " done."
fi
"$INSTALL" || added="$added,$d"
elif "$INSTALL"; then
