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

&& in the end of script or function if set -e enabled #266

Open
safinaskar opened this Issue Dec 11, 2014 · 4 comments

Comments

Projects
None yet
3 participants
@safinaskar

safinaskar commented Dec 11, 2014

Hi. I often use set -e in my scripts. In this mode foo && bar works exactly as I want: if foo fails, then bar will not be evaluated, but the script itself will not exit and will continue to evaluate. But, if this foo && bar is in the end of some function or script, then, unfortunately, exit code of this function or script will be non-zero, which will lead to very hard-to-catch bugs. Consider the following example:

#!/bin/sh

set -e

false && echo Not printed # "Not printed" really will not be printed,
# exactly as I want, this is good :)

echo Continued # But the script will continue evaluating, so
# "Continued" will be printed, again, exactly as I want, this is good,
# too :)

f(){
  false && echo Not printed # Again, "Not printed" really not printed,
  # all is OK :)
}

f
# Attention! Here "f" returned non-zero and "set -e" is enabled, so
# the script will exit here. And this is very annoying. This is very-very
# hard-to-catch bug :(

echo Not printed # Unfortunately, this will not be printed :(

# So, the right way to write such functions (if you use "set -e"):
f(){
  false && echo Not printed
  :
}

Also, && in the end of a script is bad, too. For example:

#!/bin/sh

set -e

false && echo Not printed

This script will return non-zero and this is very unexpected, too.

So, please, add to your checker warning "&& in the end of script or function and "set -e" is enabled, consider adding ":" after the command". Also, I think this should be added even if "set -e" is not enabled (because script in question may be sourced from "set -e"-script).

Also, I recommend you, koalaman, and also, I recommend all people reading this bug report to do the following: please check all your scripts and really consider adding ":" after "&&"-commands in the end of functions and scripts

@timurb

This comment has been minimized.

Show comment
Hide comment
@timurb

timurb Dec 13, 2014

I thought the construction false && echo would always produce an error with set -e enabled (could it be only some of the shells behave like that?) and for that reason I try to always append it with ||: like false && echo "This is false" ||:.

I'd like to better have this check instead which should also be easier to implement.

timurb commented Dec 13, 2014

I thought the construction false && echo would always produce an error with set -e enabled (could it be only some of the shells behave like that?) and for that reason I try to always append it with ||: like false && echo "This is false" ||:.

I'd like to better have this check instead which should also be easier to implement.

@safinaskar

This comment has been minimized.

Show comment
Hide comment
@safinaskar

safinaskar Dec 13, 2014

timurb,

16  2014-12-13 19:05:08  ~# bash -c 'set -e; false && :; echo word'
word
16  2014-12-13 19:05:16  ~# dash -c 'set -e; false && :; echo word'
word
16  2014-12-13 19:05:18  ~# ash -c 'set -e; false && :; echo word'
word
16  2014-12-13 19:05:20  ~# ksh -c 'set -e; false && :; echo word'
word
16  2014-12-13 19:05:22  ~# zsh -c 'set -e; false && :; echo word'
word

Writing a && b || : is bad idea. Because the shell will not exit if b fails (and you probably want it to exit in such situation). So, one should write a && b; : in such situation (or just a && b if you are sure this is not end of function or script)

safinaskar commented Dec 13, 2014

timurb,

16  2014-12-13 19:05:08  ~# bash -c 'set -e; false && :; echo word'
word
16  2014-12-13 19:05:16  ~# dash -c 'set -e; false && :; echo word'
word
16  2014-12-13 19:05:18  ~# ash -c 'set -e; false && :; echo word'
word
16  2014-12-13 19:05:20  ~# ksh -c 'set -e; false && :; echo word'
word
16  2014-12-13 19:05:22  ~# zsh -c 'set -e; false && :; echo word'
word

Writing a && b || : is bad idea. Because the shell will not exit if b fails (and you probably want it to exit in such situation). So, one should write a && b; : in such situation (or just a && b if you are sure this is not end of function or script)

@koalaman

This comment has been minimized.

Show comment
Hide comment
@koalaman

koalaman Mar 20, 2015

Owner

@safinaskar Why do you use set -e at all in this example if you don't want to exit on non-zero command return statuses?

Owner

koalaman commented Mar 20, 2015

@safinaskar Why do you use set -e at all in this example if you don't want to exit on non-zero command return statuses?

@safinaskar

This comment has been minimized.

Show comment
Hide comment
@safinaskar

safinaskar Mar 20, 2015

I just always use set -e. And I want to exit on non-zero return, i. e. I want to exit on every non-successful command. But I consider constructs like "a && b" as successful if "a" failed and "b" didn't fail. So, I don't want to exit if "a" failed and "b" is successful. And if this construct is outside of any function, there is really no any exit, exactly as I want. But if this "a && b" stays in the end of function, then this function surprisingly returns non-zero (this contradicts to my intuition because I consider "a && b" as successful and so I consider the function as successful) and then (when the function is called) the script exits (I don't want this).

Remember: this is just simplified examples. This cases may be parts of some real big script. In such script I want to use set -e

safinaskar commented Mar 20, 2015

I just always use set -e. And I want to exit on non-zero return, i. e. I want to exit on every non-successful command. But I consider constructs like "a && b" as successful if "a" failed and "b" didn't fail. So, I don't want to exit if "a" failed and "b" is successful. And if this construct is outside of any function, there is really no any exit, exactly as I want. But if this "a && b" stays in the end of function, then this function surprisingly returns non-zero (this contradicts to my intuition because I consider "a && b" as successful and so I consider the function as successful) and then (when the function is called) the script exits (I don't want this).

Remember: this is just simplified examples. This cases may be parts of some real big script. In such script I want to use set -e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment