CODEWITHSUNDEEP
bashshellif-statementfor-loopscripting
BattiestFawn66
BattiestFawn66

Reputation: 79

Bash scripting: syntax error near unexpected token `done'

I am new to bash scripting and I have to create this script that takes 3 directories as arguments and copies in the third one all the files in the first one that are NOT in the second one.

I did it like this:

#!/bin/bash

if [ -d $1 && -d $2 && -d $3 ]; then
    for FILE in [ ls $1 ]; do
        if ! [ find $2 -name $FILE ]; then
            cp $FILE $3
    done
else echo "Error: one or more directories are not present"
fi

The error I get when I try to execute it is: "line 7: syntax error near unexpected token `done' " I don't really know how to make it work! Also even if I'm using #!/bin/bash I still have to explicitly call bash when trying to execute, otherwise it says that executing is not permitted, anybody knows why? Thanks in advance :)

Upvotes: 0

Views: 7829

Answers (4)

Manuel Alcocer
Manuel Alcocer

Reputation: 131

this is not totally correct:

for FILE in $(ls $1); do 
    < whatever you do here >
done

There is a big problem with that loop if in that folder there is a filename like this: 'I am a filename with spaces.txt'. Instead of that loop try this:

for FILE in "$1"/*; do
    echo "$FILE"
done

Also you have to close every if statement with fi.

Another thing, if you are using BASH ( #!/usr/bin/env bash ), it is highly recommended to use double brackets in your test conditions:

if [[ test ]]; then
    ...
fi

For example:

$ a='foo bar'
$ if [[ $a == 'foo bar' ]]; then
> echo "it's ok"
> fi
it's ok

However, this:

$ if [ $a == 'foo bar' ]; then
> echo "it's ok"; 
> fi
bash: [: too many arguments

Upvotes: 1

sjsam
sjsam

Reputation: 21965

Couple of suggestions :

  1. No harm double quoting variables

    cp "$FILE" "$3" # prevents wordsplitting, helps you filenames with spaces

  2. for statement fails for the fundamental reason -bad syntax- it should've been:

    for FILE in ls "$1";

  3. But then, never parse ls output. Check [ this ].

     for FILE in ls "$1"; #drastic

  4. Instead of the for-loop in step2 use a find-while-read combination:

    find "$1" -type f -print0 | while read -rd'' filename #-type f for files
do
#something with $filename
done
  5. Use lowercase variable names for your script as uppercase variables are reserved for the system. Check [this].
  6. Use tools like [ shellcheck ] to improve script quality.

Edit

Since you have mentioned the input directories contain only files, my alternative approach would be

[[ -d "$1" && -d "$2" && -d "$3" ]] && for filename in "$1"/*
do
    [ ! -e "$2/${filename##*/}" ] && cp "$filename" "$3"
done

If you are baffled by ${filename##*/} check [ shell parameter expansion ].

Sidenote: In linux, although discouraged it not uncommon to have non-standard filenames like file name.
Courtesy: @chepner & @mklement0 for their comments that greatly improved this answer :)

Upvotes: 3

Kusalananda
Kusalananda

Reputation: 15613

Your script:

if ...; then
  for ...; do
    if ...; then
      ...
  done
else
  ...
fi

Fixed structure:

if ...; then
  for ...; do
    if ...; then
      ...
    fi       # <-- missing
  done
else
  ...
fi

If you want the script executable, then make it so:

$ chmod +x script.sh

Notice that you also have other problems in you script. It is better written as

dir1="$1"
dir2="$2"
dir3="$3"

for f in "$dir1"/*; do
  if [ ! -f "$dir2/$(basename "$f")" ]; then
    cp "$f" "$dir3"
  fi
done

Upvotes: 1

bipll
bipll

Reputation: 11940

You've forgot fi after the innermost if.

Additionally, neither square brackets nor find do work this way. This one does what your script (as it is now) is intended to on my PC:

#!/bin/bash

if [[ -d "$1" && -d "$2" && -d "$3" ]] ; then
    ls -1 "$1" | while read FILE ; do
        ls "$2/$FILE" >/dev/null 2>&1 || cp "$1/$FILE" "$3"
    done
else echo "Error: one or more directories are not present"
fi

Note that after a single run, when $2 and $3 refer to different directories, those files are still not present in $2, so next time you run the script they will be copied once more despite they already are present in $3.

Upvotes: -1

Related Questions