2

I'm newbie to bash scripting, made this script work as I expected though it's still a first draft:

#!/bin/bash find /var/www/site.com/ -type f -name "*.log" | xargs bash -c ' for name; do parent=${name%/*} parent=${parent##*/} destdir=$(dirname $name) current_year=$(date +"%Y") if [[ "$name" =~ _([0-9]{4})- ]] && [[ "$name" != *"$current_year"* ]]; then year="${BASH_REMATCH[1]}" else continue fi if [ ! -d "$destdir/$year/$parent" ]; then mkdir -p "$destdir/$year/" fi mv "$name" "$destdir/$year" done ' find /var/www/site.com/ -type d -name "20*" | xargs bash -c ' for dir; do tar_name=$(echo $dir | grep -Eo '[0-9]{4}') tar cvfj '$tar_name'.tbz $dir done ' exit 

as you can see I'm invoking bash twice inside a bash script . Is that a bad practice ? makes no / little sense ? Or I'd better off creating an array with the output results of find commands and iterate through it? Any suggestion would be appreciated. Thanks in advance. P.S. Sorry for missing indentation

1

2 Answers 2

2

Even without looking inside the inner shell script, some things to note:

Plain xargs will fail you if the filenames contain whitespace, quotes or backslash characters. It has its own set of quoting rules incompatible with mostly everything else. (I can't find a reference question right now.)

You're missing the "zeroth" argument to the inline shell script, which will cause the first filename to be skipped.

Also, the syntax highlighting reveals that your second command is broken, the single quotes in grep -Eo '[0-9]{4}' close and reopen the quotes surrounding the whole script.


Now, the issue with xargs is most easily fixed with find ... -print0 | xargs -0 command... which are supported in many implementations (GNU, FreeBSD, Busybox).

To fix the issue with the zeroth argument, you need to add a dummy argument (which ends up in $0), e.g.

... | xargs -0 bash -c '...' sh 

Or, you could also use find ... -exec bash -c '...' sh {} +.

But it's probably better to just avoid launching another shell. In Bash, you could do:

find ... -print0 | while IFS= read -r -d '' file; do something with "$file" done 

or,

while IFS= read -r -d '' file; do something with "$file" done < <(find ... -print0) 

The latter works even if you need to set variables within the loop and expect them to be available outside it.

See the relevant parts of Why is looping over find's output bad practice? (You're not using a command substitution, so not all of it is relevant.)

To put the output of find into an array, use

mapfile -t -d '' array < <(find ... -print0) 
    2

    Assuming you have bash ≥4.3, you can use recursive globbing instead of calling find. Calling find isn't wrong (if you do it right: see below), but it's less convenient when recursive globbing will do. You need to call shopt -s globstar to enable recursive globbing.

    shopt -s globstar for name in /var/www/site.com/**/*.log; do … done for dir in /var/www/site.com/**/20*/; do … done 

    If you do use find, never pipe find into xargs: they use different syntax to separate file names, so this fails with names containing whitespace or \'". Either use -exec to invoke a shell or use find … -print0 | xargs -0. See Why does my shell script choke on whitespace or other special characters? for more information.

      You must log in to answer this question.

      Start asking to get answers

      Find the answer to your question by asking.

      Ask question

      Explore related questions

      See similar questions with these tags.