5

$ sh backup-to-s3.sh

backup-to-s3.sh: 11: [: bkup_20151106_150532.zip: unexpected operator
backup-to-s3.sh: 11: [: bkup_20151106_150532.zip: unexpected operator
backup-to-s3.sh: 11: [: bkup_20151106_150532.zip: unexpected operator
backup-to-s3.sh: 11: [: bkup_20151106_150532.zip: unexpected operator
backup-to-s3.sh: 11: [: bkup_20151106_150532.zip: unexpected operator
backup-to-s3.sh: 11: [: bkup_20151106_150532.zip: unexpected operator

ubuntu@accretive-staging-32gb-ephemeral:~$ cat backup-to-s3.sh

#Script to move /home/ubuntu/backup folder  to S3://auto-backup
#Author Ashish Karpe
cd /mnt/backup
filename="bkup_$(date +%Y%m%d_)"
/bin/ls -alF | awk '{ print $9 }'  > /tmp/file

for i in $(cat /tmp/file); do
#       echo $i;
#       read a;
#       echo $filename;
        if [ $filename* = $i ]
        then
                echo "Copying " $i "to S3://auto-backup";
                s3cmd put $i s3://auto-backup

            fi

done
3
  • Do you need to support filenames with spaces in their name, or not?
    – thrig
    Nov 6, 2015 at 17:44
  • no spaces required in file name Nov 6, 2015 at 17:52
  • I want to compare bkup_20151106_171600.zip with bkup_20151106_* Nov 6, 2015 at 18:19

4 Answers 4

6
  1. don't use for to iterate over the lines of a file, use

    while IFS= read -r line; do ...; done < filename
    
  2. you don't need to pipe ls output to a file at all, especially using -F

  3. use bash [[ x == y ]] for pattern comparisons, and the pattern is on the right-hand side:
#!/bin/bash
cd /mnt/backup
prefix="bkup_$(date +%Y%m%d_)"

for file in * .*; do
    [[ -f $file ]] || continue    # skip things like directories and soft links
    if [[ $file == $prefix* ]]; then
        echo "Copying " $file "to S3://auto-backup";
        s3cmd put $file s3://auto-backup
    fi
done < /tmp/file
6
  • 1
    What are you talking about with [[ x == y ]] "for pattern comparisons". That's not in any manual I have. Maybe you are using a different shell?
    – Otheus
    Nov 6, 2015 at 18:46
  • @Otheus - see here Nov 6, 2015 at 18:56
  • @don_crissti Perhaps you misunderstood my problem with Glenn's statement. It's that you should use [[ x == y ]] for *pattern comparisons*. Your link only confirms to me that this is either wrong or misleading at best.
    – Otheus
    Nov 6, 2015 at 19:05
  • 3
    It's documented right in the bash manual: When the ‘==’ and ‘!=’ operators are used, the string to the right of the operator is considered a pattern and matched according to the rules described below in Pattern Matching Nov 6, 2015 at 19:16
  • 1
    For example, test if a string contains a vowel: if [[ $str == *[aeiou]* ]]; then echo Yes; else echo No; fi. Equivalent to case "$str" in *[aeiou]*) echo Yes;; *) echo No;; esac Nov 6, 2015 at 19:18
3

Even though you are dumping the output of 'ls' into a file and parsing that you are indirectly parsing the output of 'ls' which is either problematic, a really bad idea or WRONG!!! depending on who you ask.

Here is why you shouldn't parse the output of 'ls'!

Here is Filenames and Pathnames in Shell: How to do it Correctly!

For instance if one the files has a '-' (dash/hyphen) in the filename which is not escaped (by prefixing it with a backslash ('\')) it might be interpreted as a parameter.

Avoiding parsing 'ls' may be as simple as;

find . -maxdepth 1 -iname "*"
.
./dont_parse_ls.sh
./array.dat
./.bashrc
./BASH.Indirect.Reference.sh
./basharray.sh
./.forever

Which comes out the same as

/bin/ls -alF | awk '{ print $9 }'

./
../
.bashrc
.forever/
BASH.Indirect.Reference.sh
array.dat
basharray.sh*
dont_parse_ls.sh

YMMV

3

There are at least 2 major problems in the script. Your fundamental problem is the snippet:

if [ $filename* =

There are a few problems with this. First, in shell-scripting, you cannot "glob" a match pattern. Well, you can, but if the fileglob results in more than one match, you will get both of them, in which case the "[" program (yes, it's a program) will attempt to evaluate:

filename1 filename2 filename3 = $i

It works if and only if the fileglob expands to exactly one filename, and you can rarely guarantee that. In your case, $filename expands to at least one file, but you should be aware that's not always the case. If "$file*" expands to no file at all, you might (depending on a shopt setting) get the empty string:

= $i

which will cause the [ to fail. With the right shopt, however, you instead get:

backup-2014-whatever* = $i

With the * being part of the comparison.

The second fundamental problem is use of the -F parameter in ls. This tells ls to append to the filename one of several characters, depending on if the file is an executable, soft-link, etc.

NetScr1be is onto something, but look, you don't have to follow NetScr1be's advice and never use ls... just don't use ls -l. Instead, use ls -1 which will print only the names of the files in a single-column, no frills. (For very large directories, it will sort them, and this may be a problem, in which case there is a no-sort option; or use find.)

To be more secure, your variables should be double-quoted, and both LHS and RHS prefixed with a dummy character, in order to make sure weird filenames that begin with a - don't throw it off.

I would use Glenn's advice, more or less, and do it this way:

command ls -1 | while read file; do
    if [ x"$file" = x"$filename" ]]; then 
        echo Do Work Here
    fi
done

That's how I would do it, but Glenn's kindly informed me, I really should do it his way:

for file in *; do 
    if [[ $file == $filename ]]; then ...
5
  • I alias ls to ls -F, so you'd better do command ls | ..., but that's not the correct way to iterate over files. use for file in * Nov 6, 2015 at 19:07
  • You're right: command ls is the way to go. There are problems with for file in * because files may include whitespace. Though in theory you save yourself a fork.
    – Otheus
    Nov 6, 2015 at 19:09
  • 1
    bash [[ ... ]] does not perform word splitting or glob expansion (ref), so the quotes and the x are not required Nov 6, 2015 at 19:10
  • 1
    files with whitespace is precisely why you use for. Even filenames with newlines are handled correctly. See mywiki.wooledge.org/ParsingLs Nov 6, 2015 at 19:11
  • Holy cow, when did they fix that!?! (the for thing). Many many thanks. (I was a bash 1.2 user).
    – Otheus
    Nov 6, 2015 at 19:21
1

This script does everything. Why not this? The shell will select the correct files for you so no need to call ls:

#!/bin/sh
for file in /mnt/backup/bkup_$(date +%Y%m%d)_*
do
    s3cmd put "$file" s3://auto-backup
done
  • The only external command is s3cmd.
  • No if statements.
  • The only decision point is the for loop.
  • Easy to read.

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .