2
\$\begingroup\$

After reading this Stack Overflow question I decided to write a backup utility for local git repositories.

The result is on GitHub.

I would like this application to be well-behaved with an installer, manpages and no bugs.

It is my first time writing Ruby, so there might well be better, safer, more correct or simply more elegant ways of doing things. I also still wonder if git bundle is the best way of creating an identical repository (I still consider tar as an option too).

3/4 of the code is just error handling, but I suppose that is just normal.

The restore script is currently a functional dummy, it resides on GitHub, but is probably not worth reviewing. My tabwidth is 3, though mid-line alignment is done with spaces.

#!/usr/bin/env ruby
#
# For documentation please sea man git-backup(1)
#
# TODO:
# - make it a class rather than a function
# - check the standard format of git warnings to be conform
# - do better checking for git repo than calling git status
# - if multiple entries found in config file, specify which file
# - make it work with submodules
# - propose to make backup directory if it does not exists
# - depth feature in git config (eg. only keep 3 backups for a repo - like rotate...)
# - TESTING



# allow calling from other scripts
def git_backup


# constants:
git_dir_name    = '.git'          # just to avoid magic "strings"
filename_suffix = ".git.bundle"   # will be added to the filename of the created backup


# Test if we are inside a git repo
`git status 2>&1`

if $?.exitstatus != 0

   puts 'fatal: Not a git repository: .git or at least cannot get zero exit status from "git status"'
   exit 2


else # git status success

   until        File::directory?( Dir.pwd + '/' + git_dir_name )             \
            or  File::directory?( Dir.pwd                      ) == '/'


         Dir.chdir( '..' )
   end


   unless File::directory?( Dir.pwd + '/.git' )

      raise( 'fatal: Directory still not a git repo: ' + Dir.pwd )

   end

end


# git-config --get of version 1.7.10 does:
#
# if the key does not exist git config exits with 1
# if the key exists twice in the same file   with 2
# if the key exists exactly once             with 0
#
# if the key does not exist       , an empty string is send to stdin
# if the key exists multiple times, the last value  is send to stdin
# if exaclty one key is found once, it's value      is send to stdin
#


# get the setting for the backup directory
# ----------------------------------------

directory = `git config --get backup.directory`


# git config adds a newline, so remove it
directory.chomp!


# check exit status of git config
case $?.exitstatus

   when 1 : directory = Dir.pwd[ /(.+)\/[^\/]+/, 1]

            puts 'Warning: Could not find backup.directory in your git config file. Please set it. See "man git config" for more details on git configuration files. Defaulting to the same directroy your git repo is in: ' + directory

   when 2 : puts 'Warning: Multiple entries of backup.directory found in your git config file. Will use the last one: ' + directory

   else     unless $?.exitstatus == 0 then raise( 'fatal: unknown exit status from git-config: ' + $?.exitstatus ) end

end


# verify directory exists
unless File::directory?( directory )

   raise( 'fatal: backup directory does not exists: ' + directory )

end


# The date and time prefix
# ------------------------

prefix           = ''
prefix_date      = Time.now.strftime( '%F'       ) + ' - ' # %F = YYYY-MM-DD
prefix_time      = Time.now.strftime( '%H:%M:%S' ) + ' - '
add_date_default = true
add_time_default = false

prefix += prefix_date if git_config_bool( 'backup.prefix-date', add_date_default )
prefix += prefix_time if git_config_bool( 'backup.prefix-time', add_time_default )



# default bundle name is the name of the repo
bundle_name = Dir.pwd.split('/').last

# set the name of the file to the first command line argument if given
bundle_name = ARGV[0] if( ARGV[0] )


bundle_name = File::join( directory, prefix + bundle_name + filename_suffix )


puts "Backing up to bundle #{bundle_name.inspect}"


# git bundle will print it's own error messages if it fails
`git bundle create #{bundle_name.inspect} --all --remotes`


end # def git_backup



# helper function to call git config to retrieve a boolean setting
def git_config_bool( option, default_value )

   # get the setting for the prefix-time from git config
   config_value = `git config --get #{option.inspect}`

   # check exit status of git config
   case $?.exitstatus

      # when not set take default
      when 1 : return default_value

      when 0 : return true unless config_value =~ /(false|no|0)/i

      when 2 : puts 'Warning: Multiple entries of #{option.inspect} found in your git config file. Will use the last one: ' + config_value
               return true unless config_value =~ /(false|no|0)/i

      else     raise( 'fatal: unknown exit status from git-config: ' + $?.exitstatus )

   end
end

# function needs to be called if we are not included in another script
git_backup if __FILE__ == $0
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

There is a great deal of vertical white space; probably more than is good for readability. Unless it is an artifact of cut and paste, you might consider removing some of it.

The usual indent level for Ruby is 2 spaces. You don't have to do what's usual, of course, but it will make life easier for your code's readers if you can do it.

Good job with the constants git_dir_name and prefix. You can improve them by making them all caps: GIT_DIR_NAME = '.git'enter code here`. This tells both Ruby and your reader that they are indeed constants.

Checking that the file is run directly by comparing __FILE__ to $0 is a common practice, not always necessary, but often done even when not strictly needed. Its appearance here is unusual, in that it exits with an error if required. More commonly, it simply does nothing, e.g.:

def do_what_the_script_does
  ...
end`

do_what_the_script_does unless __FILE__ == $0

That allows a script that can be run directly to also be required by another script, so that the other script can have access to this script's functions, classes, &c.

rather than $?.exitstatus != 0, you can just check $? != 0.

When running an external program that can have an error, and the error messages reported by the program are acceptable to you, you can simplify your code by replacing sequences such as:

`run_some_program`
if $? != 0
  puts "An error happened"
  exit(1)
end

With:

system('run_some_program')
exit if $? != 0

The duplicate logic in the two big case statements ought to be put in one function. That makes maintenance easier and makes the code easier to follow.

Creating a one-line if statement using : works in Ruby 1.8, but not in Ruby 1.9 (and even though it works in Ruby 1.8, it's a little bit funky). To be compatible with future versions of ruby, use then instead. Or, if the line is at all long, just hit enter.

Consider using methods to group little bits of code together. The name of the method will serve as an executable comment.

No, you do not really need to call chomp when creating the bundle_name.

An easy way to quote interpolated strings is:

puts "Backing up to bundle #{bundle_name.inspect}"

Although Widows uses spaces in filenames as a matter of practice, Unix programmers look less kindly upon the practice. I would omit the spaces in the filename, but that's my personal preference.

Consider using optparse, a library that comes with Ruby, to parse arguments.

If you use File.join to create your path, it won't matter whether or not directory ends with a /. So do this:

bundle_path = File.join(directory, prefix + bundle_name + suffix)

and you can eliminate this:

directory = directory.chomp('/')
\$\endgroup\$
1
  • \$\begingroup\$ Thanks alot. Very helpful. I corrected the script. I kept $?.exitstatus because it is less cryptic and because $? doesn't really refer to the exit status. I used Fill::join. Regarding spaces in file names, well spaces are a natural part of our language, as well as valid filename characters. I don't see why not to use them. If it breaks something, then it probably reveals inadequate escaping. \$\endgroup\$
    – user13328
    Commented May 15, 2012 at 17:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.