[ci, tools] shellcheck-ed #397

Closed
DraVee wants to merge 8 commits from DraVee:old/shellcheck into master
Member
  • update-icons.sh/source.sh now checks for dependencies the correct way

  • add --help to license-header.sh

  • nuke tools/reset-submodules and any ref on ci/tools

  • add missing shebang to .ci/windows/package.sh

  • all script tools on root of ./ci are now POSIX-compliant

  • Below the error/warnings/infos from shellcheck:

-- SC2068 (error): Double quote array expansions to avoid re-splitting elements.

-- SC3054 (warning): In POSIX sh, array references are undefined.
-- SC2155 (warning): Declare and assign separately to avoid masking return values.
-- SC2046 (warning): Quote this to prevent word splitting.

-- SC2236 (style): Use -n instead of ! -z.
-- SC2006 (style): Use $(...) notation instead of legacy backticks ....
-- SC2001 (style): See if you can use ${variable//search/replace} instead.

-- SC2086 (info): Double quote to prevent globbing and word splitting.
-- SC2185 (info): Some finds don't have a default path. Specify '.' explicitly.

Signed-off-by: Caio Oliveira caiooliveirafarias0@gmail.com

* update-icons.sh/source.sh now checks for dependencies the correct way * add --help to license-header.sh * nuke `tools/reset-submodules` and any ref on ci/tools * add missing shebang to .ci/windows/package.sh * all script tools on root of `./ci` are now POSIX-compliant * Below the error/warnings/infos from shellcheck: -- SC2068 (error): Double quote array expansions to avoid re-splitting elements. -- SC3054 (warning): In POSIX sh, array references are undefined. -- SC2155 (warning): Declare and assign separately to avoid masking return values. -- SC2046 (warning): Quote this to prevent word splitting. -- SC2236 (style): Use -n instead of ! -z. -- SC2006 (style): Use $(...) notation instead of legacy backticks `...`. -- SC2001 (style): See if you can use ${variable//search/replace} instead. -- SC2086 (info): Double quote to prevent globbing and word splitting. -- SC2185 (info): Some finds don't have a default path. Specify '.' explicitly. Signed-off-by: Caio Oliveira <caiooliveirafarias0@gmail.com>
crueter reviewed 2025-09-09 05:47:41 +02:00
@ -16,0 +39,4 @@
exit 0
fi
RANGE="$(echo "$COMMITS" | tail -n1)^..$(echo "$COMMITS" | head -n1)"
FILES=$(git diff-tree --no-commit-id --name-only "${RANGE}" -r)
Owner

Let's just change this to use two commands. There's another PR open that has it I think

Let's just change this to use two commands. There's another PR open that has it I think
Author
Member

I'm gonna search for this PR later (power outage)

I'm gonna search for this PR later (power outage)
Author
Member

It's on #296, gonna check the difference later

It's on #296, gonna check the difference later
DraVee marked this conversation as resolved
.ci/translate.sh Outdated
@ -6,3 +9,3 @@
# requires fd
SOURCES=`fd . src/yuzu -tf -e ui -e cpp -e h -e plist`
SOURCES=$(fd . src/yuzu -tf -e ui -e cpp -e h -e plist)
Owner

Might be worth converting to POSIX find, I made this script in 5 seconds and was too lazy to fix

Might be worth converting to POSIX find, I made this script in 5 seconds and was too lazy to fix
Author
Member

This one is just style fix

This one is just style fix
Author
Member

find src/yuzu -type f \( -name '*.ui' -o -name '*.cpp' -o -name '*.h' -o -name '*.plist' \)

Will change it later

`find src/yuzu -type f \( -name '*.ui' -o -name '*.cpp' -o -name '*.h' -o -name '*.plist' \)` Will change it later
DraVee marked this conversation as resolved
Owner

In general, we need to move the CI scripts from the workflow repo. I already did this with windows in #348

In general, we need to move the CI scripts from the workflow repo. I already did this with windows in #348
Author
Member

@crueter wrote in #397 (comment):

In general, we need to move the CI scripts from the workflow repo. I already did this with windows in #348

I'm planning to do it, but first I want to see if (#349) didn't break compilation on windows

@crueter wrote in https://git.eden-emu.dev/eden-emu/eden/pulls/397#issuecomment-3307: > In general, we need to move the CI scripts from the workflow repo. I already did this with windows in #348 I'm planning to do it, but first I want to see if (#349) didn't break compilation on windows
DraVee force-pushed old/shellcheck from d4e000fc48 to 671b17ecb4 2025-09-09 22:32:33 +02:00 Compare
DraVee force-pushed old/shellcheck from 584e8419e7 to 3428fe4716 2025-09-09 23:25:54 +02:00 Compare
Author
Member

This supersede #349 and make all other scripts on ./ci POSIX-compliant

This supersede #349 and make all other scripts on `./ci` POSIX-compliant
requested review from Lizzie 2025-09-09 23:42:54 +02:00
Author
Member

@Lizzie could you check this on BSD?

@Lizzie could you check this on BSD?
Member

@DraVee wrote in #397 (comment):

@Lizzie could you check this on BSD?

applies it to every file now...

@DraVee wrote in https://git.eden-emu.dev/eden-emu/eden/pulls/397#issuecomment-3365: > @Lizzie could you check this on BSD? applies it to every file now...
Author
Member

@Lizzie wrote in #397 (comment):

@DraVee wrote in #397 (comentário):

@Lizzie could you check this on BSD?

applies it to every file now...

sorry, I didn't understand

@Lizzie wrote in https://git.eden-emu.dev/eden-emu/eden/pulls/397#issuecomment-3366: > @DraVee wrote in #397 (comentário): > > > @Lizzie could you check this on BSD? > > applies it to every file now... sorry, I didn't understand
Member

@DraVee wrote in #397 (comment):

@Lizzie wrote in #397 (comment):

@DraVee wrote in #397 (comentário):

@Lizzie could you check this on BSD?

applies it to every file now...

sorry, I didn't understand

It now changes EVERY file instead of just the changed ones

@DraVee wrote in https://git.eden-emu.dev/eden-emu/eden/pulls/397#issuecomment-3367: > @Lizzie wrote in #397 (comment): > > > @DraVee wrote in #397 (comentário): > > > @Lizzie could you check this on BSD? > > > > > > applies it to every file now... > > sorry, I didn't understand It now changes EVERY file instead of just the changed ones
Author
Member

@Lizzie I will need to install BSD to see what's broken, but this should fix (i think?)

@Lizzie I will need to install BSD to see what's broken, but this should fix (i think?)
Owner

If we go with grep instead let's also add shell scripts to the cmake check

grep -Fxq "$HEADER_HASH" "$1" || BAD_HASH="$BAD_HASH $1"
echo "license-header.sh: The following CMake and shell script files files have incorrect license headers:"

use .sh and .ps1

If we go with grep instead let's also add shell scripts to the cmake check ```sh grep -Fxq "$HEADER_HASH" "$1" || BAD_HASH="$BAD_HASH $1" ``` ``` echo "license-header.sh: The following CMake and shell script files files have incorrect license headers:" ``` use .sh and .ps1
DraVee force-pushed old/shellcheck from 194eae51e6 to dfb11f1532 2025-09-11 05:20:27 +02:00 Compare
DraVee force-pushed old/shellcheck from dfb11f1532 to b379c73583 2025-09-11 05:22:28 +02:00 Compare
DraVee force-pushed old/shellcheck from b379c73583 to 51a5ed1697 2025-09-11 05:25:30 +02:00 Compare
DraVee force-pushed old/shellcheck from 51a5ed1697 to 6bbc8e7885 2025-09-11 05:28:43 +02:00 Compare
Author
Member

@crueter should be done now, basically rewrite the entire thing

@crueter should be done now, basically rewrite the entire thing
DraVee force-pushed old/shellcheck from 6bbc8e7885 to 3f90405749 2025-09-13 00:49:16 +02:00 Compare
crueter requested changes 2025-09-13 00:56:56 +02:00
@ -23,0 +47,4 @@
COMMENT_TYPE="$1"
FILE="$2"
HEADER_LINE1=$(printf '%s\n' "$HEADER_LINE1_TEMPLATE" | sed "s|{COMMENT_TEMPLATE}|$COMMENT_TYPE|g")
Owner

This is kind of a mess, I'll look at it later when on pc

This is kind of a mess, I'll look at it later when on pc
Author
Member

Well I couldn't think of another way to make it search for the multi-line, and variable substitution is not supported on sh only on bash, so I used this mess

Well I couldn't think of another way to make it search for the multi-line, and variable substitution is not supported on sh only on bash, so I used this mess
Owner

First, it's probably better to just prepend the comment type instead of templating.

Second, if we head -n5 the content, then we can grep -A1 -e "$HEADER_LINE1" "$HEAD" | grep -B1 "$HEADER_LINE2" which should be a nearly instantaneous and much cleaner.

First, it's probably better to just prepend the comment type instead of templating. Second, if we head -n5 the content, then we can `grep -A1 -e "$HEADER_LINE1" "$HEAD" | grep -B1 "$HEADER_LINE2"` which should be a nearly instantaneous and much cleaner.
Author
Member

After searching, the commands -A and -B are GNU extensions, I will look into it later (probably will use awk)

After searching, the commands `-A` and `-B` are GNU extensions, I will look into it later (probably will use awk)
DraVee marked this conversation as resolved
DraVee added 1 commit 2025-09-13 01:44:57 +02:00
[license] remove printf usage
All checks were successful
eden-license / license-header (pull_request) Successful in 33s
4e05da5998
Signed-off-by: Caio Oliveira <caiooliveirafarias0@gmail.com>
DraVee force-pushed old/shellcheck from 4e05da5998 to 2f0c9eb9ba 2025-09-15 01:42:44 +02:00 Compare
DraVee force-pushed old/shellcheck from 2f0c9eb9ba to 51f58c695f 2025-09-16 03:23:23 +02:00 Compare
requested review from crueter 2025-09-18 02:29:15 +02:00
DraVee force-pushed old/shellcheck from 51f58c695f to e8d03a13e7 2025-09-18 02:29:31 +02:00 Compare
Author
Member

Gonna open a new PR just to changes on license-header.sh

Gonna open a new PR just to changes on license-header.sh
DraVee closed this pull request 2025-09-25 08:10:25 +02:00
All checks were successful
eden-license / license-header (pull_request) Successful in 42s
Required
Details

Pull request closed

Sign in to join this conversation.
No description provided.