cheat.sh icon indicating copy to clipboard operation
cheat.sh copied to clipboard

Posix sh

Open grayed opened this issue 5 years ago • 7 comments

The portability fans are back! :-)

There were a few things broken already. In particular, a few places had "$foo" instead of $foo, breaking the intended words expansion.

The bash arrays were replaced with either simple expansion, or written directly, resulting in no code quality loss.

The zsh bits are only needed if someone will ever run "zsh cht.sh" and could be dropped; when zsh is working as sh, the shwordsplit option gets set automatically.

The local/typeset/declare case is the most complicated one. Technically, POSIX just reserves "local", "typeset" and "declare" for use by shell, keeping them as undefined behavior. All the shells I've tested with (bash, dash, ksh93, OpenBSD ksh, zsh) support either "local" or "typeset" command. But for better compatibility sakeness, since it was meaningful enough for 884161c4c7a3ccf6eb4c6ed226ed1104b49614ef, the "eval" fallback was implemented. This makes "local foo" invalid, thought, so I had to replace all of them with "local foo=" instead. But the better option likely be just ignoring shells that do not support local-scoped variables, since the code is likely to break in this case anyway.

grayed avatar Oct 03 '20 17:10 grayed

@grayed Vadim welcome back! I'm very very (very) glad to see you again.

Thank you very much for the fixes (have you run the tests?), and I am (again) very glad that you are back, additionally to all other reasons because I plan to do a crazy thing, where a fresh look of a shell-portablity expert would be really helpful :)

chubin avatar Oct 04 '20 22:10 chubin

Nope, I've totally missed the fact that tests exist. Is there any reason to not run them in GitHub CI? If not, will you accept a PR that adds appropriate GitHub actions then?

grayed avatar Oct 05 '20 19:10 grayed

Here are results of running tests on OpenBSD: https://gist.github.com/grayed/4257f05f399df44753a3619ce7d18101

Before tweaks from the PR there was no success at all, BTW, due to broken arguments passing.

The results vary from run to run, sometimes there are 9 successfull tests, sometimes 8. I didn't dig into them, though, since there is too much Python for me now. :-\

grayed avatar Oct 05 '20 19:10 grayed

since there is too much Python for me now

You didn't guess! This crazy test (and it is actually, just one test, but with several test cases) is written in bash. The main problem here, as far as I can judge, that if you start it in the server mode, it does not work (probably because of some problems that greenlet has in OpenBSD). But you can start in the "standalone" mode (see inside the script how you can switch it), so that no greenlets will be used. Then we will see what is missing

Regarding your question, why the test is not running in CI, you are absolutely right, and the only one who is guilty is me. I believe your question will be the final drop that will make me to fix it

chubin avatar Oct 05 '20 20:10 chubin

Well, the tests are run by shell script, yes, but they try to run Python code, and it looks like something doesn't work there well. Still do not have enough time to look throughly. :(

I've sent another PR, with GitHub action draft, maybe it'll help. :)

grayed avatar Oct 07 '20 22:10 grayed

$Thank you for the fixes; actually we have CI, and eactly these tests are running there (we use travis-ci), but there are some minor fixes to be done (#224). I hope to fix it first, and then we can continue with this pull request

chubin avatar Oct 12 '20 11:10 chubin

Well, this needs to be rebased to become mergeable. :D

abitrolly avatar Nov 13 '21 18:11 abitrolly