Should #!/bin/sh scripts avoid bashisms?

General
1

On a Debian-based distro /bin/sh resolves to dash which does not understand, for example, arrays.

Yet I see scripts - also in KDE - that use #!/bin/sh or #!/usr/bin/env sh but still use such bashisms.

One example from plasma-desktop would be kconf_update/50-krunner-activate-typing.sh which uses arrays and adjacent constructs.

I find it more likely that I’m misunderstanding something here than that 30 years of KDE somehow missed that. Or maybe it’s not a problem after all, IDK.

3 Likes
2

I think you have found a potential issue. As you say sh is not bash and using bash features would be an issue.

There is the shellcheck tool that you can use check the scripts you are concerned about.
Beware that shellcheck is very strict…

3 Likes
3

Unrelated, but this file looks like I could rename it to turn of krunner. :heart_eyes:

Scratch that. Won’t work.
Only an upstream modification can give the user the option again. :smiling_face_with_tear:

4

You actually guessed correctly. Someone wrote it, it caused no issues, nobody cared afterwards. If the script is using bashisms it can safely be changed to #!/usr/bin/env bash. That is a very simple contribution, fast to merge too if you show the ShellCheck output in the MR description itself and wait for the maintainer to confirm the same thing.

6 Likes
5 
shellcheck 50-krunner-activate-typing.sh

In 50-krunner-activate-typing.sh line 43:
        candidates=()
                   ^-- SC3030 (warning): In POSIX sh, arrays are undefined.


In 50-krunner-activate-typing.sh line 45:
            candidates+=("$HOME/.config/plasma-org.kde.plasma.desktop-appletsrc")
            ^--------^ SC3024 (warning): In POSIX sh, += is undefined.
                        ^-- SC3030 (warning): In POSIX sh, arrays are undefined.


In 50-krunner-activate-typing.sh line 48:
            candidates+=("${XDG_CONFIG_HOME}/plasma-org.kde.plasma.desktop-appletsrc")
            ^--------^ SC3024 (warning): In POSIX sh, += is undefined.
                        ^-- SC3030 (warning): In POSIX sh, arrays are undefined.


In 50-krunner-activate-typing.sh line 53:
            while IFS= read -r -d '' f; do
                               ^-- SC3045 (warning): In POSIX sh, read -d is undefined.


In 50-krunner-activate-typing.sh line 54:
                candidates+=("$f")
                ^--------^ SC3024 (warning): In POSIX sh, += is undefined.
                            ^----^ SC3030 (warning): In POSIX sh, arrays are undefined.


In 50-krunner-activate-typing.sh line 55:
            done < <(find "$SEARCH_DIR" -type f -name 'plasma-org.kde.plasma.desktop-appletsrc*' -print0 2>/dev/null)
                   ^-- SC3001 (warning): In POSIX sh, process substitution is undefined.

For more information:
  https://www.shellcheck.net/wiki/SC3001 -- In POSIX sh, process substitution...
  https://www.shellcheck.net/wiki/SC3024 -- In POSIX sh, += is undefined.
  https://www.shellcheck.net/wiki/SC3030 -- In POSIX sh, arrays are undefined.

In the /usr/share/kconf_update directory, it’s only this one script. And according to checkbashisms it’s the only directly KDE-related “wrong shebang”.

I’m guessing the github repo is not the correct place for this, but it leads me to this which tells me how to create an account on invent.kde.org.
On it.

Question:

What does this do? Will krunner break if I link /bin/sh to dash? Since that is the default on Debian-like systems, I’m guessing not?

6 
# kconf update script to migrate users who previously disabled
# activateWhenTypingOnDesktop for KRunner. This script only flips
# users who explicitly set the key to "false" to "true" and
# leaves unset keys untouched.

Did nothing on my system.

7

PR done. I hope I did this right…

1 Like
8

Merge blocked. Hot damn, I have no idea what’s going on.

Clicking any of the Rebase options I get “Failed to rebase: Source branch is protected from force push.”

9

You can’t merge your own merge requests without a developer account, which is something that has to be earned with time. Just be patient; someone will come along to review it soon I’m sure. :slight_smile:

Yeah, that’s because you sent the merge request from the master branch of your fork, rather than a differently-named branch.

To resolve this, un-protect the master branch of your fork in the settings for it in invent.kde.org. That will let you (and also developers who want to merge it) rebase it and force-push to it.

2 Likes
10

Thanks for your reply. I have now unprotected the master branch of my fork, then rebased it.

So now we just wait I guess.

No, I wasn’t expecting this to get merged automatically. I didn’t think long enough before panicking…

11

Cool. If no one else does first, I’ll handle it tomorrow.

1 Like
12

Happens with me a lot…

  1. I start making a script, thinking it will be simple and only call some stuff, so I use sh on top.
  2. Then I end up filling it with multiple checks and forget to change the sh to bash.
1 Like
13

I see some activity on my merge request. Do I have to do anything more or just wait or has it been merged already?

I don’t need a big explanation, just tell me I don’t need to do anything anymore…

14

Just wait and we’ll take it from here!

1 Like