Optimizing bash script (for loops, permissions, ...)

Question

I made this script (very quickly ... :)) ages ago and use it very often, but now I'm interested how bash experts would optimize it.

What it does is it goes through files and directories in the current directory and sets the correct permissions:

#!/bin/bash
echo "Repairing chowns."
for item in "$@"; do
    sudo chown -R ziga:ziga "$item"
done

echo "Setting chmods of directories to 755."
for item in $@; do
    sudo find "$item" -type d -exec chmod 755 {} \;
done

echo "Setting chmods of files to 644."
for item in $@; do
    sudo find "$item" -type f -exec chmod 644 {} \;
done

echo "Setting chmods of scripts to 744."
for item in $@; do
    sudo find "$item" -type f -name "*.sh" -exec chmod 744 {} \;
    sudo find "$item" -type f -name "*.pl" -exec chmod 744 {} \;
    sudo find "$item" -type f -name "*.py" -exec chmod 744 {} \;
done

What I'd like to do is

1. Reduce the number of for-loops
2. Reduce the number of find statements (I know the last three can be combined into one, but I wonder if it can be reduced even further)
3. Make script accept parameters other than the current directory and possibly accept multiple parameters (right now it only works if I cd into a desired directory and then call bash /home/ziga/scripts/repairPermissions.sh .). NOTE: the parameters might have spaces in the path.

Answer 1

a) you are looping through $@ in all cases, you only need a single loop. a1) But find can do this for you, you don't need a bash loop. a2) And chown can take multiple directories as arguments.

b) Per chw21, remove the sudo for files you own.

c) One exec per found file/directory is expensive, use xargs.

d) Per chw21, combine the last three finds into one.

#!/bin/bash
echo "Repairing permissions."
sudo chown -R ziga:ziga "$@"
find "$@" -type d -print0 | xargs -0 --no-run-if-empty chmod 755
find "$@" -type f -print0 | xargs -0 --no-run-if-empty chmod 644
find "$@" -type f \
   \( -name '*.sh' -o -name '*.pl' -o -name '*.py' \) \
   -print0 | xargs -0 --no-run-if-empty chmod 744

This is 11 execs (sudo, chown, 3 * find/xargs/chmod) of other processes (if the argument list is very long, xargs will exec chmod multiple times).

This does however read the directory tree four times. The OS's filesystem caching should help though.

Edit: Explanation for why xargs is used in answer to chepner's comment:

Imagine that there is a folder with 100 files in it. a find . -type f -exec chmod 644 {} \; will execute chmod 100 times.

Using find . -type f -print0 | xargs -0 chmod 644 execute xargs once and chmod once (or more if the argument list is very long).

This is three processes started compared to 101 processes started. The resources and time (and energy) needed to execute three processes is far less.

Edit 2: Added --no-run-if-empty as an option to xargs. Note that this may not be portable to all systems.

Answer 2

I assume that you are ziga. This means that after the first chown command in there, you own every file and directory, and I don't see why you'd need any sudo after that.

You can combine the three last finds quite easily:

find "$item" -type f \( -name "*.sh" -o -name "*.py" -o -name "*.pl" \) -exec chmod 744 {} \;

Apart from that, I wonder what kind of broken permissions you tend to find. For example, chmod does know the +X modifier which only sets the x if at least one of user, group, or other already has a x.

Answer 3

You can simplify this:

for item in "$@"; do

To this:

for item; do

That's right, the default values for a for loop are taken from "$@".

---

This won't work well if some of the directories contain spaces:

for item in $@; do

Again, replace with for item; do. Same for all the other loops.

---

As the other answer pointed out, if you are running this script as ziga, then you can drop all the sudo except in the first loop.

Answer 4

You may use the idiom for item instead of for item in "$@".
We may reduce the use of find to just once (with a double loop). I am assuming that ziga is your "$USER" name. Bash 4.4 is required for the readarray with the -d option.

#!/bin/bash
user=$USER

for i
do
    # Repairing chowns.
    sudo chown -R "$user:$user" "$i"
    readarray -d '' -t line< <(sudo find "$i" -print0)
    for f in "${line[@]}"; do
        # Setting chmods of directories to 755.
        [[ -d $f ]] && sudo chmod 755 "$f"
        # Setting chmods of files to 644.
        [[ -f $f ]] && sudo chmod 644 "$f"
        # Setting chmods of scripts to 744.
        [[ $f == *.@(sh|pl|py) ]] && sudo chmod 744 "$f"
    done
done

If you have an older bash (2.04+), change the readarray line to this:

while IFS='' read -d '' line; do arr+=("$line"); done < <(sudo find "$i" -print0)

I am keeping the sudo assuming the "items" in "$@" migth be repeated inside searched directories. If there is no repetition, sudo may be omited.

There are two inevitable external commands (sudo chown) per loop. Plus only one find per loop. The other two external commands (sudo chmod) are reduced to only those needed to efect the change you need.

But the shell is allways very very slow to do its job.

So, the gain in speed depends on the type of files where the script is used.

Do some testing with time script to find out.

Answer 5

Subject to a few constraints, listed below, I would expect something similar to the following to be faster than anything mentioned thus far. I also use only Bourne shell constructs:

#!/bin/sh

set -e

per_file() {
    chown ziga:ziga "$1"
    test -d "$1" && chmod 755 "$1" || { test -f "$1" && chmod 644 "$1"; }
    file "$1" | awk -F: '$2 !~ /script/ {exit 1}' && chmod 744 "$1"
}

if [ "$process_my_file" ]
then
    per_file("$1")
    exit
fi

export process_my_file=$0

find "$@" -exec $0  {} +

The script calls find(1) on the command-line arguments, and invokes itself for each file found. It detects re-invocation by testing for the existence of an environment variable named process_my_file. Although there is a cost to invoking the shell each time, it should be dwarfed by not traversing the filesystem.

Notes:

• set -e to exit on first error
• + in find to get parallel execution
• file(1) to detect a script
• chown(1) not invoked recursively
• symbolic links not followed

Calling file(1) for every file to determine if it's a script is definitely more expensive than just testing the name, but it's also more general. Note that awk is testing only the description text, not the filename. It would misfire if the filename contained a colon.

You could lift chown back up out of the per_file() function, and use its -R option. That might be faster, as it's one execution. It would be an interesting experiment.

In terms of your requirements, there are no loops, and only one call to find(1). You also mentioned,

right now it only works if I cd into a desired directory

but that's not true. It also works if you mention the target directory on the command line,

$ fix_tree_perms my/favorite/directory

You could add, say, a -d option to change there first, but I don't see how that would be easier to use.