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

Fix shell in optimize-images script #88

Merged
merged 2 commits into from
Nov 7, 2024
Merged

Conversation

Pyker
Copy link
Contributor

@Pyker Pyker commented Nov 7, 2024

I tried to run this script with the hashbang as it was and, at the very least in Ubuntu 24.04, sh resolves to dash, which causes the script to run without processing any images, creating just the images-optimized file, with no errors or output. Changing it to use bash made it work correctly.

My best guess is in the if [ "$FILE" -nt "$DATA"/images-optimized ] check, which has a very subtle but important difference between bash and dash:

bash:

file1 -nt file2

True if file1 is newer (according to modification date) than file2, or if file1 exists and file2 does not.

dash:

file1 -nt file2

True if file1 and file2 exist and file1 is newer than file2.

Since images-optimized doesn't exist on the first run, that condition fails silently until the end of the first run in dash, and subsequent runs won't process old images, only new ones.

Regardless, changing this to bash should avoid more surprises like this in the future.

Alternatively, if you wish to keep it using dash, then it could be edited to take into account those differences, but the hashbang should probably be changed to specify dash, since sh can vary:

#!/usr/bin/env dash
(...)
    if [ ! -f "$DATA"/images-optimized ] || [ "$FILE" -nt "$DATA"/images-optimized ]; then
(...)

If a filename has another dot in it, for example, "*.ls.png", it would grab "ls.png" instead of just "png". This commit fixes that.
@Pyker
Copy link
Contributor Author

Pyker commented Nov 7, 2024

I've also added the extension detection fix if a file has dots in the name, because I ran into that just now (with filenames from Gotify). This one behaves the same between dash and bash, however.

$ export f='a.ls.png'; printf "%s\n" "${f##*.}"
png
$ export f='a.ls.png'; printf "%s\n" "${f#*.}"
ls.png

Copy link
Member

@jmattheis jmattheis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jmattheis jmattheis merged commit b51f02d into gotify:master Nov 7, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants