This patch seems pretty reasonable. A couple comments:
1) Changes to arguments processing aren't mentioned in the log
message.
2) Description of the changes to comparator in the log message could
be a little more clear.
3) Error handling for a failed import vs. failed execution should be
more verbose, with the error string differentiating between the two
cases (instead of using a single, generic "failed" message for both).
- Dan
On Sun, 19 Feb 2006, Chris Foote wrote:
> This patch adds archive support to the hot-backup.py script using the python
> modules. This is especially useful on Windows.
>
> Regards
> Chris
>
> [[[Log]]]
>
> Add option to archive the backup using gz, bz2 or zip.
>
> * tools/backup/hot-backup.py.in
> Update copyright year.
> (archive_map): New. Supported archive extentions.
> (comparator): Compare archives, if archiving.
> Step 4 archive backup.
> Index: tools/backup/hot-backup.py.in
> ===================================================================
> --- tools/backup/hot-backup.py.in (revision 18522)
> +++ tools/backup/hot-backup.py.in (working copy)
> @@ -7,7 +7,7 @@
> # See http://subversion.tigris.org for more information.
> #
> # ====================================================================
> -# Copyright (c) 2000-2004 CollabNet. All rights reserved.
> +# Copyright (c) 2000-2006 CollabNet. All rights reserved.
> #
> # This software is licensed as described in the file COPYING, which
> # you should have received as part of this distribution. The terms
> @@ -36,12 +36,21 @@
> # Number of backups to keep around (0 for "keep them all")
> num_backups = 64
>
> +# Archive types/extentions
> +archive_map = {
> + 'gz' : ".tar.gz",
> + 'bz2' : ".tar.bz2",
> + 'zip' : ".zip"
> + }
> +
> ######################################################################
> # Command line arguments
>
> -
> -if len(sys.argv) != 3:
> - print "Usage: ", os.path.basename(sys.argv[0]), " <repos_path> <backup_path>"
> +if len(sys.argv) < 3 or len(sys.argv) > 4:
> + scriptname = os.path.basename(sys.argv[0])
> + sys.stderr.write(
> +"""USAGE: %s <REPOS_PATH> <BACKUP_PATH> [ARCHIVE_TYPE]
> +""" % (scriptname,))
> sys.exit(1)
>
> # Path to repository
> @@ -53,12 +62,21 @@
> # revision.
> backup_dir = sys.argv[2]
>
> +# Do we want to create an archive of the backup
> +if len(sys.argv) < 4 or not archive_map.has_key(sys.argv[3]):
> + archive = None
> + ext_re = ""
> +else:
> + archive = sys.argv[3]
> + ext_re = "(" + re.escape(archive_map[archive]) + ")?"
> +
> ######################################################################
> # Helper functions
>
> def comparator(a, b):
> # We pass in filenames so there is never a case where they are equal.
> - regexp = re.compile("-(?P<revision>[0-9]+)(-(?P<increment>[0-9]+))?$")
> + regexp = re.compile("-(?P<revision>[0-9]+)(-(?P<increment>[0-9]+))?" +
> + ext_re + "$")
> matcha = regexp.search(a)
> matchb = regexp.search(b)
> reva = int(matcha.groupdict()['revision'])
> @@ -109,7 +127,8 @@
> # rather than start from 1 and increment because the starting
> # increments may have already been removed due to num_backups.
>
> -regexp = re.compile("^" + repo + "-" + youngest + "(-(?P<increment>[0-9]+))?$")
> +regexp = re.compile("^" + repo + "-" + youngest +
> + "(-(?P<increment>[0-9]+))?" + ext_re + "$")
> directory_list = os.listdir(backup_dir)
> young_list = filter(lambda x: regexp.search(x), directory_list)
> if young_list:
> @@ -134,16 +153,74 @@
> print "Done."
>
>
> -### Step 4: finally, remove all repository backups other than the last
> +### Step 4: Make an archive of the backup if required.
> +if archive:
> + archive_path = backup_subdir + archive_map[archive]
> +
> + old_cwd = os.getcwd()
> + try:
> + os.chdir(backup_dir)
> + if os.path.exists(archive_path):
> + os.remove(archive_path)
> +
> + print "Archiving backup to '" + archive_path + "'..."
> + if archive == 'gz' or archive == 'bz2':
> + try:
> + import tarfile
> + tar = tarfile.open(archive_path, 'w:' + archive)
> + tar.add(backup_subdir, os.path.basename(backup_subdir))
> + tar.close()
> + except ImportError:
> + err_code = -2
> + except tarfile.TarError:
> + err_code = -3
> +
> + elif archive == 'zip':
> + try:
> + import zipfile
> +
> + def add_to_zip(baton, dirname, names):
> + zip = baton[0]
> + root = os.path.join(baton[1], '')
> +
> + for file in names:
> + path = os.path.join(dirname, file)
> + if os.path.isfile(path):
> + zip.write(path, path[len(root):])
> + elif os.path.isdir(path) and os.path.islink(path):
> + os.path.walk(path, add_to_zip, (zip, path))
> +
> + zip = zipfile.ZipFile(archive_path, 'w', zipfile.ZIP_DEFLATED)
> + os.path.walk(backup_subdir, add_to_zip, (zip, backup_subdir))
> + zip.close()
> + except ImportError:
> + err_code = -4
> + except zipfile.error:
> + err_code = -5
> +
> + finally:
> + os.chdir(old_cwd)
> +
> + if err_code != 0:
> + print "Unable to archive the backup."
> + sys.exit(err_code)
> + else:
> + print "Archive created, removing backup '" + backup_subdir + "'..."
> + shutil.rmtree(backup_subdir)
> +
> +### Step 5: finally, remove all repository backups other than the last
> ### NUM_BACKUPS.
>
> if num_backups > 0:
> - regexp = re.compile("^" + repo + "-[0-9]+(-[0-9]+)?$")
> + regexp = re.compile("^" + repo + "-[0-9]+(-[0-9]+)?" + ext_re + "$")
> directory_list = os.listdir(backup_dir)
> old_list = filter(lambda x: regexp.search(x), directory_list)
> old_list.sort(comparator)
> del old_list[max(0,len(old_list)-num_backups):]
> for item in old_list:
> - old_backup_subdir = os.path.join(backup_dir, item)
> - print "Removing old backup: " + old_backup_subdir
> - shutil.rmtree(old_backup_subdir)
> + old_backup_item = os.path.join(backup_dir, item)
> + print "Removing old backup: " + old_backup_item
> + if os.path.isdir(old_backup_item):
> + shutil.rmtree(old_backup_item)
> + else:
> + os.remove(old_backup_item)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Feb 20 03:19:31 2006