to bash or not to bash

[fkobi]

Hello,

with the recent inclusion of the distribution files in assets/linux/ I have noticed that some of the scripts were using bash when they could be using just the posix sh.

Here's the current state of the shebangs for sh:

$ rg "^#\!.*sh$"
assets/linux/unregisterBiglyBT
1:#!/usr/bin/env bash

assets/linux/updateBiglyBT
1:#!/usr/bin/env bash

assets/linux/registerBiglyBT
1:#!/usr/bin/env bash

mvnw
1:#!/bin/sh

core/src/com/biglybt/platform/unix/startupScript
1:#!/usr/bin/env bash

for {un,}registerBiglyBT it's trivial but updateBiglyBT & startupScript require more work.

One thing is clear: they are not written for bash -- they are shell scripts with occasional bash syntax which makes them not portable.
Right now we do not the portability (and speed, for people with alternative /bin/sh) of the posix shell nor the speed and the convenience of bash.

I think it should be decided if the project should:

• go all in for bash
• use bash only on Linux (FreeBSD does not ship it by default)
• stick to the posix sh

I am willing to handle all this myself but I do not want my work to go to waste so please decide on the shell policy for this project.

also as a sidenote: Does it make sense to have the unix startup script written for bash?
as far as I know it's the standard only on GNU/Linux.

[TuxPaper]

If I had to make an educated guess, these scripts started off simple posix shell scripts, but as time and went on, different devs went with whatever was easier or what they knew (bash syntax).

I don't have an issue with having them as posix sh, but I'll have to discuss with with parg since he may know something I don't.

[parg]

I knows nothing on this, feel free to do whatever :)

[ferdnyc]

I've been looking at this, and at the two PRs opened (one merged, one in draft). My take is, it doesn't seem to make sense to me that one script be a bash script, while the others are straight Bourne shell scripts.

In fact, that mismatch seems like it would be the worst of both worlds. It increases the likelihood that some user on a system where bash isn't a given (Ubuntu's bundled bare-bones is apparently dash, these days) will now succeed in installing BiglyBT (because the register/unregister scripts are /bin/sh-compatible), but then updates will fail due to a lack of bash to run updateBiglyBT. Previously, when all the scripts used bash, at least a lack of it would surface earlier.

Why not make the update script Bourne-shell compatible as well? The only reason it needs bash currently is because it sucks its arguments into the $PARAMS array variable, and array variables are a Bashism. But the way it uses $PARAMS is ugly and breakable anyway. It looks to me like it can be rewritten for /bin/sh compatibility, and come out more robust as a bonus.

Here's what I'd propose, for the update script:

1. Instead of creating and examining $PARAMS, differentiate the arguments to the script itself from the arguments that just need to be passed to the JRE, using the time-honored tradition of a -- separator argument.
2. Make the script arguments follow shell conventions: --longname options, some of which may accept an argument.
3. Parse the script flags in standard Bourne-shell fashion: With a while loop that shifts each one off the front of the argument list as it's consumed.

So, instead of rerun as the first argument to indicate it's been relaunched, use --rerun. Instead of the original user as the second argument, --origUser <username>. Instead of finding the <userPath> and <appPath> in the Java arguments, pass them (redundantly) as --appPath <appPath> and --userPath <userPath> before the --. (If they're only there for the scripting, and not needed by the Java code, they can also be removed from the positional arguments.)

Whereas an update script invocation today might look like this:

updateBiglyBT -Dsome.property=foo -Dother.property=bar \
  -Dazureus.script.version="1"  com.biglybt.update.Updater \
  restart /usr/lib/biglybt /home/foo/.biglybt

And a second run of the same script through sudo would become:

sudo updateBiglyBT rerun foo -Dsome.property=foo -Dother.property=bar \
  -Dazureus.script.version="1"  com.biglybt.update.Updater \
  restart /usr/lib/biglybt /home/foo/.biglybt

The initial invocation would now be:

updateBiglyBT --appPath /usr/lib/biglybt --userPath /home/foo/.biglybt -- \
  -Dsome.property=foo -Dother.property=bar \
  -Dazureus.script.version="1"  com.biglybt.update.Updater \
  restart /usr/lib/biglybt /home/foo/.biglybt

And if it re-executed itself using sudo it would call:

sudo updateBiglyBT --rerun --origUser foo \
  --appPath /usr/lib/biglybt --userPath /home/foo/.biglybt -- \
  -Dsome.property=foo -Dother.property=bar \
  -Dazureus.script.version="1"  com.biglybt.update.Updater \
  restart /usr/lib/biglybt /home/foo/.biglybt

The script would shift off all of the arguments up to and including --, storing the values in script variables. Then it would run:

java -cp "${USER_PATH}/plugins/azupdater/Updater.jar" "$@"

# and possibly
if [ "${IS_RERUN}" -eq 1 ]; then
    echo "Ensuring ${ORIG_USER} still owns ${USER_PATH}"
    chown -R "${ORIG_USER}:${ORIG_USER}" "${USER_PATH}"
fi

Easy-peasy, and fully /bin/sh-compatible.

[ferdnyc]

We would need a way to coordinate migration of existing installs... Can com.biglybt.update.Updater install new script files, theoretically, in addition to the update JAR?

We could give the new update script a different filename, distribute it to existing users in an update, and have ClientRestarterImpl::restart_Unix tailor the arguments it passes to whichever version of the script it has available to it. That way the scripting changes can be rolled out without breaking anything, and they don't have to be backwards-compatible.

It also provides an escape hatch in case anything goes wrong, since the old script would still be there.

[ferdnyc]

On an unrelated note, is the update script used on macOS as well? Because, I notice it uses realpath... macOS doesn't have a realpath command. (Which is very annoying, and causes endless frustrations when writing cross-platform scripts. That's why I know.)