Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
anarcat opened this issue Jan 22, 2024 · 3 comments · Fixed by #269
Closed

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

anarcat opened this issue Jan 22, 2024 · 3 comments · Fixed by #269

Comments

@anarcat
Copy link
Contributor

anarcat commented Jan 22, 2024

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

if [ $RC -ne 0 ] ; then

... 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...

@adrelanos
Copy link
Contributor

Good catch. That one slipped through.

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

if [ $RC -ne 0 ] ; then

... 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
Copy link
Contributor Author

anarcat commented Jan 23, 2024

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 added a commit to anarcat/grml-debootstrap that referenced this issue Jan 23, 2024
We were using the old-style "RC" error checking. Now that has all been
moved to a global error handler, so we need to move log parsing there
as well.

Closes: grml#268
@anarcat
Copy link
Contributor Author

anarcat commented Jan 23, 2024

something like #269 perhaps?

@mika mika closed this as completed in cfa1651 Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants