3
\$\begingroup\$

Background

My team develops in a git monorepo. User stories are broken into very small discrete sub-tasks and merged directly into master, rather than into a feature branch for the story.

Topic branches

Our topic branch names are all of the form issue/<developer username>/<subtask ticket>.

Those are pushed to origin and merged via a code-reviewed pull-request process.

Release branches

We also maintain long-lived release branches named release/sprint/<number> used for hot-patching.

Release branches are never re-merged to master. The individual fixes, if relevant in the current release, are re-implemented in another branch or applied as a patch. There are few enough of them to manage manually.

Work

This works well for us, but it means we have hundreds of topic branches hanging around as developers often forget to delete them as they go.

I wrote a simple bash script to

  1. Sync up with origin (we have only one),
  2. Figure out which branches have been merged into master (or, optionally, a different branch, so far nobody has used that),
  3. Delete locally-merged branches,
  4. Optionally run the gc

The verbose prompting at every stage is intentional as reassurance.

Concerns

Primarily, is this "safe", meaning:

  1. Have I used an appropriate method to list branches?
  2. Have I used an appropriate method for deleting branches?
  3. Have I accounted for all possible pathways that could result in destruction of a local branch?
  4. Have I adequately guarded against destruction of a local branch not merged into master?
  5. Should I verify that topic branches have been merged into origin's master and not the local master, in the event that a user mistakenly merges locally and loses their branch?

I'm very happy for any additional comments, no matter how nit-picky.

Code

#!/bin/bash
MERGED_INTO=${1:-"master"}

statistics() {
  echo
  echo "Remaining local branches:"
  git branch

  echo
  echo "These local branches are not merged into ${MERGED_INTO}:"
  git branch --no-merged ${MERGED_INTO}

  echo
  echo "Size of local repo: $(git count-objects)"

  git status
}

error () {
  echo "WARNING: Something went wrong. Aborting..."
  if [[ ${1} ]]; then echo "The previous command exited with ${?}"; fi

  echo "This is the current state of your working copy:"
  git status
}

confirm () {
  read -r -p "Are you sure? [y/N] " response
  case ${response} in
    [yY][eE][sS]|[yY])
      true
      ;;
    *)
     echo "Ok, aborting..."
     echo
     false
     ;;
  esac
}

check_on_main_branch() {
  local current_branch;
  current_branch=$(git symbolic-ref -q --short HEAD)
  if [ "${current_branch}" != "${MERGED_INTO}" ]; then
    echo "You should be on '${MERGED_INTO}' for this operation. I can switch branches for you."
    confirm && git checkout "${MERGED_INTO}" || error $?
  fi
}

delete_local_merged_branches() {
  git branch --merged ${MERGED_INTO} | egrep -v "(^\*|${MERGED_INTO})" | xargs git branch --delete
}

# Step 1
echo "Fetching remote branch state"
git fetch --all

# Step 2
echo "The following local branches have been merged into ${MERGED_INTO} and will be deleted locally:"
git branch --merged ${MERGED_INTO} | egrep -v "(^\*|${MERGED_INTO})"
confirm && delete_local_merged_branches

# Step 3
echo "Would you like to run the garbage collector?"
confirm && \
 echo "Before garbage collecting: $(git count-objects)" && \
 git gc

trap statistics EXIT
trap error SIGHUP SIGINT SIGTERM
\$\endgroup\$
3
\$\begingroup\$

The final line has no effect:

trap error SIGHUP SIGINT SIGTERM

As the program is about to exit, we're exceedingly unlikely to receive any of those signals between this point and the end of the program.

It's much more usual to set the trap handler as soon as you've defined the error function.

Similarly, the exit trap

trap statistics EXIT

is the same as simply

statistics

when we're about to exit. I suggest that this line is moved to before Step 1, too.

\$\endgroup\$
  • 1
    \$\begingroup\$ Many references I've encountered explaining trap tend to show them at the end of a script. I did not realize they were only registered when they were encountered (though, saying that aloud I do feel rather inane). Useful information! \$\endgroup\$ – msanford Dec 13 '17 at 18:28
  • 1
    \$\begingroup\$ Look carefully at the examples you linked - some of those are just partial scripts; in the others, the traps are registered after defining the functions but before using them. \$\endgroup\$ – Toby Speight Dec 14 '17 at 8:58
3
\$\begingroup\$

Previous exit code

This will only ever print 0 exit code:

if [[ ${1} ]]; then echo "The previous command exited with ${?}"; fi

Because the last exit code at the time of the echo will be the result of the [[ builtin. Since $1 in this function has the intended exit code to report, you should use just that.

Exit code of a function

Although this works:

confirm () {
  read -r -p "Are you sure? [y/N] " response
  case ${response} in
    [yY][eE][sS]|[yY])
      true
      ;;
    *)
     echo "Ok, aborting..."
     echo
     false
     ;;
  esac
}

It would be better to write return 0 instead of true and return 1 instead of false, to return from the function. The current true / false solution only works because they are the last statement in their execution paths. If later you accidentally add some code after esac, that will break the function. If you want to return from a function, just use the return statement.

Redundant semicolon

The semicolon here is redundant:

local current_branch;
\$\endgroup\$
  • 1
    \$\begingroup\$ "You need to save the original exit code in a variable". That's not strictly necessary because$1 already holds the desired value, so OP only needs to put $1 instead of $? in that echo statement. \$\endgroup\$ – nxnev Dec 14 '17 at 6:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.