Shell script rewrite to comply with POSIX and best practices

[9ao9ai9ar]

Resolves #1855 Resolves #1446 Fixes #1810

This is not the first POSIX solution to have ever been requested or presented, though certainly the most comprehensive one yet. Regrettably, the robustness goal hasn't truly been realized, so it would need to be addressed in a separate pull request. I'd also like to point out that alternatives like the one in #1696 exist and are worth considering.

Target Platforms for Modern Desktop Versions of Firefox

Based on the supported build hosts and targets of Firefox.

Tier-1

• [x] Windows (WSL, Cygwin, MSYS2, etc.) - Not targeted by the shell scripts.
• [x] macOS - Automated testing only.
• [x] Fedora
• [x] Debian
• [x] Ubuntu

Tier-2

• [x] openSUSE
• [x] Arch
• [x] Alpine
• [x] NixOS - Requires the psmisc or lsof package for prefsCleaner.sh to work.

Tier-3

• [x] FreeBSD
• [x] OpenBSD
• [x] NetBSD
• [x] Solaris - Version 11 and above only.

Non-Tiered

• [x] DragonFly
• [x] OpenIndiana
• [x] Haiku (Iceweasel)

POSIX Compliant Shells

• [x] bash 3 (default macOS /bin/sh) - Automated testing only.
• [x] bash (default Linux and Haiku /bin/sh and user shell)
• [x] dash (default Debian and Ubuntu /bin/sh)
• [x] ash (default BusyBox, FreeBSD and NetBSD /bin/sh and user shell)
• [x] ksh (default SunOS /bin/sh and user shell)
• [x] oksh (default OpenBSD /bin/sh and user shell)
• [x] zsh (default macOS user shell) - Requires emulate sh before sourcing.
• [x] XPG4 sh (alternative Solaris POSIX sh)
• [x] pdksh (alternative NetBSD user shell)
• [x] mksh
• [x] yash
• [x] posh
• [x] gwsh
• [x] bosh - Known issue: Alias is messing with the case statement pattern which results in a syntax error.
• [x] osh - Known issue: https://github.com/oils-for-unix/oils/issues/2228.
• [x] hush - Not targeted for the time being due to missing crucial features: command -p and the noglob shell option.
• [x] gash - Not targeted for the time being due to missing crucial feature: command -p.
• [x] mrsh - Not targeted for the time being due to many deficiencies, e.g. https://github.com/emersion/mrsh/issues/196 and https://github.com/emersion/mrsh/issues/204.

[MagicalDrizzle]

prefCleaner works fine for me, however updater doesn't seem to be able to append overrides.

[MagicalDrizzle]

Seems to be just a broken check? This fixes it and updater seems to work fine now. if ! [ "$SKIPOVERRIDE" ]; > if [ "$SKIPOVERRIDE" = false ];

[MagicalDrizzle]

Error in both scripts - you were checking for the literal file named SCRIPT_FILE:

// from
SCRIPT_FILE=$(preadlink "$0") && [ -f SCRIPT_FILE ] || exit 1
// to
SCRIPT_FILE=$(preadlink "$0") && [ -f $SCRIPT_FILE ] || exit 1

and for your missing /bin/sh question - no it doesn't seem to work.

[JimOswick]

@9ao9ai9ar Thoughts on using #!/usr/bin/env? sh could be busybox, the gimped version of Bash (which is not POSIX-compliant), etc.

[9ao9ai9ar]

@9ao9ai9ar Thoughts on using #!/usr/bin/env? sh could be busybox, the gimped version of Bash (which is not POSIX-compliant), etc.

The POSIX way to do it is to overwrite the shebang with the path of sh as determined by getconf PATH at install-time (see the relevant section in the POSIX document), but I think that is overkill and I would just ask the users to change it themselves if needed. My personal opinion is that /bin/sh works better than /usr/bin/env sh for the shebang due to the following reasons:

1. /usr might not be mounted.
2. env might not be located in /usr/bin.
3. On some systems, at most one argument can be passed to the shebang interpreter, which is already used up in the case of #!/usr/bin/env sh, but in the alternative, we could, for example, turn on tracing by writing #!/bin/sh -x.
4. It is more efficient to execute /bin/sh than /usr/bin/env sh.
5. The result of running /bin/sh is not affected by the value of PATH, unlike /usr/bin/env sh, so there are fewer surprises.
6. The /bin/sh in Solaris 10 and earlier does not comply with POSIX as it predates the standards, but we don't have to worry about that since modern Firefox has only been made available in Solaris beginning with version 11.

[aartoni]

Hey @9ao9ai9ar @MagicalDrizzle I've noticed that progress on this PR is slowing down, so I'm willing to create a checklist of things to ensure that the script is working properly, and then make a GitHub workflow to test the script on different platforms.

Here are some things to be checked:

• [ ] normal execution returns EX_OK
• [ ] check every non-zero EX_*
• [ ] user.js gets updated
• [ ] user-overrides.js is respected

This will tick the GitHub available runners off the list of things that testers have to check manually. Manual tester can then copy the workflow to test locally.

What do you think? An alternative would be splitting this PR into smaller PRs.

[MagicalDrizzle]

@9ao9ai9ar prefsCleaner works, updater only partly - works only if there's already an user.js file. It won't download a new user.js if doesn't exist, then will try to backup the nonexistent user.js file which fails.

+ userjs_backup=/home/demo/Downloads/profile/userjs_backups/user.js.backup.2024-12-01_0511
+ mkdir -p /home/demo/Downloads/profile/userjs_backups
+ cp /home/demo/Downloads/profile/user.js /home/demo/Downloads/profile/userjs_backups/user.js.backup.2024-12-01_0511
cp: cannot stat '/home/demo/Downloads/profile/user.js': No such file or directory
+ userjs_backup=

[9ao9ai9ar]

Are you planning on getting this merged now?

Yes, it's ready for merging.

I'd be interested in contributing a few more changes after this gets merged, such as an extra step for checking for normal execution

You might want to check out ShellSpec. Writing the tests in the workflow file is cumbersome, limiting and a form of vendor lock-in. For example, I can't just run the tests in my VM without setting it up as a self-hosted runner on GitHub or relying on tools such as act, which you mentioned has some differing behaviors from the hosted runners. Ideally, the only workflow step you need after installing and setting up the shells is to call ShellSpec or some command to invoke the test scripts.

[aartoni]

Thanks for the pointers!