CODEWITHSUNDEEP
gitbash
Kapil Gupta
Kapil Gupta

Reputation: 7721

Bash Script not looping correctly

I don't know anything about bash scripting. I have written this script to update all the repositories that I have in my directory. But instead of going through all the repos like it should, it is going through the first repository only and looping on the first one. What is the error in the code that is causing it ?

Code :

rm -rf log.txt
touch log.txt
ls > log.txt
for (( i = 0; i < $(cat log | wc -l); i++ )); do
    dir_name=$(sed "${i+1}q;d" log.txt)
    cd ${dir_name}
    git pull origin master
    cd ..
done

Upvotes: 0

Views: 207

Answers (4)

silel
silel

Reputation: 587

Your script is quite convoluted, instead you can try:

for dir_name in "$(find -maxdepth 1 -type d)"; do
    cd "$dir_name" && git pull origin master && cd - || break
done

EDIT (handling whitespaces correctly):

find -maxdepth 1 -type d -print0 | while read -d $'\0' dir_name; do
    ( cd "$dir_name" && git pull origin master ) || break
done

In case not all directories are git repositories you can use this:

for dir_name in "$(find -maxdepth 1 -type d)"; do
find -maxdepth 1 -type d -print0 | while read -d $'\0' dir_name; do
    (
    cd "$dir_name" || exit $?
    [ "$(git rev-parse --is-inside-work-tree 2>/dev/null)" == "true" ] ||
        { echo "$dir_name: not a git repository"; exit 0; }
    git pull origin master || exit $?
    ) || break
done

Upvotes: 2

Jan Hudec
Jan Hudec

Reputation: 76316

rm -rf log.txt
touch log.txt
ls > log.txt

There is rarely good reason to create temporary files in bash. Simple script like this should be able to handle everything with piping—or even better, just filename generation.

for (( i = 0; i < $(cat log | wc -l); i++ )); do

This is a really, really, rare and obscure way to do loop in bash. Normal loop is

for variable in words…; do …; done

In this case, you don't need ls at all, because shell can generate list of files itself via filename generation. So just:

for dir_name in *; do

If you use a variable or shell expansion, it will be split, but on whitespace. You can pipe into a while loop for other splitting (see below).

    dir_name=$(sed "${i+1}q;d" log.txt)

We've replaced this line by directly iterating over the names. But I should note, that this line is the reason why your original code didn't work (besides the missing extension). The problem is that

${variable+value}

is not an arithmetic expression. Instead, it returns value if variable is defined and nothing otherwise. So ${i+1} is always 1.

Arithmetic expression is instead written with $(()).

    cd ${dir_name}

Normally the {} are only used if you need to delimit the variable name from the following text. However, what you should do is quote the expansion, because the directory name might include spaces and then the shell would split the name and the command would fail—or worse, do something dangerous! So

cd "$dir_name"

    git pull origin master
    cd ..

If the cd "$dir_name" fails for some reason, the cd .. will take you somewhere you didn't want to be:

  • if $dir_name is a file, the first cd won't do anything and the second will take you one level up and
  • if $dir_name is a symlink, the first cd will resolve it and the second will take you to parent of the actual directory.

I would normally use a subshell here:

(
    cd "$dir_name"
    git pull origin master
)

The () create a subshell. All changes made to variables and current directory are local to that block.

Alternately, you can use pushd and popd:

pushd "$dir_name"
git pull origin master
popd

but that requires a bash, while the rest of what I am writing here is POSIX shell. Writing shells without the bash extensions is good, because many systems have a POSIX shell under /bin/sh, that is faster than bash, but does not have most of those extensions.

done

I mentioned a while loop. If you did want to generate the values by a program that outputs one per line, instead of a for loop you'd pipe into a while loop. So if you wanted to use ls here (there is no good reason to), you'd replace the for line with:

ls | while read -r dir_name; do

The read command reads a line from input and sets the specified variable to it (and fails when there is no line to read). The while then runs until the command fails. The -r flag tells it not to interpret \ escape sequences. I don't remember off the top of my head whether the escaping mode of ls would match what read expects—if yes, that would be more reliable way.

Upvotes: 2

ElpieKay
ElpieKay

Reputation: 30888

Here is another possible approach.

ls -F | grep -e "/$" | while read repo
do
    git --git-dir=${repo}.git pull origin master
done

Upvotes: -1

nivesnine
nivesnine

Reputation: 169

cat 'log.txt' instead of 'log'

rm -rf log.txt
touch log.txt
ls > log.txt
for (( i = 1; i <= $(cat log.txt | wc -l); i++ )); do
    dir_name=$(sed "${i}q;d" log.txt)
    cd ${dir_name}
    git pull origin master
    cd ..
done

Upvotes: 0

Related Questions