0

I'm working on a bash scripts that will essentially shift the entire contents of a directory, where the filenames all start with a number (0001, 0002, etc). I'm iterating in reverse order through those files to shift the last one down first.

I have one function that iterates through the files and calls another function to actually shift them. When I call the second function, it short-circuits the for loop and exits. I can't figure out why.

validation_dir="scripts/validation"
content_directories=($validation_dir "tutorials")

function shift_contents() {
    local start_index=$1
    local positions=$2

    local files=(${validation_dir}/*)
    for ((i=${#files[@]}-1; i>=0; i--)); do
        # Only gets called once
        echo $i
        local f="${files[$i]}"
        local old_index=$(echo $f | tr -dc '0-9' | sed -e 's/^0*//')
        if [ "$old_index" -lt "$start_index" ]
        then
            # Only start when we're at the start index
            continue
        fi
        local new_index=$((old_index + positions))
        # When this is called, it exits the loop
        move_exercise $old_index $new_index
    done
}

Here's the function that does the shifting:

function move_exercise() {
    local start_index=$1
    local end_index=$2
    local start_prefix=$(echo_filename_prefix $start_index)
    local end_prefix=$(echo_filename_prefix $end_index)

    for i in ${content_directories[@]}; do
        start_file_glob="${i}/${start_prefix}*"

        for f in $start_file_glob; do
            if [ -e $f ]
            then
                start_filename=$f
            else
                log_error "No content found with index ${start_index}"
                exit 1
            fi
            break
        done

        end_file_glob="${i}/${end_prefix}*"

        if [ -e $end_file_glob ]
        then
            log_error "Content was already found for end index ${end_index}: ${end_file_glob}"
            exit 1
        fi

        # Generate the new file name
        new_filename="${start_filename/$start_prefix/$end_prefix}"
        # We get down here once
        echo $start_filename $new_filename
    done

    return
}

It looks like the second function is causing the first to exit early, but I don't see how that could happen. See what's going on?

EDIT:

When I run with bash -x script.sh, it ends with the following output when it should have one more iteration:

+ echo path/to/second-to-last-file new-path
+ return
+ (( i-- ))
+ (( i>=0 ))

Does that imply that it's failing the for loop condition check? If so, how could move_exercise impact that? If I comment out that line, it works as expected.

12
  • 2
    I might be stating the obvious here, but try running your script with -x: bash -x script.sh. It's often the best debugging tool for bash scripts. Commented Apr 3, 2017 at 22:07
  • The move_exercise function has 2 exit points, is it possible that either one of these points are reached ? Commented Apr 3, 2017 at 22:17
  • 1
    As an aside, there are a whole bunch of other bugs here that shellcheck.net will catch (as well as some it may not). Commented Apr 3, 2017 at 22:45
  • 1
    ...so, let's start on the "bunch of other bugs". I'm assuming you expect [ -e $globexpr ] to tell you whether at least one file matching the given glob expression actually exists. However, that's not what it does in practice. If you have more than match, then you end up with a command line [ -e file1 file2 ], which is insofar as [ is concerned simply invalid syntax. Commented Apr 3, 2017 at 22:54
  • 1
    Moreover, using glob expressions from a variable is a precarious practice in and of itself. It's much more reliable to store the results of a glob expression in an array and expand that array. Consider: dir='/directory/with spaces' -- you can expand "$dir"/*.txt without any problem, but glob=$dir/*.txt and then expanding $glob will create /directory/with as the first result, and spaces/*.txt as the second (unless you also have a directory named spaces with files matching *.txt in it, in which case those would result). Commented Apr 3, 2017 at 22:56

1 Answer 1

2

If you want i to be local, you need to declare it as such:

shift_contents() {
  local i
  # ...etc...
}

move_exercise() {
  local i
  # ...etc...
}

Otherwise there's only one variable named i shared between both functions. When that variable contains a filename, its numeric value is 0 (unless that filename is also the name of a shell variable with a nonzero numeric value assigned) -- so when you decrement it after assigning a filename, it becomes negative.

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.