grml / grml-debootstrap

wrapper around debootstrap
59 stars 27 forks source link

line 1728: [: -ne: unary operator expected #268

Closed anarcat closed 7 months ago

anarcat commented 7 months ago

in the latest release, grml-debootstrap yields the above warning. Line 1728 is:

https://github.com/grml/grml-debootstrap/blob/1675ca627a824e701a55d5b50bcf9a969db7c6e1/grml-debootstrap#L1728

... which seems pretty innocuous, but RC is actually never defined. It was ripped out in https://github.com/grml/grml-debootstrap/commit/9706bd99799c2d690d4c8f4f69d158aee307c408 (#262).

I don't quite understand how that patch was supposed to work, it seems to me it just completely removes error-checking here...

adrelanos commented 7 months ago

Good catch. That one slipped through.

in the latest release, grml-debootstrap yields the above warning. Line 1728 is:

https://github.com/grml/grml-debootstrap/blob/1675ca627a824e701a55d5b50bcf9a969db7c6e1/grml-debootstrap#L1728

... which seems pretty innocuous, but RC is actually never defined. It was ripped out in 9706bd9 (#262).

I don't quite understand how that patch was supposed to work, it seems to me it just completely removes error-checking here...

Do you mean script wide? In that case, unless specifically handling an error, it all relies now in the general error handling:

set -e
set -E
set -o pipefail
trap "error_handler" ERR
export -f "error_handler"

line 1728: [: -ne: unary operator expected

The error handling in this specific case, this specific feature was an oversight. Related source code:

  if [ $RC -ne 0 ] ; then
    if [ -r "$MNTPOINT/debootstrap/debootstrap.log" ] && \
      [ -s "$MNTPOINT/debootstrap/debootstrap.log" ] ; then
      einfo "Presenting last ten lines of debootstrap.log:"
      tail -10 "${MNTPOINT}"/debootstrap/debootstrap.log
      einfo "End of debootstrap.log"
    fi
  fi

Condition if [ $RC -ne 0 ] ; then needs to be removed because error handling does not work like that anymore. And the rest of that code / feature could probably be moved to function error_handler.

anarcat commented 7 months ago

Do you mean script wide? In that case, unless specifically handling an error, it all relies now in the general error handling:

Yeah, so I guess that's what i mean: that wasn't made explicit in the patch, so we're left guessing what the plan is.

Condition if [ $RC -ne 0 ] ; then needs to be removed because error handling does not work like that anymore. And the rest of that code / feature could probably be moved to function error_handler.

oh yeah, that seems simple enough...

anarcat commented 7 months ago

something like https://github.com/grml/grml-debootstrap/pull/269 perhaps?