Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Updater.sh shell agnostic #1855

Open
asimovc opened this issue Jun 17, 2024 · 7 comments · May be fixed by #1908
Open

Make Updater.sh shell agnostic #1855

asimovc opened this issue Jun 17, 2024 · 7 comments · May be fixed by #1908
Labels

Comments

@asimovc
Copy link

asimovc commented Jun 17, 2024

My distros use busybox ash and the Updater.sh is using bash to work. I think making shell agnostic is better for just works in all systems.

Wha i expect is it just run as normal and i get this instead.

env: can't execute 'bash': No such file or directory

@yusuf-daglioglu
Copy link

Not all bash scripts are compatible with Shell: https://www.gnu.org/software/bash/manual/html_node/Shell-Compatibility-Mode.html

Therefore all script file should tested before run other interpreters.

But ash is a shell implementation. Most of OS put /bin/sh to reference to default interpreter (which in your case /bin/ash). So for temporary solution on your local machine you try to change first line:

https://github.com/arkenfox/user.js/blob/master/updater.sh#L1

as:

#!/bin/sh

or

#!/usr/bin/env sh

And run the script directly from your terminal. Then you can tell us the results please :) You will be the first tester :)

@sertonix
Copy link

sertonix commented Jul 9, 2024

The current scripts uses a lot more bash extensions than supported by busybox ash. You check that with shellcheck 0.10.0+ quit easily: shellcheck -s busybox -S error -e SC3036 *.sh

Compatibility with more shells would only work when using a simpler standard like POSIX shell.

@asimovc
Copy link
Author

asimovc commented Jul 10, 2024

@yusufdaglioglu when i put #!/usr/bin/env sh i get this

./updater.sh: line 20: syntax error: bad substitution
./updater.sh: line 21: syntax error: bad substitution

@MagicalDrizzle
Copy link
Contributor

checkbashisms and shellcheck would be useful if anyone is willing to port the script. (former is just a perl script, latter is both cross-platform and has a browser version).
currently the script does make use of select which doesn't have any pure sh equivalent, so unfortunately it's not trivial...

@9ao9ai9ar
Copy link

9ao9ai9ar commented Oct 16, 2024

@MagicalDrizzle Most of the fixes are trivial, I've got it down to five blocks of code that need a bit more thinking, select being one of them but should be simple as it just needs a hand-rolled implementation. There are some convoluted logic, so for the first commit I would just port it as-is as much as possible. I would also need some testers, especially for macOS as I don't have easy access to it.

@MagicalDrizzle
Copy link
Contributor

@9ao9ai9ar I would be willing to be a tester for BusyBox. (plus the windows port and dash too for good measure)

@9ao9ai9ar 9ao9ai9ar linked a pull request Oct 17, 2024 that will close this issue
34 tasks
9ao9ai9ar added a commit to 9ao9ai9ar/user.js that referenced this issue Nov 30, 2024
The rewrite focuses on the following five areas of interest:
1. Portability. The scripts should be useable across a number of Unix
   operating systems and shells; goes hand in hand with POSIX-
   compliance.
2. Robustness. Fail early; borrow from more battle-tested open source
   code; pass all valid ShellCheck checks.
3. Composability. Put everything inside functions; make the scripts
   dot source friendly.
4. Consistency. Abstract away terminal color codes with tput;
   uniform diagnostic messages and standardized use of status codes
   and redirections.
5. Readability. Extensive comments and descriptive names; use here-
   docs to ease writing multiline messages.

Known behavioral changes:
1. Changed the way the options are parsed and acted on.
   For example, when both the -p and -l options of updater.sh are
   specified, -l will be ignored. The old behavior would depend on the
   order of the options passed, where the last one wins, and the
   profile path passed as the argument to -p couldn't be named 'list'
   or it would be treated as if the option -l was specified.
2. All temporary files are created using mktemp, so users won't find
   them in the working directory anymore should an error occur and
   they were not removed as a result of that.
9ao9ai9ar added a commit to 9ao9ai9ar/user.js that referenced this issue Feb 24, 2025
The rewrite is focused on the following five areas of interest:
1. Portability. The scripts have been tested to work in recent versions
   of the following operating systems and shells: macOS, Linux (Fedora,
   Debian, Ubuntu, openSUSE, Arch, Alpine, NixOS), BSD (FreeBSD,
   OpenBSD, NetBSD, DragonFly), SunOS (Solaris, OpenIndiana), Haiku;
   bash, dash, ash, ksh, oksh, zsh, XPG4 sh, pdksh, mksh, yash, posh,
   gwsh, bosh, osh.
2. Robustness. Employ secure shell scripting techniques, incorporate
   battle-tested open source code, clear all ShellCheck warnings, and
   fail early.
3. Composability. Put (almost) everything inside functions and make the
   scripts dot source friendly.
4. Consistency. Use tput to abstract away terminal color codes, write
   templated diagnostic messages and follow conventions in the use of
   exit status and redirections.
5. Readability. Comment extensively, assign descriptive names to
   variables and functions, and use here-documents to ease reading and
   writing multi-line messages.

Known behavioral changes:
1. There are changes to the way some options are parsed and acted on.
   For example, when both the -l and -p options are specified, -l will
   be ignored; in the old behavior, the last specified option would
   take effect. Also, an old quirk where passing the argument 'list' to
   -p was equivalent to specifying the -l option has been fixed.
2. The -h, -l and -p options of updater.sh have been added to
   prefsCleaner.sh as well.
3. All temporary files are now created using mktemp and no longer
   actively deleted, so users won't find them in the working directory
   anymore in the case of error.
4. The old prefs.js cleaning logic, which relied on non-POSIX features,
   is not preserved in the rewrite.

Resolves arkenfox#1855
Resolves arkenfox#1446
Fixes arkenfox#1810
@9ao9ai9ar
Copy link

Now this looks ugly with the dangling commits. Apologies for my inexperience with Git and GitHub and the inconvenience/frustrations the forced-pushes may have caused you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

6 participants