Warning messages for handlers are never called

Bug #1331814 reported by Hamilton Turner
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
kickseed (Ubuntu)
Fix Committed
Undecided
Hamilton Turner

Bug Description

Multiple handlers use the following syntax:

     eval set -- "$(getopt -o '' -l interpreter: -- "$@")" || { warn_getopt %pre; return; }

As mentioned in "/usr/share/doc/util-linux-ng-2.17.2/getopt-parse.bash", this will never
call the warning:

    # We need TEMP as the `eval set --' would nuke the return value of getopt.
   TEMP=`getopt -o ab:c:: --long a-long,b-long:,c-long:: \
      -n 'example.bash' -- "$@"`

Here is a simple example to prove this:

    #!/bin/sh
    trap 'echo ERROR happened' ERR
    echo begin
    eval set -- "$(false)"
    echo end

Because of this, almost every bug report mentions something along the lines of "getopt printed an error". This is getout outputting to stderr, but the non-zero exit code is being swallowed and errexit is not happening.

This coding was probably designed intentionally to disable automatic exit whenever getopt found an unexpected argument, due to the fact that this (currently) happens on almost every kickstart file as our syntax support is quite limited.

However, it also hides the warning from the user, which I believe is the real bug. Moreover, the only error is printed briefly to a terminal and then never seen again, which makes it tough for people to put in bug reports.

Perhaps a better solution would be to use set +x and set -e to temporarily disable errexit, and use a trap on ERR to ensure that these errors are caught and printed to both syslog and the current TTY.

description: updated
Revision history for this message
Hamilton Turner (hamiltont-x) wrote :

Fixed. Ash/Dash don't support ERR traps, so I instead did this:

        set +e
        output=$(getopt -o '' -l $options -- "$@")
        errout=$(getopt -o '' -l $options -- "$@" 2>&1 >/dev/null)
        set -e

        if [[ -n "$errout" ]]; then
                warn_getopt $type
                warn "$type Error: $errout"
        fi

This results in much better logging during install, without stopping output to console. For example:

    $ cat /var/log/syslog | grep kickseed
    > ... snip ... <
    Jun 19 01:56:26 kickseed: Failed to parse %pre options
    Jun 19 01:56:26 kickseed: %pre Error: getopt: unrecognized option '--log=/test.log'

I've also integrated this with test-kickseed, which outputs this:

    $ ./test-kickseed custom-test.ks
    getopt: unrecognized option '--log=/test.log'
    LOG: Failed to parse some %pre options
    LOG: %pre Error: getopt: unrecognized option '--log=/test.log'

As you can see, getopt still prints to stderr, but now we have log messages printed as well.

Changed in kickseed (Ubuntu):
assignee: nobody → Hamilton Turner (hamiltont-x)
status: New → Fix Committed
Revision history for this message
Hamilton Turner (hamiltont-x) wrote :

Also, I added a "reference" file for kickstart that lists all commands with all command options. By doing

    $ ./test-kickseed tests/full-full-06-2014.ks

we can see which kickstart options are not currently supported, as they will print out "LOG" messages. It's
not exactly test driven dev, but it's something ;-)

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.