shellcheck icon indicating copy to clipboard operation
shellcheck copied to clipboard

`set -o pipefail` is part of POSIX now, consider disabling the warning for `sh`

Open tgross35 opened this issue 3 years ago • 11 comments

The POSIX:2022 specification adds support for set -o pipefail, so it might make sense to disable this warning even for #/bin/sh.

Source on adding pipefail: POSIX discussion https://www.austingroupbugs.net/view.php?id=789, list of shell support https://unix.stackexchange.com/questions/654885/who-is-responsible-for-providing-set-o-pipefail/654932#654932, work on dash adding it https://lore.kernel.org/dash/[email protected]/

As mentioned in stackoverflow, most sh implementations support pipefail already, dash being about the last big one to not support it. And they are working on it, as mentioned above.

For bugs

  • Rule Id (if any, e.g. SC1000): SC3040 (pipefail)
  • My shellcheck version (shellcheck --version or "online"): v0.8.0
  • [x] The rule's wiki page does not already cover this
  • [x] I tried on https://www.shellcheck.net/ and verified that this is still a problem on the latest commit

Here's a snippet or screenshot that shows the problem:

#!/bin/sh
set -o pipefail

Here's what shellcheck currently says:

set -o pipefail
       ^-- [SC3040](https://www.shellcheck.net/wiki/SC3040) (warning): In POSIX sh, set option pipefail is undefined.

Here's what I wanted or expected to see:

No error

tgross35 avatar Aug 11 '22 17:08 tgross35

It might be worth waiting a year or two for these updates to flow through to mainstream distributions. For example it will probably be at least a year before there is a stable version of Debian that carries a version of dash with support for this.

Maybe downgrade it to a warning instead of an error?

djmattyg007 avatar Oct 04 '22 13:10 djmattyg007

Notably, Alpine Linux, popular in the world of containers, uses ash as the /bin/sh implementation. Other distributions tend to implement sh with simply bash.

Has ash caught up to dash's pipefail support yet?

Busybox's ash has pipefail support tests. What about Toybox's ash?

Which UNIX distributions, if any, use a pre-ksh93 implementation of sh?

mcandre avatar Feb 08 '23 05:02 mcandre

I think you have it backwards - Ash, as used by Alpine, has pipefail but Dash, used by Debian, does not. Still.

$ docker run --rm -it alpine sh
/ # set -o pipefail
/ # grep some-string /non/existent/file | sort
grep: /non/existent/file: No such file or directory
/ # echo $?
2        <- correct
docker run --rm -it debian sh
# set -o pipefail
sh: 1: set: Illegal option -o pipefail

...which sucks. I tried to poke on the mailing list, but nobody picked it up https://lore.kernel.org/dash/[email protected]/

If you know any maintainers of dash, ping them and ask them to review that patch...

tgross35 avatar Feb 08 '23 07:02 tgross35

Thank you for the detailed correction.

Can we push the POSIX group to finally, belatedly release the standard? That would help with adoption.

mcandre avatar Mar 01 '23 21:03 mcandre

It is a very serious WG that rightfully prefers to get things right then to get them fast, so I don’t really think there’s anything to push them for - we’ll get the standard when it’s done. But if you’d like to get involved with the standard process, I believe getting on the Austin Group mailing list is the place to start.

But I think it’s probably better to push Dash to merge the feature. You can hop on their mailing list (linked above) and gently push for a maintainer review, or do a review/rebase of the patch yourself to help get the ball rolling.

tgross35 avatar Mar 01 '23 21:03 tgross35

Even when supported everywhere I think it is better to demote it to "info" or "style" rather than remove it for those that need to support older shells.

ale5000-git avatar Mar 01 '23 21:03 ale5000-git

Even when supported everywhere I think it is better to demote it to "info" or "style" rather than remove it for those that need to support older shells.

Depending on how/where a script is used, it might make sense to use a more recent feature if it is deemed helpful enough (such as -o pipefail in a project-specific POSIX shell script) or to strictly target a more conservative common denominator (such as something that is intended to run on as many systems as possible).

Considering both scenarios, I think that it would be good to have a way to explicitly denote what standard version or shell version a shell script is targeting.

For example:

#!/bin/sh
# shellcheck shell=sh std=posix-2008
#!/bin/bash
# shellcheck shell=bash std=bash-3.2

Or:

#!/bin/sh
# shellcheck shell=sh-posix-2008
#!/bin/bash
# shellcheck shell=bash-3.2

This would free the maintainers from having to decide what is or isn't appropriate for all versions of any given shell (and what exactly is to be classified as a portability warning vs just an info/style message).

In other words, it would allow specifying what is or is not appropriate for each "feature level" of each shell.

Exactly which versions of which shells would make sense to define is a separate matter, but having at least one for POSIX.1-2008 (Issue 7) and another for POSIX.1-202X (Issue 8) would be helpful.

Note that some scripting languages have native support for specifying the minimum version directly in the script (for example, use 5.010; in Perl to target Perl 5.10), so this wouldn't be anything too novel.

kmk3 avatar Mar 02 '23 07:03 kmk3

yash shell also lacks support for pipefail atm

dxlr8r avatar Sep 17 '23 19:09 dxlr8r

FYI, another workaround for now is to just try setting pipefail in a subshell (so failures don't propagate), and then set it in your main shell only if it succeeds. I added this to the wiki

# shellcheck disable=SC3040
(set -o pipefail 2> /dev/null) && set -o pipefail

Austin Group released draft 4.1 last month, hopefully they are getting close to final... https://www.opengroup.org/austin/

tgross35 avatar Mar 04 '24 07:03 tgross35

It might be worth waiting a year or two for these updates to flow through to mainstream distributions

2024

Talador12 avatar Jun 04 '24 16:06 Talador12

It might be worth waiting a year or two for these updates to flow through to mainstream distributions

2024

The standard only just got approved (see the link in my previous comment) and the patch only just got applied to Dash with no release yet. Nothing has really changed.

tgross35 avatar Jun 04 '24 17:06 tgross35