POSIX strict mode

[RichiH]

In light of https://github.com/RichiH/vcsh/pull/302 I think we must tighten vcsh up a lot.

https://gist.github.com/EvgenyOrekhov/5c1418f4710558b5d6717d9e69c6e929 & https://jmmv.dev/2018/03/shell-readability-strict-mode.html largely agree but I don't know off-hand if either form is adopted more widely. I would prefer the more verbose set -o form for readability. @mirabilos what would you suggest?

[alerque]

I'm far mor familiar with set -e instead of set -o errexit, but haven't seem much of either set -u or set -o nounset in use. At the end of the day I don't cane which we use but completely agree we should be using these.

I seem to recall previous discussion about them, but that might have been as far back as when the goal was still to make vcsh a set of sourceable functions.

[mirabilos]

Richard Hartmann dixit:

@.*** what would you suggest?

POSIX knows set -e and set -u (and thus set -eu).

I would advice strongly against “set -o errexit” as this is almost unheard of and would confuse many, while “set -e” is very common.

In the same vain, “set -u” is more well-known than “set -o …what was it again… ah, nounset”, see, I had to look it up while writing this.

I’d advice against using “set -u” at all though. It tends to introduce errors at the user’s when least expected. Maybe as developer option, though scripts without “set -u” are massively more readable — in the same manner, I’d generally advice against “set -e”, which makes proper error handling impossible by imposing an aborting behaviour on nōnzero exit codes (though I use it in some scripts, or partially, enabling and disabling it during the lifetime, where useful).

“set -e” bites unexpectedly occasionally:

$ sh -ec '(( 1 + 1 )); echo meow'; echo $? meow 0 $ sh -ec '(( 1 - 1 )); echo meow'; echo $? 1

Therefore, I consider “set -e” a shell newbie tool, counterproductive to more experienced developers, and “set -u” is even worse, severely reducing legibility.

bye, //mira“you d̲i̲d̲ ask for my opinion”bilos -- Gestern Nacht ist mein IRC-Netzwerk explodiert. Ich hatte nicht damit gerechnet, darum bin ich blutverschmiert… wer konnte ahnen, daß SIE so reagier’n… gestern Nacht ist mein IRC-Netzwerk explodiert~~~ (as of 2021-06-15 The MirOS Project temporarily reconvenes on OFTC)

[RichiH]

Yes, I asked on purpose. If anyone knows the hairy bits of shells from 20 years ago and the intricacies of everything up to now, it's you. :)

The inverse is that -u is likely the shake out ALL the silent bugs in production. If there's an intermediary period in which bugs are reported that is less than ideal, but silent bugs are even worse; and afterwards, it should be more resilient.

-e is similar: once bugs are shaken out, it should hopefully make for smoother sailing.

The question is if 2.0.0 should have them or if we should have a grace period. As a major release sees more and wider scrutiny, I would tend towards keeping it in.

[mirabilos]

Richard Hartmann dixit:

The inverse is that -u is likely the shake out ALL the silent bugs in

Not writing code -u or -e conformant is not a bug, though…

production.

I’d suggest that, if you must have it, you enable it in dev builds only for now, plus make it optional for release users. You can then decide later whether you want to keep it. This is safer.

bye, //mirabilos -- Gestern Nacht ist mein IRC-Netzwerk explodiert. Ich hatte nicht damit gerechnet, darum bin ich blutverschmiert… wer konnte ahnen, daß SIE so reagier’n… gestern Nacht ist mein IRC-Netzwerk explodiert~~~ (as of 2021-06-15 The MirOS Project temporarily reconvenes on OFTC)

[alerque]

My current take on this is that it should not go into the v2 release. I do think it should be added to keep the dev process honest, but I think this should be introduced after we overhaul the test suite. I have an idea there are going to be a couple subtle things this might actually break for some people's scripted usage. I would like to get the test suite working using as close to the current logic as possible, then flip this on and see what happens, then fix anything that broke, then ship it (or not depending on how we want the dev cycle to work after that).

@RichiH How does that sit with you?

[RichiH]

I slept this over a few times. There's a lot of arguments to make in both directions; the strongest one, to me, is that major version changes signal potential breaking changes and a .0.0 is commonly expected to have bugs that need shaking out.

Phrased differently, following the principle of least [user] surprise I believe 2.0.0 is the best time.

The question then becomes if we should wait for 2.0.0 until tests are overhauled; there does not seem to be an immediate need to cut a new release soon, but I am talking about someone else's time (yours) and I don't know what your time plans are for testing.

[alerque]

I've now slept on your feedback a couple times to, and so far I have not come round to that way of thinking. Two reasons stand out to me.

1. Changing something significant like the mode the interpreter runs it is not a small thing and hence runs against my №3 point in wanting to release at all. I know it would be (to many) socially acceptable to break something big in a major-dot-zero-dot-zero release, but I really don't want to do that. I want to change as little as possible so we're sure the result is a drop in replacement, just with different distribution (need to build or switch to standalone releases).

2. I do not expect to have time to overhaul the test suite to be comprehensive enough to trust with a change like this any time soon. Not never, but I have 4 or 5 projects I manage with major chunks waiting on me that I need to prioritize before sitting down to rebase and overhaul the proposed test suite changes and/or fill in any gabs so we can say the whole things is being regression tested.

I'm not saying I can't be convinced, just that I'm not there yet.

[RichiH]

I can see how the mental framing of "mode the interpreter runs" leads to a different risk assessment outcome than "make silent errors visible". I am not saying your framing is wrong, it's very valid. Same as mine in https://github.com/RichiH/vcsh/issues/303#issuecomment-867131881 . Under this lens, the current mode of the interpreter is a potentially serious bug. With those different vantage points, it stands to reason that we come out at different conclusions.

A potential middle ground would be another safe harbour 2.0.0 with an intent to release a 3.0.0 with safer interpreter mode.

And yes, I used the word "safe" in both framings within one sentence on purpose.

[mirabilos]

Richard Hartmann dixit:

I can see how the mental framing of "mode the interpreter runs" leads to a different risk assessment outcome than "make silent errors visible".

“mode the interpreter runs” is more correct though when it comes to the shell; “make silent errors visible” is actually incorrect for shell.

It is perfectly fine, even expected, in shell for commands to return nōnzero for various conditions. Take diff(1) for example: 0 for no differences, 1 for differences and 2+ for errors. Or even arithmetics!

(( --n ))

This returns 1 if n was 1, 0 otherwise, even if it is meant as a statement whose return value does not necessarily need to be checked. (Granted this isn’t (yet) POSIX sh, but it’s a construct that shook me up with set -e, and there’s bound to be many more.)

FTR, there are places in Debian that basically prescribe set -e; it’s always a hassle to work with it, some maintainer script constructs even require temporarily disabling that (or be written in a totally unmain‐ tainable way), so I’ve got experience with both.

Under this lens, the current mode of the interpreter is a potentially serious bug.

No. Missing error handling is a bug. Error handling can sometimes be implemented with set -e but not always (consider 「false | true」, for example; this needs set -o pipefail (not POSIX) to be catchable with set -e) and sometimes it hinders perfectly good code, requiring ugly workarounds, such as replacing 'test x"$a" = x"$b" && echo yes' with 'test x"$a" != x"$b" || echo yes' and things like that. Error handling can, just as well, be implemented manually. This is more effort and a bit more prone to overseeing something but the better approach in the long run. And this wasn’t even adding set -u in the mix…

bye, //mirabilos -- Gestern Nacht ist mein IRC-Netzwerk explodiert. Ich hatte nicht damit gerechnet, darum bin ich blutverschmiert… wer konnte ahnen, daß SIE so reagier’n… gestern Nacht ist mein IRC-Netzwerk explodiert~~~ (as of 2021-06-15 The MirOS Project temporarily reconvenes on OFTC)

[alerque]

A potential middle ground would be another safe harbour 2.0.0 with an intent to release a 3.0.0 with safer interpreter mode.

Sure, something like this. I think what we call it (2.1 or 3.0 or 5.9) will depend on circumstances when we get there, how it behaves in testing, whether there are any use cases expected to break, etc. But yes whatever work we do on that along with the test suite can and should be for another release. I am still not even convinced it is the way to go, as @mirabilos notes the interpreter mode isn't just safer, it is different with some trade offs that may or may not actually be "safer". I'm more convinced than ever that evaluating whether that made is better for this project should be done after a robust test suite is working with the current mode so we stand a chance of noticing what changes.

[RichiH]

If there are major concerns, cutting a major version number seems safest. Either way, let's not block a 2.0 on this.

I have 14d of PTO coming up, hopefully that means some time to churn through too much email and my vcsh backlog.