You are not logged in.

#1 2018-10-05 07:41:59

Ataraxy
Member
Registered: 2016-11-21
Posts: 10

set -uo pipefail

As standard bash scripting practice, I generally put:

set -euo pipefail
shopt -s failglob

At the top of any scripts, as recommended in unofficial bash strict mode.

I know that makepkg already sets -e.

I need to use a git tag in variable $_tag and want to ensure against any pipeline failures in generating it, or the $pkgver derived from it.

However, when I do this, I get:

/usr/share/makepkg/lint_pkgbuild/pkgver.sh: line 33: $2: unbound variable

Would Arch's makepkg folk be likely to accept a PR on this?  If so, can you point me to the contributing guide?

Last edited by Ataraxy (2018-10-05 07:42:34)

Offline

#2 2018-10-05 07:58:29

Alad
Wiki Admin/IRC Op
From: Bagelstan
Registered: 2014-05-04
Posts: 2,407
Website

Re: set -uo pipefail

set -euo pipefail is not recommended in any way. Don't trust blogs on the internet. For example:

https://mywiki.wooledge.org/BashFAQ/105

for set -e. Instead of set -u, use shellcheck:

https://www.shellcheck.net/

Instead of pipefail, consider PIPESTATUS. For example, SIGPIPE might cause unexpected (but otherwise harmless) errors:

http://www.gnu.org/software/libc/manual … nding-Data

Last edited by Alad (2018-10-05 08:03:02)


Mods are just community members who have the occasionally necessary option to move threads around and edit posts. -- Trilby

Offline

#3 2018-10-05 10:55:40

Everette88
Member
Registered: 2018-02-17
Posts: 41

Re: set -uo pipefail

Alad wrote:

set -euo pipefail is not recommended in any way. Don't trust blogs on the internet. For example:

https://mywiki.wooledge.org/BashFAQ/105

1. Say "Don't trust blogs on the internet".
2. Link to a blog.
3. Profit.

Offline

#4 2018-10-05 22:24:51

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: set -uo pipefail

Ataraxy wrote:

As standard bash scripting practice, I generally put:

set -euo pipefail
shopt -s failglob

At the top of any scripts, as recommended in unofficial bash strict mode.

The author of this article is a fool and shouldn't be trusted to tie his own shoelaces, let alone advise people how to write bash.

For exhibit A, I will point out that part of his advice is to set IFS=$'\t\n', with the rationale:

a terrible blog wrote:

Got all that? The next question is, why are we setting IFS to a string consisting of a tab character and a newline? Because it gives us better behavior when iterating over a loop. By "better", I mean "much less likely to cause surprising and confusing bugs". This is apparent in working with bash arrays:

#!/bin/bash
names=(
  "Aaron Maxwell"
  "Wayne Gretzky"
  "David Beckham"
  "Anderson da Silva"
)

echo "With default IFS value..."
for name in ${names[@]}; do
  echo "$name"
done

The root of his problem, mind you, is that he used ${names[@]} without quoting it as "${names[@]}", and as a result his array was *re-evaluated and word-splitted*. Whereas anyone with any clue knows to quote this, to ensure bash will parameterize this as originally intended, one word per array element.

Ataraxy wrote:

I know that makepkg already sets -e.

makepkg *only* does this for user-provided build()/package() etc functions. We disabled it for the main script over 6 years ago in favor of writing our own error handling.

Ataraxy wrote:

Would Arch's makepkg folk be likely to accept a PR on this?  If so, can you point me to the contributing guide?

We deliberately disabled errexit, so we will reject any proposed patch to re-add it. (Aside: pacman/makepkg is developed on a mailing list, not on Github, so you cannot send PRs and will have to mail in a patch using git send-email. No, we're not interested in changing this process either.)

Why do you think you need it? The source array is not supposed to be dynamically determined, as it is evaluated when sourcing the file and not after the prepare/pkgver functions are executed. And when generating the pkgver itself using the pkgver() function, we already define error handling for empty versions -- we re-run the pkgver linting code, which among other things forbids empty $pkgver

...

I'm having a difficult time envisioning what you are doing, so it is difficult to know what kind of suggestions I should be providing...

Last edited by eschwartz (2018-10-05 22:30:03)


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

#5 2018-10-06 11:18:00

Everette88
Member
Registered: 2018-02-17
Posts: 41

Re: set -uo pipefail

Eschwartz wrote:

The root of his problem, mind you, is that he used ${names[@]} without quoting it as "${names[@]}", and as a result his array was *re-evaluated and word-splitted*. Whereas anyone with any clue knows to quote this, to ensure bash will parameterize this as originally intended, one word per array element.

The author did acknowledged this and even commented about it: http://redsymbol.net/articles/unofficia … footnote-2

Is it smart to call someone fool without reading the whole thing?

Last edited by Everette88 (2018-10-06 14:31:29)

Offline

#6 2018-10-06 13:49:39

Trilby
Inspector Parrot
Registered: 2011-11-29
Posts: 29,442
Website

Re: set -uo pipefail

Why change the entire behavior of the shell to check whether a variable has been set?  Just check if the variable has been set:

[[ -z "$_tag" ]] && exit 1

"UNIX is simple and coherent..." - Dennis Ritchie, "GNU's Not UNIX" -  Richard Stallman

Offline

#7 2018-10-06 14:19:10

Everette88
Member
Registered: 2018-02-17
Posts: 41

Re: set -uo pipefail

Trilby wrote:

Why change the entire behavior of the shell to check whether a variable has been set?  Just check if the variable has been set:

[[ -z "$_tag" ]] && exit 1

I guess the point is to not having to make such checks for each variable.

Offline

#8 2018-10-06 14:22:15

Trilby
Inspector Parrot
Registered: 2011-11-29
Posts: 29,442
Website

Re: set -uo pipefail

There is only one variable that the OP wants to check.


"UNIX is simple and coherent..." - Dennis Ritchie, "GNU's Not UNIX" -  Richard Stallman

Offline

#9 2018-10-07 00:43:38

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: set -uo pipefail

Everette88 wrote:
Eschwartz wrote:

The root of his problem, mind you, is that he used ${names[@]} without quoting it as "${names[@]}", and as a result his array was *re-evaluated and word-splitted*. Whereas anyone with any clue knows to quote this, to ensure bash will parameterize this as originally intended, one word per array element.

The author did acknowledged this and even commented about it: http://redsymbol.net/articles/unofficia … footnote-2

Is it smart to call someone fool without reading the whole thing?

I did read the whole thing, and I dismissed that footnote as being additional proof that the author is an incompetent fool... not a counterproof to defend his rationale.

He calls it "another approach", casts aspersions on it as being "unmaintainable" and unwise to rely on, but acknowledges "This changes loop semantics to produces the nicer behavior, and even handles a few edge cases better". And he finishes off with this masterpiece of incorrectness: "In short, relying on quotes has a high risk of introducing subtle time-bomb bugs. Setting IFS renders this impossible."

Unfortunately that is utterly wrong in nearly every conceivable way. It doesn't change the loop semantics, it simply prevents any and all word-splitting no matter what. It's a freaking array, it's already fundamentally split in the only way that matters.

He seems to be under the incredible delusion that splitting on tabs and newlines but not spaces, will make it "impossible" to break your code with word splitting. I say it makes it more likely, since word splitting only exists because it's needed, like, a lot. Whether it breaks the use of $EDITOR (which cannot be quoted since it can contain arguments, and cannot be an array because it's a legacy variable that needs to be compatible with all shells as well as every other program ever, even the ones that aren't shells, *none* of which support arrays, because there's no such unix concept as environment arrays), or whether it breaks the use of builtins like "read" where you actually do want to parameterize text based on whitespace (for example, safely whitesplitting a variable into an array in order to keep your code safe from dangerously unquoted variables), changing the fundamental shell semantics *globally* for such ridiculously foolish reasons is simply unacceptable.

He calls "having the most basic form of sanity" a case of "handling a few edge cases". This is not the sign of a mind that comprehends programming.

He calls it "unmaintainable" to leave "time-bomb bugs" around, as if there aren't a multitude of tools like shellcheck whose greatest purpose in existence is to loudly warn you every time you don't quote a variable, in accordance with the wisdom of most actually knowledgeable bash experts who will take excessive pleasure in repetitively pounding the following logic into the thick heads of fools like this blog author:

the wise bash experts wrote:

Quote your variables, all the time, always, no matter what. No exceptions at all. If you think you have an exception, you're almost certainly wrong, and in the unlikely event you are right, you should still quote them anyway to prevent actual, real ticking time bombs.

In summary, quote your variables. Variables should be quoted. End of story. Because unquoted variables are bad and will cause you grief and anguish.

By the way, in case you forgot, it's a good idea to quote your variables.

P.S. Please don't forget to quote your variables, it's exceedingly important.


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

#10 2018-10-08 01:12:26

Ataraxy
Member
Registered: 2016-11-21
Posts: 10

Re: set -uo pipefail

It's been about 2 years since I read that article. Yup, he gets it totally wrong on quoting, I do agree.

When I raised this because initially, I was setting a variable $_tag because I didn't understand how -git PKGBUILDs are expected to work (I was selecting a non-alpha tag). I had to check if $_tag was set in pkgver(), as prepare() may not have been run with `makepkg -e`.

Regardless of the backstory, I realised that `set -u` broke the pkgbuild framework:

/usr/share/makepkg/lint_pkgbuild/pkgver.sh: line 33: $2: unbound variable

@Alad, I love shellcheck, but its no substitute for `set -u` - it can't detect what has been exported from the parent process, and therefore what is unset.

I thought that `s/$2/${2-}/` would have been possible...

If in understand aright from the AUR mailing list, the `shopt` and `set`s are saved and restored before / after entering user functions. Why not do the same before / after sourcing PKGBUILD, except for a possible few which are whitelisted?

I'd love to see how `shopt -s failglob` would cause any issues in a PKGBUILD. It seems sensible not only to allow, but indeed to set by default. (It causes an early exit if an unquoted `*` or `?` would fall back to a literal as it didn't match any files.)

Offline

#11 2018-10-08 02:50:30

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: set -uo pipefail

Ataraxy wrote:

If in understand aright from the AUR mailing list, the `shopt` and `set`s are saved and restored before / after entering user functions. Why not do the same before / after sourcing PKGBUILD, except for a possible few which are whitelisted?

Maybe because no one implemented it. But I don't really see much of a reason to reset it, since the practical effect would be to ensure that people who did set something in the PKGBUILD, would have their attempts undone.

Whereas restoring it after entering user functions, is to allow users to specify options they definitely want in their functions, but which shouldn't spread out of the function.

I'd love to see how `shopt -s failglob` would cause any issues in a PKGBUILD. It seems sensible not only to allow, but indeed to set by default. (It causes an early exit if an unquoted `*` or `?` would fall back to a literal as it didn't match any files.)

Now that I can actually get behind, except I'd prefer nullglob plus explicitly handling the case where you have no matches. This has utility for loops, where you don't have to check

for file in *glob*; do
    if [ -f "$i" ]; then
        do_per_file_things
    fi
done

I'm also a big fan of extglob as it should really default to on... in fact, makepkg makes extensive use of extglob.


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

#12 2018-10-08 03:42:23

fukawi2
Ex-Administratorino
From: .vic.au
Registered: 2007-09-28
Posts: 6,217
Website

Re: set -uo pipefail

Let's keep this on topic of the original question folks....  https://xkcd.com/386/

Offline

#13 2018-10-08 11:29:17

Alad
Wiki Admin/IRC Op
From: Bagelstan
Registered: 2014-05-04
Posts: 2,407
Website

Re: set -uo pipefail

Everette88 wrote:
Alad wrote:

set -euo pipefail is not recommended in any way. Don't trust blogs on the internet. For example:

https://mywiki.wooledge.org/BashFAQ/105

1. Say "Don't trust blogs on the internet".
2. Link to a blog.
3. Profit.

A blog, by definition, is a website with personal commentary by one (or more) authors, which does not allow outside contributions or review. BashFAQ is a wiki, and the only one which has (with respect to Bash) proven reliable over time.

Ataraxy wrote:

I thought that `s/$2/${2-}/` would have been possible...

I guess you can also use [[ -v foo ]] with newer bash versions. (makepkg depends on 4.4)

Last edited by Alad (2018-10-08 11:39:53)


Mods are just community members who have the occasionally necessary option to move threads around and edit posts. -- Trilby

Offline

Board footer

Powered by FluxBB