Bash script to minify PNG's keeps stopping

Question

I have tried to run this script a few times now and it keeps hanging. I have 60,000 images and it goes through about a 1000 and then stops. I have been running the top command and the process pngquant dissapears it seems. Any help would be much appreciated.

The last output I got was

Optimized PNG: ./12/thumbnail/112.png
Optimized PNG: ./12/thumbnail/photography.png
Optimized PNG: ./12/thumbnail/Christmas-01.png
Optimized PNG: ./12/thumbnail/OA1920.png
error: cannot open ./12/thumbnail/The for reading
Optimized PNG: ./12/thumbnail/The
error: cannot open Official for reading
Optimized PNG: Official
error: cannot open AndreasCY for reading
Optimized PNG: AndreasCY

And then it stopped. The script looks like this.

#!/bin/bash

cd /var/www/html/wp-content/uploads/2014/

for PNG in $(find . -name '*.png'); do

  pngquant --ext .png --force 256 ${PNG}
  echo "Optimized PNG: ${PNG}"

done

Answer 1

Rui’s answer contains some good advice, although he seems to have missed the actual cause of the failure of your command.  It appears from the output that you show that you have a file named ./12/thumbnail/The Official AndreasCY ….png.

When you say

for PNG in $(find . -name '*.png')

it’s like saying

for PNG in ./12/thumbnail/112.png ./12/thumbnail/photography.png ./12/thumbnail/Christmas-01.png \
           ./12/thumbnail/OA1920.png ./12/thumbnail/The Official AndreasCY ….png  …

with the result that ./12/thumbnail/The, Official, and AndreasCY get treated as if they were separate filenames — and, since they aren’t, you get the error you got.  You should be very careful of using $(…) to generate a list of filenames — by which I mean, avoid doing that if at all possible.  find … -exec is a much better approach.  However, the command Rui gave won’t work: -exec "Optimized PNG: " will fail because there’s no such command as "Optimized PNG: ".

I suggest that you keep the following parts of your original answer and Rui’s answer:

#!/bin/sh

cd /var/www/html/wp-content/uploads/2014        (You don’t really need the “/” at the end.)
find . -name '*.png' -type f …

and then finish it with one of the following:

• -exec pngquant --ext .png --force 256 {} \; -exec echo "Optimized PNG:" {} \;
• -exec pngquant --ext .png --force 256 {} \; -printf "Optimized PNG: %p\n"
• -exec sh -c 'pngquant --ext .png --force 256 "$1"; echo "Optimized PNG: $1"' sh {} \;, or

• -exec myscript {} \;
  where myscript is

  #!/bin/sh
  pngquant --ext .png --force 256 "$1"
  echo "Optimized PNG: $1"
  

Note that you should always quote shell variables (e.g., "$PNG" and "$1") unless you have a good reason not to, and you’re sure you know what you’re doing.  By contrast, while braces (e.g., ${PNG}) can be important, they’re not as important as quotes.  And, on general principles, I suggest that you use #!/bin/sh unless you're explicitly writing a script that depends on bash features, and will not work with a POSIX-compliant shell — and I advise against that.

Answer 2

In your script, the find output is expanded into the actual command line, which has limitations in size. So, when the arguments fed by find exceed the size, the rest are ignored.

This article deals with problematic: http://www.cyberciti.biz/faq/argument-list-too-long-error-solution/

The find command is also a very potent command, and has an option to invoke commands, and is one of the ways of escaping those limitations when dealing with large files.

You can execute commands as this:

find $DIR -name '*.png' -type f -exec command {} \;

Where {} is the file being handled, type f is to define you are handling a file, and $DIR the base directory where the command is working recursively.

One easy way of doing what you want in one line liner is:

find /var/www/html/wp-content/uploads/2014 . -name '*.png' -type f \ 
-exec pngquant --ext .png --force 256 {} \; -exec echo "Optimized PNG: " {} \;

(\ used to break the command in two lines to add clarity to the post)

I will also leave here an article about using find together with the exec option.

http://linuxaria.com/howto/linux-shell-how-to-use-the-exec-option-in-find-with-examples

Answer 3

Globbing syntax can be put directly into the for loop. You don't need find at all. And if you use globbing directly in the for loop, you won't have to worry about word splitting since the shell takes care of that automatically.

It's also a good idea to quote your shell variables when they appear in commands, or else they will undergo word splitting after substitution.

Finally, prefer printf instead of echo, since it will behave more correctly if your filename has certain characters in it and is more portable across different implementations.

for PNG in *.png; do

  pngquant --ext .png --force 256 "$PNG"
  printf "Optimized PNG: %s\n" "$PNG"

done

In zsh, and bash with the relevant option set, you can do recursive globs like this:

for PNG in **/*.png; do

  pngquant --ext .png --force 256 "$PNG"
  printf "Optimized PNG: %s\n" "$PNG"

done

This will cause the search to descend into subdirectories, just like the find command does. However, this is not POSIX standard, so if you need something to work portably on all POSIX-like systems, you should use one of the other answers.