8
\$\begingroup\$

The code takes a user input dir path, then looks for git remote url that is still using SSH, and the repo is belong to an fixed Organisation. Then it will convert these repos' git remote url from SSH to HTTPS.

Goal is to let users (on Linux or Mac) effortlessly convert all dir containing the Organisation's code to use HTTPS instead of SSH.

My senior programmer said problems in the code are so many that he has to sit down with me next week to talk through! Please help me to spot problems before embarrassment starts. Thank you

#!/usr/bin/env bash

# This script loops through all directories within the given path,
# and run a conversion script to convert Git CLI auth type.
GITHUB_USERNAME='Mock-Technology'

converter () {
  # This function will convert origin url from SSH style to HTTPS style, for the
  # git containing local directory

  if [[ -z $GITHUB_USERNAME ]]; then
    GITHUB_USERNAME='Mock-Technology'
  fi

  echo "-- converter starting..."
  cd "$1" || exit
  if [[ ! -f "config" ]]; then
    echo "This directory does not contain Git, exitting..."
    return
  else
    origin_str=$(cat "config")
  fi

  repo_url=$(echo "$origin_str" | sed -Ene's#.*(git@[^[:space:]]*).*#\1#p')
  if [[ -z "$repo_url" ]]; then
    echo "-- ERROR:  Could not identify Repo url."
    if [[ $origin_str == *"https"* ]]; then
      echo "   This repo is already using HTTPS instead of SSH."; else
      echo "   Valid remote URL was not found!"
    fi
    return
  fi
  echo "-- Confirmed: It is using SSH..."

  user_name=$(echo "$repo_url" | sed -Ene's#git@github.com:([^/]*)/(.*).git#\1#p')
  if [[ -z "$user_name" ]]; then
    echo "-- ERROR:  Could not identify User."
    return
  elif [ "$user_name" != "$GITHUB_USERNAME" ]; then
    echo "-- WARNING: The repo does not belong to '$GITHUB_USERNAME', ignoring..."
    return
  fi
  echo "-- User name extracted: $user_name"

  repo_name=$(echo "$repo_url" | sed -Ene's#git@github.com:([^/]*)/(.*).git#\2#p')
  if [[ -z "$repo_name" ]]; then
    echo "-- ERROR:  Could not identify Repo."
    return
  fi
  echo "-- Repo name extracted: $repo_name"

  new_url="https://github.com/$user_name/$repo_name.git"
  echo "-- Changing repo url from: "
  echo "  '$repo_url'"
  echo "      to "
  echo "  '$new_url'"
  echo ""

  change_cmd="git remote set-url origin $new_url"
  eval "$change_cmd"
  echo "-- Success!"

}

validate_path() {
  # this module validates input path against empty string and non-existing path
  if [[ -z $1 ]]; then
    echo "path is empty string, existting..."
    exit
  fi

  if [[ ! -d $1 ]]; then
    echo "path does not exist, exitting..."
    exit
  fi
  echo "-- path validation check passed..."
}

loop_subdirectories() {
  # visits subfolders and try to find any ".git" directory
  # It invokes converter func for each ".git" directory found
  while read -r dir; do
    converter "$dir"
  done <<< "$(find "$1" -name .git -type d)"

}

read -p "Enter path: " path
validate_path "$path"
loop_subdirectories "$path"
\$\endgroup\$
3
\$\begingroup\$

Extracting remote urls

Parsing the config file is error-prone. A much better way to extract the information about remotes is using Git commands. This will simplify your script a great deal.

Limitations

Be aware that the script as it is will not be able to convert remotes that were created using URL shorthands. For example, I have this in my global .gitconfig:

[url "git@github.com:"]
    insteadOf = "gh:"

This lets me clone a GitHub repo with git clone gh:user/repo instead of git clone git@github.com:user/repo. The config file will be written using the shorthand instead of the real URL, so your script will not be able to pick up the pattern.

You can easily solve this limitation by using Git commands to extract Git remote URLs.

String transformations

Instead of echo ... | sed ..., it's better to use here-strings:

sed ... <<< ...

But when the text transformations are simple enough, and Bash's parameter expansion can handle it, that's even better. For example, instead of this:

  user_name=$(echo "$repo_url" | sed -Ene's#git@github.com:([^/]*)/(.*).git#\1#p')

This would be better:

# chop off the beginning until the first :
user_name=${repo_url#*:}
# chop off everything after the first /
user_name=${user_name%%/*}

Exiting with error

There are multiple exit points in the validate_path function. Without parameter, exit will use the exit code of the previous command, and if that was echo, it's likely to be success, when in fact you probably want to exit with error instead.

Usability

Reading paths inside a script is not user-friendly, because you cannot use tab-completion. It would be better to let users pass paths as parameters to the script.

\$\endgroup\$
  • \$\begingroup\$ I've got a question about using git commands. I know I can check url by using git remote -v, but it only works at current path. Therefore, if I want to have a wrapper function for it, i.e. loop_subdirectories(), that calls this function, and have a clause cd $1 trying to jump into each folder. Then error happens: running git remote -v only shows url of current path that the script was calling from. Do you know how to avoid this error, and be able to run git remote -v inside sub-folders? Thank you. \$\endgroup\$ – Song Jin Sep 9 '17 at 7:53
  • \$\begingroup\$ @SongJin You could do GIT_DIR=the/path/.git git remote -v or git --git-dir=the/path/.git remote -v \$\endgroup\$ – janos Sep 9 '17 at 20:30
0
\$\begingroup\$

Since Janos has covered the functionality pretty well I will focus on semantics. Nothing in this answer will fundamentally improve the functionality of the code, but will result in a better user experience and will thus work towards making your code more acceptable to your reviewer.

Spelling

Please spell check anything that might be visible to an end user. If I was an end user and saw 'existting' or 'exitting' instead of 'exiting' it would lead me, maybe incorrectly, to believe the author did not have good attention to detail. This in turn would potentially lead me to have less trust in the program to operate correctly.

Portability

GITHUB_USERNAME having a hard coded value makes the code less portable. It is absolutely reasonable to have a default, but if you allow this to be overwritten by user input then it allows the same program to be used in a more flexible environment. Following from Janos' suggestion of passing the path as a parameter, the username could be passed as an optional second parameter. Such functionality would not affect users who do not need it.

Pattern logic

Think about what message I would see if my remote path was
git@mysubdomain.httpscorp.com:/gitrepo/myrepo.git

\$\endgroup\$
0
\$\begingroup\$

I(the original poster) have made some updates based on above valuable feedbacks. commit msg was

add usage&log&die, change path-input from inside script to arg to improve usability i.e.tab-completion, correct typo

usage ()  {
    echo "usage: $0 [directory path]"
    exit 0
}

die() { printf "!!! %s\n" "$*"; exit 1; }

log () {
    ts="$(date '+%Y-%m-%d %H:%M:%S')"
    echo "$ts $*"
}

converter_ssh_https () {
  # This function will convert origin url from SSH style to HTTPS style, for the
  # git containing local directory
  echo " "
  log "-- converter starting..."

  if [[ ! -z "$(git -C . rev-parse)" ]]; then
    log "This directory does not contain Git, exiting..."
    return
  else
    origin_str=$(git remote -v | grep -m1 '^origin')
  fi

  repo_url=$(echo "$origin_str" | sed -Ene's#.*(git@[^[:space:]]*).*#\1#p')
  if [[ -z "$repo_url" ]]; then
    log "-- ERROR:  Could not identify Repo url."
    if [[ $origin_str == *"https"* ]]; then
      log "   This repo is already using HTTPS instead of SSH."; else
      log "   Valid remote URL was not found!"
    fi
    return
  fi
  log "-- Confirmed: It is using SSH..."

  user_name=$(echo "$repo_url" | sed -Ene's#git@github.com:([^/]*)/(.*).git#\1#p')
  if [[ -z "$user_name" ]]; then
    log "-- ERROR:  Could not identify User."
    return
  elif [ "$user_name" != "$GITHUB_USERNAME" ]; then
    echo "-- WARNING: The repo does not belong to '$GITHUB_USERNAME', ignoring..."
    return
  fi
  log "-- User name extracted: $user_name"

  repo_name=$(echo "$repo_url" | sed -Ene's#git@github.com:([^/]*)/(.*).git#\2#p')
  if [[ -z "$repo_name" ]]; then
    log "-- ERROR:  Could not identify Repo."
    return
  fi
  log "-- Repo name extracted: $repo_name"

  new_url="https://github.com/"$user_name"/"$repo_name".git"
  log "-- Changing repo url from: "
  log "  '$repo_url'"
  log "      to "
  log "  '$new_url'"
  log ""

  change_cmd="git remote set-url origin $new_url"
  eval "$change_cmd"
  log "-- Success!"
}

validate_path() {
  # this module validates input path against empty string and non-existing path
  if [[ -z $1 ]]; then
    die "path was an empty string, exiting..."
  fi

  if [[ ! -d $1 ]]; then
    die "path does not exist, exiting..."
  fi

  log "-- path validation check passed..."
}

loop_subdirectories() {
  while read -r dir; do
    cd "$dir" && converter_ssh_https
    # cd "$dir" && converter_https_ssh
  done <<< "$(find "$1" -name .git -type d)"
}


# config
GITHUB_USERNAME='Mock-Technology'

# pre-flight check
: "${GITHUB_USERNAME?Export GITHUB_USERNAME and try again}"
[[ "$#" == "1" ]] || usage
validate_path "$1"

loop_subdirectories "$1"
\$\endgroup\$
  • 1
    \$\begingroup\$ If you posted this as a follow-up question, I'd have some more feedback about it. \$\endgroup\$ – janos Sep 9 '17 at 20:37
  • \$\begingroup\$ How to do that please? Add it to your comment, or my original question? \$\endgroup\$ – Song Jin Sep 9 '17 at 22:36
  • \$\begingroup\$ Ask a new question, with the new code, and a brief summary of what you changed, which tips you used from previous answers, and which you didn't and why \$\endgroup\$ – janos Sep 10 '17 at 5:33

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.