[rancid] Re: [PATCH] Use Git to store configs

[Austin Schutz]

On Fri, Mar 30, 2007 at 06:53:38AM -0500, Jeffrey C. Ollie wrote:
> Here's a patch that I've been working on that allows you to store your
> configs in a Git[1] repository.
> 

	I like the change to case() where the syntax differs.
	Comments inline.

> Adding a third RCS system necessitated a little bit of code
> reorganization, so I may have inadvertantly broken something (I haven't
> tested CVS or Subversion repositories with the patch applied).  The
> other big differences are:
> 
> 1) When using CVS and Subversion, RANCID is working on local checkouts
> (located in $BASEDIR) of a repository that is located elsewhere on disk
> ($CVSROOT).  Git works differently, so $BASEDIR is the complete
> repository and $CVSROOT isn't really necessary.

	$BASEDIR is the dir into which all groups go. If you put a generic
lockfile here you will make it so multiple groups can't be polled at one
time. Typically a temp dir is used where the file includes the group name
so there is no stepping on of toes, and stale lockfiles aren't left in
unexpected places.

> 
> 2) Since there could be multiple processes acting simultaneously on the
> same repository (with CVS and Subversion RANCID had a separate checkout
> for each group) I guard all operations on the repository using flock(1).
> flock(1) does not operate over NFS.
> 
	IMO that's a problem: flock isn't available on all platforms, and
dealing with nfs support could be annoying. I have a perl based lock
script which uses fcntl (works w/ nfs) and should be reasonably cross
platform, if that's useful (no other dependencies). Rancid's existing lockfile
support is fairly dumb (simple). I get stale hangs sometimes after a reboot,
myself- though it generally doesn't have to be that smart because it doesn't
run that often.

> I've attached the patch, or you can follow progress on my web site[2].
> 
...
>  fi
> @@ -138,6 +138,7 @@ then
>      rm -f .cvsignore
>      cat >.cvsignore <<EOF
>  .cvsignore
> +.gitignore
  ^^^^^^^^^^^ this seems a little funky to me. Seems like there should be
an if/then or switch/case for git here and in the surrounding code.

> @@ -265,21 +280,27 @@ then
>      cd $DIR/configs
>  
>      # Add new routers to the CVS structure.
> -    for router in `comm -13 $DIR/routers.up $DIR/routers.up.new`
> +    for router in `comm -13 $DIR/routers.up $DIR/routers.up.new | cut -d: -f1`

	If there are other bugs you might want to submit them independent of
the git changes.

> +    case $RCSSYS in
> +	cvs )
> +	    cvs status $router | grep -i 'status: unknown' > /dev/null 2>&1
> +	    if [ $? -eq 0 ]; then
> +		touch $router
> +		cvs add -ko $router
> +		echo "$RCSSYS added missing router $router"
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	This doesn't need to be repeated for each case statement.
	
> +	    fi
> +	    ;;
> +	svn )
> +	    svn status $router | grep '^?' > /dev/null 2>&1
> +	    if [ $? -eq 0 ]; then
> +		touch $router
> +		svn add $router
> +		echo "$RCSSYS added missing router $router"
> +	    fi
> +	    ;;
> +	git )
> +	    git ls-files $router > /dev/null 2>&1 
> +	    if [ $? -eq 1 ]; then
> +		touch $router
> +		(
> +		    flock -x 200
> +		    git add $router
> +		    git commit -m "added missing router $router"
> +		) 200>$BASEDIR/.lockfile
> +		echo "$RCSSYS added missing router $router"
> +	    fi
> +	    ;;
> +    esac
>  done
>  echo

	Otherwise looks like a nice bit of code.

	Austin