local -i not supported

[zimbatm]

I was playing with replacing bash with osh nixpkgs: NixOS/nixpkgs#105233 and found this compatibility issue.

Bash:

$ foo() { local -ri bar; echo OK; }; foo
OK

Osh:

osh$ foo() { local -ri bar; echo OK; }; foo
  foo() { local -ri bar; echo OK; }; foo
                ^~~
[ interactive ]:5: 'local' doesn't accept flag -i
OK

Tested with Osh v0.8.4 and 0.8.5

[andychu]

Thanks for trying it! Do you know if the += operator is being used on this variable?

As far as I know that's the only place where the -i flag makes a difference (and I'd be interested to hear otherwise). If you're not using +=, you could just remove -i and it may work.

$ declare -i foo=0; foo+=1; echo $foo; foo+=1; echo $foo
1
2
$ declare +i foo=0; foo+=1; echo $foo; foo+=1; echo $foo
01
011

I would say the canonical way to do it is like this:

(( foo += 1 ))

With the (( )), you don't need the -i flag.

---

This is technically documented here, although I could probably expand on the -i bit:

https://www.oilshell.org/release/0.8.5/doc/known-differences.html#values-are-tagged-with-types-not-cells

Basically Oil behaves like Python and JS with respect to types. Bash barely has types (there is so much auto-conversion), and most people don't know how to use them, so osh does something a little more familiar.

[andychu]

And if there are a ton of scripts using this, I'd be interested in how many ... it should be a fairly greppable pattern, e.g. something like

egrep '\blocal -.?.?i' *.sh

although maybe you also include declare, readonly, etc.

egrep '\b(local|declare|readonly) -.?.?i' *.sh

[zimbatm]

There are two places where it's being used in nixpkgs directly:

• pkgs/build-support/setup-hooks/auto-patchelf.sh (1 time)
• pkgs/stdenv/generic/setup.sh (14 times)
  This is just for the build infrastructure.

However, if osh is being used as a bash replacement, we might also encounter this issue in any of the 40k+ packages being built with it.

Is it not part of the goal of the project to get to 100% compatibility with bash?

[zimbatm]

Reading the docs, it looks like osh is already making the -A and -a options noops. Maybe it's sufficient to add -i to that list?

[andychu]

Hm I found that the += operator isn't the only thing affected. -i actually causes dynamic parsing and evaluation of arithmetic inside strings.

This actually goes back over 3 years to issue #26 ! That's when I added spec/type-compat.test.sh.

---

Answers:

• -a and -A aren't quite no-ops. They are actually the cause of the only documented quirk:
  • https://www.oilshell.org/release/0.8.5/doc/quirks.html
• I would like Oil to run Nix, and a -i might admit some similar hacks, but it's not a priority now. I would help someone who wants to implement it, but I think what will almost certainly happen is that such a person would make small changes to their shell scripts rather than go through the pain of implementing -i :-)
• The goal of OSH isn't 100.0% bash compatibility -- it's to run real bash scripts, which would include Nix. Sometimes they involve small patches that preserve bash compatibility, like in the case of git completion, but very often they run unmodified (deboostrap, Alpine, Aboriginal, etc.)

---

Looking at the code, here are some patches that could be applied:

• Simply remove -i. You're not using it in this case, and the code will work the same.
  • https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/setup-hooks/auto-patchelf.sh#L144
• Rewrite arithmetic expressions to use $(( )) or (( )) (though the latter has pitfalls with regard to errexit)
  • https://github.com/NixOS/nixpkgs/blob/master/pkgs/stdenv/generic/setup.sh#L379

local -ri outputOffset="$inputOffset + $hostOffset"

=>

local -r outputOffset=$(( inputOffset + hostOffset ))

The latter looks cleaner to me anyway ...

I understand if people don't want to do the work to migrate to OSH. But my goal right now is to add so many other compelling features to OSH that you will want to do this work :)

If there were a ton of other scripts that use -i, then it would be a higher priority, but OSH runs say all of bash completion, and none of that code uses -i, even though it's some of the hairiest bash around. I noticed 3 years ago that Nix does make more use of bash than almost any other Linux distro or shell program, so unfortunately it's a little further from running. (But not very far, AFAICT).

---

Also since you mentioned you were interested in strictness, I would view these patches as making your script more strict.

The -i features involves dynamic parsing, which is bad from a security POV.

Even if we implemented -i, it would be off by default, because it confuses code and data. You would have to set shopt --set parse_dynamic_arith to enable it (which does something right now with respect to $(( ))).

[zimbatm]

Thanks a lot for the detailed explanation and for sharing all that context with me. I can see much better where you are coming from now.

As a Bash user, I thought that the intent of local -i was to mark the variable as being a number. I suspect that's also why people used it in nixpkgs. Ideally, I would like this to act as an assert that fails if the stored string is not also a number. I completely agree that changing the parser dynamically like that is bad and that $(( )) is a clearer notation for arithmetic.

Do you think using local -i as an assert would be an interesting venue for the language?

For now, I will remove the instances of it in nixpkgs. I want to run osh of the packages and am curious to find out how many will break (and hopefully find more compatibility issues).

[andychu]

Using -i as an assert is interesting, but it would only be easy on the declaration.

local -i x=foo  # could check here
x=bar  # harder to check here

But really Oil already provides such asserts with strict_arith (I recommend you set strict:all to find bugs [1].

No error checks from bash:

$ bash -c 'x=badnum; declare -i y=x+42; echo $y'
42
$ bash -c 'x=badnum; declare y=$((x+42)); echo $y'
42

Compatibility, but error checks with Oil:

$ osh -c 'x=badnum; declare y=$((x+42)); echo $y'
42

$ osh -O strict_arith -c 'x=badnum; declare y=$((x+42)); echo $y'
  x=badnum; declare y=$((x+42)); echo $y
                         ^
[ -c flag ]:1: fatal: Invalid integer constant 'badnum'

$ osh -O strict_arith -c 'declare y=$((x+42)); echo $y'
  declare y=$((x+42)); echo $y
               ^
[ -c flag ]:1: fatal: Undefined value in arithmetic context

[1] http://www.oilshell.org/release/latest/doc/oil-options.html

[andychu]

So basically I would view this as an instance where the -i error of OSH alerts you of a way to improve the program:

• you port to a POSIX compatible syntax
• you get static parsing of arithmetic, to find parse errors before you run the program
• you get more runtime errors, shown by strict_arith
• bash does not have parse time or runtime error checks with either syntax.

Similar to what happeneed with mal Lisp:

https://www.oilshell.org/blog/2020/06/release-0.8.pre6.html#patch-to-run-the-mal-lisp

---

Example of parse time errors:

$ bash -c 'f() { local y=$((x + )); }'

< no error until you actually RUN function f >

$ osh -c 'f() { local y=$((x + )); }'
  f() { local y=$((x + )); }
                       ^
[ -c flag ]:1: Token can't be used in prefix position

(the error message could be improved, but at least it's there)

[andychu]

Thanks for trying Oil! I'm closing this now but please let me know what other issues you run into

I am adding things to Oil so this kind of work will be worth it, including rich tracing (xtrace) right now :) Open for discussion on Zulip!