Tarball names: Account for 'v[a-z]+', refactoring

klaus/utils.py

@@ -170,8 +170,18 @@ def extract_author_name(email):
     return email
 
 
+def is_hex_prefix(s):
+    if len(s) % 2:
+        s += '0'
+    try:
+        binascii.unhexlify(s)
+        return True
+    except binascii.Error:
+        return False
+
+
 def shorten_sha1(sha1):
-    if re.match(r'[a-z\d]{20,40}', sha1):
+    if 20 <= len(sha1) <= 40 and is_hex_prefix(sha1):
         sha1 = sha1[:7]
     return sha1
 
@@ -212,8 +222,6 @@ def replace_dupes(ls, replacement):
             last = elem
 
 
-
-
 def guess_git_revision():
     """Try to guess whether this instance of klaus is run directly from a klaus
     git checkout.  If it is, guess and return the currently checked-out commit
@@ -247,18 +255,17 @@ def escape_html(s):
 
 def tarball_basename(repo_name, rev):
     """Determine the name for a tarball."""
-    sanitized_rev = sanitize_branch_name(rev, chars='/')
-    # If the rev is a tag name that already starts with the
-    # repo name, skip it.
-    if sanitized_rev.startswith(repo_name + '-'):
-        return sanitized_rev
-    if sanitized_rev.startswith('v'):
-        return "%s-%s" % (repo_name, sanitized_rev[1:])
-    if len(sanitized_rev) == 40:
-        try:
-            binascii.unhexlify(sanitized_rev)
-        except binascii.Error:
-            pass
-        else:
-            return "%s@%s" % (repo_name, sanitized_rev)
-    return "%s-%s" % (repo_name, sanitized_rev)
+    rev = sanitize_branch_name(rev, chars='/')
+    if rev.startswith(repo_name + '-'):
+        # If the rev is a tag name that already starts with the repo name,
+        # skip it.
+        return rev
+    elif len(rev) >= 2 and rev[0] == 'v' and not rev[1].isalpha():
+        # If the rev is a tag name prefixed by a 'v', skip the 'v'.
+        # So, v-1.0 -> 1.0, v1.0 -> 1.0, but vanilla -> vanilla.
+        return "%s-%s" % (repo_name, rev[1:])
+    elif len(rev) == 40 and is_hex_prefix(rev):
+        # If the rev is a commit hash, simply use that.
+        return "%s@%s" % (repo_name, rev)
+    else:
+        return "%s-%s" % (repo_name, rev)

tests/test_utils.py

@@ -30,6 +30,7 @@ class TarballBasenameTests(unittest.TestCase):
             ('0.1', 'klaus-0.1'),
             ('b3e70e08344ca3f83cc7033ecdbefa90443d7d2e',
                 'klaus@b3e70e08344ca3f83cc7033ecdbefa90443d7d2e'),
+            ('vanilla', 'klaus-vanilla'),
             ]
         for (rev, basename) in examples:
             self.assertEqual(utils.tarball_basename('klaus', rev), basename)