Ytdyklly

I'm doing a lot of Python development lately for various (small-)data analysis pipelines at work. I've been wrestling with how to robustly and ~automatically version the code at a fine-grained level, so as to provide strong guarantees of reproducibility of a result generated from any particular version of the code, at any particular point in my development process.

I've settled on a CalVer approach. Given that I often want to have multiple versions of the code tagged within a single day, I'm using a ~nonstandard $TIMESTAMP format of YYYY.MM.DD.hhmm. (hhmmss seemed like it would be overkill.)

In any event, I want two things to happen every time I commit code to one of these data analysis repos:

1. Wherever relevant in the package (usually just in the main __init__.py), __version__ should be updated to $TIMESTAMP.


2. Once the code is committed, a tag named $TIMESTAMP should be applied to the new commit

Ancillary goals are the usual: easy to configure, minimal likelihood of breaking all the things, and minimal additional cleanup effort in common non-happy-path scenarios.

The following is a bash script I've put together for the purpose:

#! /bin/bash



export TIMESTAMP="$( date '+%Y.%m.%d.%H%M' )"

export VERPATH='.verpath'



if [ -z $VERPATH ]

then

  # Complain and exit

  echo "ERROR: Path to files with versions to update must be provided in {repo root}/.verpath"

  echo " "

  exit 1

fi



# $VERPATH must contain the paths to the files to be updated with

# the timestamped version, one per line

while read VERFILE

do

  # Cosmetic

  echo ""



  if [ -e "$VERFILE" ]

  then

    # File to be updated with version exists; update and add to commit.

    # Tempfile with old file stored in case of commit cancellation.

    echo "Updating $VERFILE"

    cp "$VERFILE" "$VERFILE.tmp"

    sed -i "s/^__version__ = .*$/__version__ = '$TIMESTAMP'/" $VERFILE

    git add "$VERFILE"



  else

    echo "$VERFILE not found!"



  fi



done < $VERPATH



# Cosmetic

echo ""



# So user can see what was updated

sleep 2s



# Actually do the commit, passing through any parameters

git commit $@



# If the commit succeeded, tag HEAD with $TIMESTAMP and delete temp file(s).

# If the commit failed, restore the prior state of the $VERFILEs.

if [ "$?" -eq "0" ]

then

  git tag -f "$TIMESTAMP"



  while read VERFILE

  do

    rm -f "$VERFILE.tmp"

  done < $VERPATH



else

  while read VERFILE

  do

    if [ -e "$VERFILE.tmp" ]

    then

      git reset HEAD "$VERFILE" > /dev/null 2>&1

      rm "$VERFILE"

      mv "$VERFILE.tmp" "$VERFILE"

    fi

  done < $VERPATH



fi

The contents of .verpath in my test repo are:

pkg/__init__.py

pkg/__dupe__.py

pkg/nofile.py

Both pkg/__init__.py and pkg/__dupe__.py exist; pkg/nofile.py does not.

I have *.tmp in my .gitignore so that the $VERFILE.tmp don't show up as untracked files when drafting the commit message.

It works like I want it to... the happy path works great, and it handles aborted commits and nonexistent .verpath files gracefully.

I'm no bash expert, though, so I'm partly concerned about subtle misbehaviors I haven't thought of. Also, I'm not super thrilled about the use of in-folder temporary files, and per here and here the while read VERFILE ... done < $VERPATH has the potential to be fragile if I don't set it up correctly.

It's good practice to preserve file permissions on copy, especially as your backup may replace the original. cp -p does so.

Most file-handling commands can't handle filenames beginning with hyphen, unless you terminate the options with --, as in mv -- $old $new. It's prudent to include the terminator whenever you're sending user-supplied filenames to a command.

while read .. do rm gives me the willies. I'd instead keep track of .tmp files we've created, and remove those, as in:

# array

declare -a to_remove

...

# queue removal on successful copy else exit

cp -p -- "$VERFILE" "$VERFILE.tmp" && to_remove+=( "$VERFILE.tmp" ) || exit 1 

...

if git commit "$@"

then 

    git tag -f "$TIMESTAMP"

    rm -f -- "${to_remove[@]}"

else

    for backup in "${to_remove[@]}"

    do

        original="${backup%.tmp}"

        # no need to explicitly test existence: just attempt the mv + bail on failure

        mv -- "$backup" "$original" &&  git reset HEAD "$original" >& /dev/null

    done

fi

echo and echo "" are equivalent.

$@ should be double-quoted; this protects internal quotes. Unquoted $* is appropriate to use when you know quoting is unnecessary.

You can use [[ .. ]] instead of [ .. ] to do tests. The former is a bash builtin and saves a fork.

Your tests can use the more specific -f (is file or symlink to file) instead of -e (exists).

>& /dev/null is equivalent to >/dev/null 2>&1.

First of all I'd like to recommend shellcheck (online, GitHub) as a very useful tool to detect errors (even small misspellings of variables) and possible misbehaviours, etc.. With this you would have been warned about double-quoting your variables to prevent accidental splitting.

#! /bin/bash

There is no need to export these variables, since they won't be used after the termination of the script.

I'd personally stay clear from all caps variable names, if they are not environment variables. (By this convention, shellcheck will not complain for these variables if they are unset, as it assumes these to be provided by the shell.)

In principle, there is no need to double-quote a subshell, but it does not hurt.

timestamp="$( date '+%Y.%m.%d.%H%M' )"

verpath='.verpath'

Since you have just set the variable, this if clause will never trigger, as it is never empty. You probably would like to check whether the file exists (and is readable).

if [ -z $VERPATH ]

then

  # Complain and exit

  echo "ERROR: Path to files with versions to update must be provided in {repo root}/.verpath"

  echo " "

  exit 1

fi

I second switching to the bash built-in test [[ expression ]].

For debugging purposes I try to stick with positive evaluations, usually inserting a statement that it works as intended.

While in a simple script like this, echo is perfectly fine, I still recommend looking at printf for more complex things. I'd probably use something like the following:

if [[ -f "$verpath" && -r "$verpath" ]] ; then

  :  # do nothing (or give a debug statement)

else

  # complain and exit

  printf 'ERROR: Paths to files with versions to update should be provided in %s.nn' "$verpath"

  exit 1

fi

Be aware that read without the option -r will mangle backslashes (see SC2162). If for whatever reason the carriage return is missing from the last line, it will be ignored, better use the following while loop, where -n tests for a non-zero length string:

while read -r verfile || [[ -n "$verfile" ]] ; do

  : # do something

done < "$verpath"

Like in the other answer suggested, I also prefer to keep track of temporary files, instead of assuming they have been created as intended. This will also spare you of reading in the file again.

Since you are making backups, I would also switch to a different ending like .bak. Initialise the array first:

declare -a backups

I'd again check whether the file exists and is readable.

Since these are backups, I'd also use the -a option to cp to archive them. I'd advise on exiting if the command fails. You might also want to consider checking whether the target backup file does already exist and exit if it does.

The inline substitution with sed might have a catch, if there are fewer of more spaces in the search pattern. I think you could be a bit more greedy.

if [[ -f "$varfile" && -r "$varfile" ]] ; then 

  printf 'INFO: Updating %s.n' "$varfile"

  # Add backup file to array

  backups+=( "$varfile.bak" )

  [[ -e "${backups[-1]} ]] && { printf 'WARNING: backup %s exists.n' "${backups[-1]}" ; exit 1 ; }

  cp -a -- "$varfile" "${[backups[-1]}" || { printf 'ERROR: backup of %s failed.n' "$varfile" ; exit 1 ; }

  sed -i "s/^__version__.*$/__version__ = '$timestamp'/" "$verfile"

  git add "$verfile" || { printf 'ERROR: adding %s to repository failedn' "$verfile" ; exit 1 ; }

  printf 'n'

else

  printf 'INFO: file %s not found.nn' "$verfile"

fi



# So user can see what was updated

sleep 2s

The bash built-in $@ is an array, it needs to be double-quoted to prevent resplitting. On the other hand, $* is a string, i.e. it looses the elements of an array. Unquoted, it will also be resplit, which is hardly ever anything you would want.

Always check the exit state of a command directly to prevent it being overwritten.

# Actually do the commit, passing through any parameters

if git commit "$@"

then

# Commit succeeded: tag HEAD with $timestamp and delete backup file(s).

  git tag -f "$TIMESTAMP" || { printf 'WARNING: Adding tag failed.n' ; }

  rm -f -- "${backups[@]}"

else

# Commit failed: restore the backups to its original location.

  for file in "${backups[@]}" ; do

    mv -- "$file" "${file%.bak}" && git reset HEAD "$file" &> /dev/null 

  done

fi

Generally I do all file handlings much more verbose, wrapping everything into a function and switch off the output of that function as necessary, i.e.

#! /bin/bash

mybackup ()

{

  while [[ -n $1 ]] ; do

    : # Things to do

    source="$1"

    shift

    target="$source.bak"

    cp -vp "$source" "$target" >&3 2>&3

  done

  : # more things to do ...

}



if [[ "$1" == "-s" ]] ; then

  exec 3> /dev/null

  shift

else

  exec 3>&1

fi



mybackup "$@"

This would also make things simpler if you were to create logfiles of your script, but that is beyond this review.

Right now your versioning depends on an auxiliary file, that you probably create manually. I don't know how large your repositories are and how often you are using the versioning statements. It might be better to either check every file, or hardcode the ones that need to be checked into the script, instead of an external file.