Don't touch ref files manually

Messing with files manually doesn't work if the refs are packed. The
officially preferred way is to use git-update-ref, git-show-ref,
et.al. So do that.

As a consequence of this, we have some small behavior changes:

  * We used to not leave empty directories behind in the refs tree.
    But now that's all in the hands of git-update-ref, which does
    leave them behind. Tests that assumed the old behavior have been
    fixed.

  * We (that is, git-show-ref) no longer distinguish between a ref
    that doesn't exist and a ref that contains garbage. So the tests
    that assumed we'd fail when encountering a spurious ref with
    garbage in it have had to go through attitude readjustment.

Signed-off-by: Karl Hasselström <kha@treskal.com>

diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index 2fb5f5993a6302a9e7a9f2eb9bcf78abaabb94d1..75a9046ad95f41baad5863d8337dd0a736627314 100644 (file)
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -208,10 +208,7 @@ def func(parser, options, args):
         if len(args) != 0:
             parser.error('incorrect number of arguments')
 
         if len(args) != 0:
             parser.error('incorrect number of arguments')
 

-        branches = []
-        basepath = os.path.join(basedir.get(), 'refs', 'heads')
-        for path, files, dirs in walk_tree(basepath):
-            branches += [os.path.join(path, f) for f in files]
+        branches = git.get_heads()

         branches.sort()
 
         if branches:
         branches.sort()
 
         if branches:

diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index 14dbf678b6b36387f3c3bc6c6cfa6160a0cd3000..dddee85db38e2b422b00fe99937498d4abba9814 100644 (file)
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -42,9 +42,7 @@ def parse_rev(rev):
     """Parse a revision specification into its
     patchname@branchname//patch_id parts. If no branch name has a slash
     in it, also accept / instead of //."""
     """Parse a revision specification into its
     patchname@branchname//patch_id parts. If no branch name has a slash
     in it, also accept / instead of //."""

-    files, dirs = list_files_and_dirs(os.path.join(basedir.get(),
-                                                   'refs', 'heads'))
-    if len(dirs) != 0:
+    if '/' in ''.join(git.get_heads()):

         # We have branch names with / in them.
         branch_chars = r'[^@]'
         patch_id_mark = r'//'
         # We have branch names with / in them.
         branch_chars = r'[^@]'
         patch_id_mark = r'//'

diff --git a/stgit/git.py b/stgit/git.py
index 57c156eb1753fb74f512f4ccb6d3a434a2fee469..1807f69f2af2421ea69f707044471dc3a986e0b3 100644 (file)
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -273,6 +273,13 @@ def local_changes(verbose = True):
     """
     return len(tree_status(verbose = verbose)) != 0
 
     """
     return len(tree_status(verbose = verbose)) != 0
 

+def get_heads():
+    heads = []
+    for line in _output_lines(['git-show-ref', '--heads']):
+        m = re.match('^[0-9a-f]{40} refs/heads/(.+)$', line)
+        heads.append(m.group(1))
+    return heads
+

 # HEAD value cached
 __head = None
 
 # HEAD value cached
 __head = None
 


@@ -297,14 +304,16 @@ def set_head_file(ref):
     # head cache flushing is needed since we might have a different value
     # in the new head
     __clear_head_cache()
     # head cache flushing is needed since we might have a different value
     # in the new head
     __clear_head_cache()

-    if __run('git-symbolic-ref HEAD',
-             [os.path.join('refs', 'heads', ref)]) != 0:
+    if __run('git-symbolic-ref HEAD', ['refs/heads/%s' % ref]) != 0:

         raise GitException, 'Could not set head to "%s"' % ref
 
         raise GitException, 'Could not set head to "%s"' % ref
 

+def set_ref(ref, val):
+    """Point ref at a new commit object."""
+    if __run('git-update-ref', [ref, val]) != 0:
+        raise GitException, 'Could not update %s to "%s".' % (ref, val)
+

 def set_branch(branch, val):
 def set_branch(branch, val):

-    """Point branch at a new commit object."""
-    if __run('git-update-ref', [branch, val]) != 0:
-        raise GitException, 'Could not update %s to "%s".' % (branch, val)
+    set_ref('refs/heads/%s' % branch, val)

 
 def __set_head(val):
     """Sets the HEAD value
 
 def __set_head(val):
     """Sets the HEAD value


@@ -312,7 +321,7 @@ def __set_head(val):
     global __head
 
     if not __head or __head != val:
     global __head
 
     if not __head or __head != val:

-        set_branch('HEAD', val)
+        set_ref('HEAD', val)

         __head = val
 
     # only allow SHA1 hashes
         __head = val
 
     # only allow SHA1 hashes


@@ -338,16 +347,15 @@ def rev_parse(git_id):
     except GitException:
         raise GitException, 'Unknown revision: %s' % git_id
 
     except GitException:
         raise GitException, 'Unknown revision: %s' % git_id
 

+def ref_exists(ref):
+    try:
+        rev_parse(ref)
+        return True
+    except GitException:
+        return False
+

 def branch_exists(branch):
 def branch_exists(branch):

-    """Existence check for the named branch
-    """
-    branch = os.path.join('refs', 'heads', branch)
-    for line in _output_lines('git-rev-parse --symbolic --all 2>&1'):
-        if line.strip() == branch:
-            return True
-        if re.compile('[ |/]'+branch+' ').search(line):
-            raise GitException, 'Bogus branch: %s' % line
-    return False
+    return ref_exists('refs/heads/%s' % branch)

 
 def create_branch(new_branch, tree_id = None):
     """Create a new branch in the git repository
 
 def create_branch(new_branch, tree_id = None):
     """Create a new branch in the git repository


@@ -374,8 +382,7 @@ def switch_branch(new_branch):
     if not branch_exists(new_branch):
         raise GitException, 'Branch "%s" does not exist' % new_branch
 
     if not branch_exists(new_branch):
         raise GitException, 'Branch "%s" does not exist' % new_branch
 

-    tree_id = rev_parse(os.path.join('refs', 'heads', new_branch)
-                        + '^{commit}')
+    tree_id = rev_parse('refs/heads/%s^{commit}' % new_branch)

     if tree_id != get_head():
         refresh_index()
         if __run('git-read-tree -u -m', [get_head(), tree_id]) != 0:
     if tree_id != get_head():
         refresh_index()
         if __run('git-read-tree -u -m', [get_head(), tree_id]) != 0:


@@ -386,27 +393,33 @@ def switch_branch(new_branch):
     if os.path.isfile(os.path.join(basedir.get(), 'MERGE_HEAD')):
         os.remove(os.path.join(basedir.get(), 'MERGE_HEAD'))
 
     if os.path.isfile(os.path.join(basedir.get(), 'MERGE_HEAD')):
         os.remove(os.path.join(basedir.get(), 'MERGE_HEAD'))
 

+def delete_ref(ref):
+    if not ref_exists(ref):
+        raise GitException, '%s does not exist' % ref
+    sha1 = _output_one_line(['git-show-ref', '-s', ref])
+    if __run('git-update-ref -d %s %s' % (ref, sha1)):
+        raise GitException, 'Failed to delete ref %s' % ref
+

 def delete_branch(name):
 def delete_branch(name):

-    """Delete a git branch
-    """
-    if not branch_exists(name):
-        raise GitException, 'Branch "%s" does not exist' % name
-    remove_file_and_dirs(os.path.join(basedir.get(), 'refs', 'heads'),
-                         name)
+    delete_ref('refs/heads/%s' % name)

 
 

-def rename_branch(from_name, to_name):
-    """Rename a git branch
-    """
-    if not branch_exists(from_name):
-        raise GitException, 'Branch "%s" does not exist' % from_name
-    if branch_exists(to_name):
-        raise GitException, 'Branch "%s" already exists' % to_name
+def rename_ref(from_ref, to_ref):
+    if not ref_exists(from_ref):
+        raise GitException, '"%s" does not exist' % from_ref
+    if ref_exists(to_ref):
+        raise GitException, '"%s" already exists' % to_ref

 
 

+    sha1 = _output_one_line(['git-show-ref', '-s', from_ref])
+    if __run('git-update-ref %s %s %s' % (to_ref, sha1, '0'*40)):
+        raise GitException, 'Failed to create new ref %s' % to_ref
+    if __run('git-update-ref -d %s %s' % (from_ref, sha1)):
+        raise GitException, 'Failed to delete ref %s' % from_ref
+
+def rename_branch(from_name, to_name):
+    """Rename a git branch."""
+    rename_ref('refs/heads/%s' % from_name, 'refs/heads/%s' % to_name)

     if get_head_file() == from_name:
         set_head_file(to_name)
     if get_head_file() == from_name:
         set_head_file(to_name)

-    rename(os.path.join(basedir.get(), 'refs', 'heads'),
-           from_name, to_name)
-

     reflog_dir = os.path.join(basedir.get(), 'logs', 'refs', 'heads')
     if os.path.exists(reflog_dir) \
            and os.path.exists(os.path.join(reflog_dir, from_name)):
     reflog_dir = os.path.join(basedir.get(), 'logs', 'refs', 'heads')
     if os.path.exists(reflog_dir) \
            and os.path.exists(os.path.join(reflog_dir, from_name)):

diff --git a/stgit/stack.py b/stgit/stack.py
index c7569b21ee4cd2029cad8b1541f3cd550c9e80de..030562d9250e718bf4eba0e86890239e1799ece3 100644 (file)
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -142,14 +142,16 @@ class StgitObject:
 class Patch(StgitObject):
     """Basic patch implementation
     """
 class Patch(StgitObject):
     """Basic patch implementation
     """

-    def __init__(self, name, series_dir, refs_dir):
+    def __init_refs(self):
+        self.__top_ref = self.__refs_base + '/' + self.__name
+        self.__log_ref = self.__top_ref + '.log'
+
+    def __init__(self, name, series_dir, refs_base):

         self.__series_dir = series_dir
         self.__name = name
         self._set_dir(os.path.join(self.__series_dir, self.__name))
         self.__series_dir = series_dir
         self.__name = name
         self._set_dir(os.path.join(self.__series_dir, self.__name))

-        self.__refs_dir = refs_dir
-        self.__top_ref_file = os.path.join(self.__refs_dir, self.__name)
-        self.__log_ref_file = os.path.join(self.__refs_dir,
-                                           self.__name + '.log')
+        self.__refs_base = refs_base
+        self.__init_refs()

 
     def create(self):
         os.mkdir(self._dir())
 
     def create(self):
         os.mkdir(self._dir())


@@ -160,33 +162,31 @@ class Patch(StgitObject):
         for f in os.listdir(self._dir()):
             os.remove(os.path.join(self._dir(), f))
         os.rmdir(self._dir())
         for f in os.listdir(self._dir()):
             os.remove(os.path.join(self._dir(), f))
         os.rmdir(self._dir())

-        os.remove(self.__top_ref_file)
-        if os.path.exists(self.__log_ref_file):
-            os.remove(self.__log_ref_file)
+        git.delete_ref(self.__top_ref)
+        if git.ref_exists(self.__log_ref):
+            git.delete_ref(self.__log_ref)

 
     def get_name(self):
         return self.__name
 
     def rename(self, newname):
         olddir = self._dir()
 
     def get_name(self):
         return self.__name
 
     def rename(self, newname):
         olddir = self._dir()

-        old_top_ref_file = self.__top_ref_file
-        old_log_ref_file = self.__log_ref_file
+        old_top_ref = self.__top_ref
+        old_log_ref = self.__log_ref

         self.__name = newname
         self._set_dir(os.path.join(self.__series_dir, self.__name))
         self.__name = newname
         self._set_dir(os.path.join(self.__series_dir, self.__name))

-        self.__top_ref_file = os.path.join(self.__refs_dir, self.__name)
-        self.__log_ref_file = os.path.join(self.__refs_dir,
-                                           self.__name + '.log')
+        self.__init_refs()

 
 

+        git.rename_ref(old_top_ref, self.__top_ref)
+        if git.ref_exists(old_log_ref):
+            git.rename_ref(old_log_ref, self.__log_ref)

         os.rename(olddir, self._dir())
         os.rename(olddir, self._dir())

-        os.rename(old_top_ref_file, self.__top_ref_file)
-        if os.path.exists(old_log_ref_file):
-            os.rename(old_log_ref_file, self.__log_ref_file)

 
     def __update_top_ref(self, ref):
 
     def __update_top_ref(self, ref):

-        write_string(self.__top_ref_file, ref)
+        git.set_ref(self.__top_ref, ref)

 
     def __update_log_ref(self, ref):
 
     def __update_log_ref(self, ref):

-        write_string(self.__log_ref_file, ref)
+        git.set_ref(self.__log_ref, ref)

 
     def update_top_ref(self):
         top = self.get_top()
 
     def update_top_ref(self):
         top = self.get_top()


@@ -358,8 +358,7 @@ class Series(PatchSet):
         # initialized, but don't touch it if it isn't.
         self.update_to_current_format_version()
 
         # initialized, but don't touch it if it isn't.
         self.update_to_current_format_version()
 

-        self.__refs_dir = os.path.join(self._basedir(), 'refs', 'patches',
-                                       self.get_name())
+        self.__refs_base = 'refs/patches/%s' % self.get_name()

 
         self.__applied_file = os.path.join(self._dir(), 'applied')
         self.__unapplied_file = os.path.join(self._dir(), 'unapplied')
 
         self.__applied_file = os.path.join(self._dir(), 'applied')
         self.__unapplied_file = os.path.join(self._dir(), 'unapplied')


@@ -416,20 +415,22 @@ class Series(PatchSet):
         def rm(f):
             if os.path.exists(f):
                 os.remove(f)
         def rm(f):
             if os.path.exists(f):
                 os.remove(f)

+        def rm_ref(ref):
+            if git.ref_exists(ref):
+                git.delete_ref(ref)

 
         # Update 0 -> 1.
         if get_format_version() == 0:
             mkdir(os.path.join(branch_dir, 'trash'))
             patch_dir = os.path.join(branch_dir, 'patches')
             mkdir(patch_dir)
 
         # Update 0 -> 1.
         if get_format_version() == 0:
             mkdir(os.path.join(branch_dir, 'trash'))
             patch_dir = os.path.join(branch_dir, 'patches')
             mkdir(patch_dir)

-            refs_dir = os.path.join(self._basedir(), 'refs', 'patches', self.get_name())
-            mkdir(refs_dir)
+            refs_base = 'refs/patches/%s' % self.get_name()

             for patch in (file(os.path.join(branch_dir, 'unapplied')).readlines()
                           + file(os.path.join(branch_dir, 'applied')).readlines()):
                 patch = patch.strip()
                 os.rename(os.path.join(branch_dir, patch),
                           os.path.join(patch_dir, patch))
             for patch in (file(os.path.join(branch_dir, 'unapplied')).readlines()
                           + file(os.path.join(branch_dir, 'applied')).readlines()):
                 patch = patch.strip()
                 os.rename(os.path.join(branch_dir, patch),
                           os.path.join(patch_dir, patch))

-                Patch(patch, patch_dir, refs_dir).update_top_ref()
+                Patch(patch, patch_dir, refs_base).update_top_ref()

             set_format_version(1)
 
         # Update 1 -> 2.
             set_format_version(1)
 
         # Update 1 -> 2.


@@ -441,7 +442,7 @@ class Series(PatchSet):
                     config.set('branch.%s.description' % self.get_name(), desc)
                 rm(desc_file)
             rm(os.path.join(branch_dir, 'current'))
                     config.set('branch.%s.description' % self.get_name(), desc)
                 rm(desc_file)
             rm(os.path.join(branch_dir, 'current'))

-            rm(os.path.join(self._basedir(), 'refs', 'bases', self.get_name()))
+            rm_ref('refs/bases/%s' % self.get_name())

             set_format_version(2)
 
         # Make sure we're at the latest version.
             set_format_version(2)
 
         # Make sure we're at the latest version.


@@ -458,7 +459,7 @@ class Series(PatchSet):
     def get_patch(self, name):
         """Return a Patch object for the given name
         """
     def get_patch(self, name):
         """Return a Patch object for the given name
         """

-        return Patch(name, self.__patch_dir, self.__refs_dir)
+        return Patch(name, self.__patch_dir, self.__refs_base)

 
     def get_current_patch(self):
         """Return a Patch object representing the topmost patch, or
 
     def get_current_patch(self):
         """Return a Patch object representing the topmost patch, or


@@ -580,7 +581,7 @@ class Series(PatchSet):
         """
         if self.is_initialised():
             raise StackException, '%s already initialized' % self.get_name()
         """
         if self.is_initialised():
             raise StackException, '%s already initialized' % self.get_name()

-        for d in [self._dir(), self.__refs_dir]:
+        for d in [self._dir()]:

             if os.path.exists(d):
                 raise StackException, '%s already exists' % d
 
             if os.path.exists(d):
                 raise StackException, '%s already exists' % d
 


@@ -593,7 +594,6 @@ class Series(PatchSet):
 
         self.create_empty_field('applied')
         self.create_empty_field('unapplied')
 
         self.create_empty_field('applied')
         self.create_empty_field('unapplied')

-        os.makedirs(self.__refs_dir)

         self._set_field('orig-base', git.get_head())
 
         config.set(self.format_version_key(), str(FORMAT_VERSION))
         self._set_field('orig-base', git.get_head())
 
         config.set(self.format_version_key(), str(FORMAT_VERSION))


@@ -606,14 +606,18 @@ class Series(PatchSet):
         if to_stack.is_initialised():
             raise StackException, '"%s" already exists' % to_stack.get_name()
 
         if to_stack.is_initialised():
             raise StackException, '"%s" already exists' % to_stack.get_name()
 

+        patches = self.get_applied() + self.get_unapplied()
+

         git.rename_branch(self.get_name(), to_name)
 
         git.rename_branch(self.get_name(), to_name)
 

+        for patch in patches:
+            git.rename_ref('refs/patches/%s/%s' % (self.get_name(), patch),
+                           'refs/patches/%s/%s' % (to_name, patch))
+            git.rename_ref('refs/patches/%s/%s.log' % (self.get_name(), patch),
+                           'refs/patches/%s/%s.log' % (to_name, patch))

         if os.path.isdir(self._dir()):
             rename(os.path.join(self._basedir(), 'patches'),
                    self.get_name(), to_stack.get_name())
         if os.path.isdir(self._dir()):
             rename(os.path.join(self._basedir(), 'patches'),
                    self.get_name(), to_stack.get_name())

-        if os.path.exists(self.__refs_dir):
-            rename(os.path.join(self._basedir(), 'refs', 'patches'),
-                   self.get_name(), to_stack.get_name())

 
         # Rename the config section
         for k in ['branch.%s', 'branch.%s.stgit']:
 
         # Rename the config section
         for k in ['branch.%s', 'branch.%s.stgit']:


@@ -715,9 +719,9 @@ class Series(PatchSet):
                                      % self._dir())
 
             try:
                                      % self._dir())
 
             try:

-                os.removedirs(self.__refs_dir)
-            except OSError:
-                out.warn('Refs directory %s is not empty' % self.__refs_dir)
+                git.delete_branch(self.get_name())
+            except GitException:
+                out.warn('Could not delete branch "%s"' % self.get_name())

 
         # Cleanup parent informations
         # FIXME: should one day make use of git-config --section-remove,
 
         # Cleanup parent informations
         # FIXME: should one day make use of git-config --section-remove,

diff --git a/t/t0001-subdir-branches.sh b/t/t0001-subdir-branches.sh
index fac6339102ae458959455014f0e049914a11afa9..1685233401df71738c5ef935a594cc29939dab2e 100755 (executable)
--- a/t/t0001-subdir-branches.sh
+++ b/t/t0001-subdir-branches.sh
@@ -50,10 +50,11 @@ test_expect_success 'Create patch in slashy branch' \
 
 test_expect_success 'Rename branches' \
   'stg branch --rename master goo/gaa &&
 
 test_expect_success 'Rename branches' \
   'stg branch --rename master goo/gaa &&

-   test ! -e .git/refs/heads/master &&
+   ! git show-ref --verify --quiet refs/heads/master &&

    stg branch --rename goo/gaa x1/x2/x3/x4 &&
    stg branch --rename goo/gaa x1/x2/x3/x4 &&

-   test ! -e .git/refs/heads/goo &&
+   ! git show-ref --verify --quiet refs/heads/goo/gaa &&

    stg branch --rename x1/x2/x3/x4 servant &&
    stg branch --rename x1/x2/x3/x4 servant &&

-   test ! -e .git/refs/heads/x1'
+   ! git show-ref --verify --quiet refs/heads/x1/x2/x3/x4
+'

 
 test_done
 
 test_done

diff --git a/t/t1000-branch-create.sh b/t/t1000-branch-create.sh
index cca5504d4f19fc3ab02ea6fe0388a23745ce48c9..e920e934e241df0e2f3e2f714aaf14becfab7120 100755 (executable)
--- a/t/t1000-branch-create.sh
+++ b/t/t1000-branch-create.sh
@@ -12,27 +12,10 @@ Exercises the "stg branch" commands.
 
 stg init
 
 
 stg init
 

-test_expect_success \
-    'Create a spurious refs/patches/ entry' '
-    find .git -name foo | xargs rm -rf &&
-    touch .git/refs/patches/foo
-'
-
-test_expect_failure \
-    'Try to create an stgit branch with a spurious refs/patches/ entry' '
-    stg branch -c foo
-'
-
-test_expect_success \
-    'Check that no part of the branch was created' '
-    test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/patches/foo" &&
-    ( grep foo .git/HEAD; test $? = 1 )
-'
-

 test_expect_success \
     'Create a spurious patches/ entry' '
     find .git -name foo | xargs rm -rf &&
 test_expect_success \
     'Create a spurious patches/ entry' '
     find .git -name foo | xargs rm -rf &&

-    touch .git/patches/foo
+    mkdir -p .git/patches && touch .git/patches/foo

 '
 
 test_expect_failure \
 '
 
 test_expect_failure \


@@ -69,15 +52,4 @@ test_expect_success \
     touch .git/refs/heads/foo
 '
 
     touch .git/refs/heads/foo
 '
 

-test_expect_failure \
-    'Try to create an stgit branch with an invalid refs/heads/ entry' '
-    stg branch -c foo
-'
-
-test_expect_success \
-    'Check that no part of the branch was created' '
-    test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/heads/foo" &&
-    ( grep foo .git/HEAD; test $? = 1 )
-'
-

 test_done
 test_done

diff --git a/t/t1001-branch-rename.sh b/t/t1001-branch-rename.sh
index 28af8ae738da1ce848034a778552f8dfdd861a38..285440f00365be8486218f4d75e12f6125a8ce20 100755 (executable)
--- a/t/t1001-branch-rename.sh
+++ b/t/t1001-branch-rename.sh
@@ -26,7 +26,7 @@ test_expect_success \
     'Rename an stgit branch' \
     'stg branch -c buz &&
      stg branch -r foo bar &&
     'Rename an stgit branch' \
     'stg branch -c buz &&
      stg branch -r foo bar &&

-     test -z `find .git -name foo | tee /dev/stderr` &&
+     [ -z $(find .git -type f | grep foo | tee /dev/stderr) ] &&

      test -z $(git config -l | grep branch\\.foo)
 '
 
      test -z $(git config -l | grep branch\\.foo)
 '