{"thread":{"id":"9463","subject":"[StGIT PATCH 1/2] New test: make sure that StGIT can handle packed refs","startedAt":"2007-08-10T03:23:02Z","lastAt":"2007-08-23T14:04:02Z","messageCount":11,"participants":["Karl Hasselström","Catalin Marinas","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"50374","messageId":"20070810031949.19791.54562.stgit@yoghurt","threadId":"9463","inReplyTo":"20070809203957.GA14441@diana.vm.bytemark.co.uk","subject":"[StGIT PATCH 0/2] Teach StGIT to survive git-gc","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-10T03:23:02Z","receivedAt":"2007-08-10T03:23:02Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"... and here's the fix, and a test.\n\nNote that while this is fairly intrusive (and took quite some time to\ndebug), it's also a rather high-priority fix, since git-gc will break\nStGIT just fine even without the DAG patches.\n\nYou can also get this from git://repo.or.cz/stgit/kha.git.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"50372","messageId":"20070810032312.19791.68367.stgit@yoghurt","threadId":"9463","inReplyTo":"20070810031949.19791.54562.stgit@yoghurt","subject":"[StGIT PATCH 1/2] New test: make sure that StGIT can handle packed refs","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-10T03:23:12Z","receivedAt":"2007-08-10T03:23:12Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"It currently can't, which is why this test is needed. The situation\nhas recently taken a turn for the worse, since git-gc nowadays packs\nrefs by default.\n\n---\n\n t/t1004-pack-ref.sh |   24 ++++++++++++++++++++++++\n 1 files changed, 24 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t1004-pack-ref.sh b/t/t1004-pack-ref.sh\nnew file mode 100755\nindex 0000000..a5eb17c\n--- /dev/null\n+++ b/t/t1004-pack-ref.sh\n@@ -0,0 +1,24 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2007 Karl Hasselström\n+#\n+\n+test_description='Test that StGIT can handle packed refs'\n+\n+. ./test-lib.sh\n+stg init\n+\n+test_expect_success \\\n+    'Pack refs and make sure that we can still see them' '\n+    stg branch -c foo &&\n+    [ $(stg branch -l | tee /dev/stderr | wc -l) -eq 2 ] &&\n+    git pack-refs --all &&\n+    [ $(stg branch -l | tee /dev/stderr | wc -l) -eq 2 ]\n+'\n+\n+test_expect_success \\\n+    'Try to delete a branch whose ref has been packed' '\n+    stg branch -d master\n+'\n+\n+test_done\n"},{"id":"50373","messageId":"20070810032318.19791.70483.stgit@yoghurt","threadId":"9463","inReplyTo":"20070810031949.19791.54562.stgit@yoghurt","subject":"[StGIT PATCH 2/2] Don't touch ref files manually","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-10T03:23:18Z","receivedAt":"2007-08-10T03:23:18Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"Messing with files manually doesn't work if the refs are packed. The\nofficially preferred way is to use git-update-ref, git-show-ref,\net.al. So do that.\n\nAs a consequence of this, we have some small behavior changes:\n\n  * We used to not leave empty directories behind in the refs tree.\n    But now that's all in the hands of git-update-ref, which does\n    leave them behind. Tests that assumed the old behavior have been\n    fixed.\n\n  * We (that is, git-show-ref) no longer distinguish between a ref\n    that doesn't exist and a ref that contains garbage. So the tests\n    that assumed we'd fail when encountering a spurious ref with\n    garbage in it have had to go through attitude readjustment.\n\nSigned-off-by: Karl Hasselström <kha@treskal.com>\n\n---\n\n stgit/commands/branch.py   |    5 +--\n stgit/commands/common.py   |    4 +-\n stgit/git.py               |   79 ++++++++++++++++++++++++++------------------\n stgit/stack.py             |   70 +++++++++++++++++++++------------------\n t/t0001-subdir-branches.sh |    7 ++--\n t/t1000-branch-create.sh   |   30 +----------------\n t/t1001-branch-rename.sh   |    2 +\n 7 files changed, 91 insertions(+), 106 deletions(-)\n\ndiff --git a/stgit/commands/branch.py b/stgit/commands/branch.py\nindex 2fb5f59..75a9046 100644\n--- a/stgit/commands/branch.py\n+++ b/stgit/commands/branch.py\n@@ -208,10 +208,7 @@ def func(parser, options, args):\n         if len(args) != 0:\n             parser.error('incorrect number of arguments')\n \n-        branches = []\n-        basepath = os.path.join(basedir.get(), 'refs', 'heads')\n-        for path, files, dirs in walk_tree(basepath):\n-            branches += [os.path.join(path, f) for f in files]\n+        branches = git.get_heads()\n         branches.sort()\n \n         if branches:\ndiff --git a/stgit/commands/common.py b/stgit/commands/common.py\nindex 14dbf67..dddee85 100644\n--- a/stgit/commands/common.py\n+++ b/stgit/commands/common.py\n@@ -42,9 +42,7 @@ def parse_rev(rev):\n     \"\"\"Parse a revision specification into its\n     patchname@branchname//patch_id parts. If no branch name has a slash\n     in it, also accept / instead of //.\"\"\"\n-    files, dirs = list_files_and_dirs(os.path.join(basedir.get(),\n-                                                   'refs', 'heads'))\n-    if len(dirs) != 0:\n+    if '/' in ''.join(git.get_heads()):\n         # We have branch names with / in them.\n         branch_chars = r'[^@]'\n         patch_id_mark = r'//'\ndiff --git a/stgit/git.py b/stgit/git.py\nindex 72bf889..832a2dc 100644\n--- a/stgit/git.py\n+++ b/stgit/git.py\n@@ -270,6 +270,13 @@ def local_changes(verbose = True):\n     \"\"\"\n     return len(tree_status(verbose = verbose)) != 0\n \n+def get_heads():\n+    heads = []\n+    for line in _output_lines(['git-show-ref', '--heads']):\n+        m = re.match('^[0-9a-f]{40} refs/heads/(.+)$', line)\n+        heads.append(m.group(1))\n+    return heads\n+\n # HEAD value cached\n __head = None\n \n@@ -294,14 +301,16 @@ def set_head_file(ref):\n     # head cache flushing is needed since we might have a different value\n     # in the new head\n     __clear_head_cache()\n-    if __run('git-symbolic-ref HEAD',\n-             [os.path.join('refs', 'heads', ref)]) != 0:\n+    if __run('git-symbolic-ref HEAD', ['refs/heads/%s' % ref]) != 0:\n         raise GitException, 'Could not set head to \"%s\"' % ref\n \n+def set_ref(ref, val):\n+    \"\"\"Point ref at a new commit object.\"\"\"\n+    if __run('git-update-ref', [ref, val]) != 0:\n+        raise GitException, 'Could not update %s to \"%s\".' % (ref, val)\n+\n def set_branch(branch, val):\n-    \"\"\"Point branch at a new commit object.\"\"\"\n-    if __run('git-update-ref', [branch, val]) != 0:\n-        raise GitException, 'Could not update %s to \"%s\".' % (branch, val)\n+    set_ref('refs/heads/%s' % branch, val)\n \n def __set_head(val):\n     \"\"\"Sets the HEAD value\n@@ -309,7 +318,7 @@ def __set_head(val):\n     global __head\n \n     if not __head or __head != val:\n-        set_branch('HEAD', val)\n+        set_ref('HEAD', val)\n         __head = val\n \n     # only allow SHA1 hashes\n@@ -335,16 +344,15 @@ def rev_parse(git_id):\n     except GitException:\n         raise GitException, 'Unknown revision: %s' % git_id\n \n+def ref_exists(ref):\n+    try:\n+        rev_parse(ref)\n+        return True\n+    except GitException:\n+        return False\n+\n def branch_exists(branch):\n-    \"\"\"Existence check for the named branch\n-    \"\"\"\n-    branch = os.path.join('refs', 'heads', branch)\n-    for line in _output_lines('git-rev-parse --symbolic --all 2>&1'):\n-        if line.strip() == branch:\n-            return True\n-        if re.compile('[ |/]'+branch+' ').search(line):\n-            raise GitException, 'Bogus branch: %s' % line\n-    return False\n+    return ref_exists('refs/heads/%s' % branch)\n \n def create_branch(new_branch, tree_id = None):\n     \"\"\"Create a new branch in the git repository\n@@ -371,8 +379,7 @@ def switch_branch(new_branch):\n     if not branch_exists(new_branch):\n         raise GitException, 'Branch \"%s\" does not exist' % new_branch\n \n-    tree_id = rev_parse(os.path.join('refs', 'heads', new_branch)\n-                        + '^{commit}')\n+    tree_id = rev_parse('refs/heads/%s^{commit}' % new_branch)\n     if tree_id != get_head():\n         refresh_index()\n         if __run('git-read-tree -u -m', [get_head(), tree_id]) != 0:\n@@ -383,27 +390,33 @@ def switch_branch(new_branch):\n     if os.path.isfile(os.path.join(basedir.get(), 'MERGE_HEAD')):\n         os.remove(os.path.join(basedir.get(), 'MERGE_HEAD'))\n \n+def delete_ref(ref):\n+    if not ref_exists(ref):\n+        raise GitException, '%s does not exist' % ref\n+    sha1 = _output_one_line(['git-show-ref', '-s', ref])\n+    if __run('git-update-ref -d %s %s' % (ref, sha1)):\n+        raise GitException, 'Failed to delete ref %s' % ref\n+\n def delete_branch(name):\n-    \"\"\"Delete a git branch\n-    \"\"\"\n-    if not branch_exists(name):\n-        raise GitException, 'Branch \"%s\" does not exist' % name\n-    remove_file_and_dirs(os.path.join(basedir.get(), 'refs', 'heads'),\n-                         name)\n+    delete_ref('refs/heads/%s' % name)\n \n-def rename_branch(from_name, to_name):\n-    \"\"\"Rename a git branch\n-    \"\"\"\n-    if not branch_exists(from_name):\n-        raise GitException, 'Branch \"%s\" does not exist' % from_name\n-    if branch_exists(to_name):\n-        raise GitException, 'Branch \"%s\" already exists' % to_name\n+def rename_ref(from_ref, to_ref):\n+    if not ref_exists(from_ref):\n+        raise GitException, '\"%s\" does not exist' % from_ref\n+    if ref_exists(to_ref):\n+        raise GitException, '\"%s\" already exists' % to_ref\n \n+    sha1 = _output_one_line(['git-show-ref', '-s', from_ref])\n+    if __run('git-update-ref %s %s %s' % (to_ref, sha1, '0'*40)):\n+        raise GitException, 'Failed to create new ref %s' % to_ref\n+    if __run('git-update-ref -d %s %s' % (from_ref, sha1)):\n+        raise GitException, 'Failed to delete ref %s' % from_ref\n+\n+def rename_branch(from_name, to_name):\n+    \"\"\"Rename a git branch.\"\"\"\n+    rename_ref('refs/heads/%s' % from_name, 'refs/heads/%s' % to_name)\n     if get_head_file() == from_name:\n         set_head_file(to_name)\n-    rename(os.path.join(basedir.get(), 'refs', 'heads'),\n-           from_name, to_name)\n-\n     reflog_dir = os.path.join(basedir.get(), 'logs', 'refs', 'heads')\n     if os.path.exists(reflog_dir) \\\n            and os.path.exists(os.path.join(reflog_dir, from_name)):\ndiff --git a/stgit/stack.py b/stgit/stack.py\nindex a54a3b2..3880a94 100644\n--- a/stgit/stack.py\n+++ b/stgit/stack.py\n@@ -142,14 +142,16 @@ class StgitObject:\n class Patch(StgitObject):\n     \"\"\"Basic patch implementation\n     \"\"\"\n-    def __init__(self, name, series_dir, refs_dir):\n+    def __init_refs(self):\n+        self.__top_ref = self.__refs_base + '/' + self.__name\n+        self.__log_ref = self.__top_ref + '.log'\n+\n+    def __init__(self, name, series_dir, refs_base):\n         self.__series_dir = series_dir\n         self.__name = name\n         self._set_dir(os.path.join(self.__series_dir, self.__name))\n-        self.__refs_dir = refs_dir\n-        self.__top_ref_file = os.path.join(self.__refs_dir, self.__name)\n-        self.__log_ref_file = os.path.join(self.__refs_dir,\n-                                           self.__name + '.log')\n+        self.__refs_base = refs_base\n+        self.__init_refs()\n \n     def create(self):\n         os.mkdir(self._dir())\n@@ -160,33 +162,31 @@ class Patch(StgitObject):\n         for f in os.listdir(self._dir()):\n             os.remove(os.path.join(self._dir(), f))\n         os.rmdir(self._dir())\n-        os.remove(self.__top_ref_file)\n-        if os.path.exists(self.__log_ref_file):\n-            os.remove(self.__log_ref_file)\n+        git.delete_ref(self.__top_ref)\n+        if git.ref_exists(self.__log_ref):\n+            git.delete_ref(self.__log_ref)\n \n     def get_name(self):\n         return self.__name\n \n     def rename(self, newname):\n         olddir = self._dir()\n-        old_top_ref_file = self.__top_ref_file\n-        old_log_ref_file = self.__log_ref_file\n+        old_top_ref = self.__top_ref\n+        old_log_ref = self.__log_ref\n         self.__name = newname\n         self._set_dir(os.path.join(self.__series_dir, self.__name))\n-        self.__top_ref_file = os.path.join(self.__refs_dir, self.__name)\n-        self.__log_ref_file = os.path.join(self.__refs_dir,\n-                                           self.__name + '.log')\n+        self.__init_refs()\n \n+        git.rename_ref(old_top_ref, self.__top_ref)\n+        if git.ref_exists(old_log_ref):\n+            git.rename_ref(old_log_ref, self.__log_ref)\n         os.rename(olddir, self._dir())\n-        os.rename(old_top_ref_file, self.__top_ref_file)\n-        if os.path.exists(old_log_ref_file):\n-            os.rename(old_log_ref_file, self.__log_ref_file)\n \n     def __update_top_ref(self, ref):\n-        write_string(self.__top_ref_file, ref)\n+        git.set_ref(self.__top_ref, ref)\n \n     def __update_log_ref(self, ref):\n-        write_string(self.__log_ref_file, ref)\n+        git.set_ref(self.__log_ref, ref)\n \n     def update_top_ref(self):\n         top = self.get_top()\n@@ -358,8 +358,7 @@ class Series(PatchSet):\n         # initialized, but don't touch it if it isn't.\n         self.update_to_current_format_version()\n \n-        self.__refs_dir = os.path.join(self._basedir(), 'refs', 'patches',\n-                                       self.get_name())\n+        self.__refs_base = 'refs/patches/%s' % self.get_name()\n \n         self.__applied_file = os.path.join(self._dir(), 'applied')\n         self.__unapplied_file = os.path.join(self._dir(), 'unapplied')\n@@ -416,20 +415,22 @@ class Series(PatchSet):\n         def rm(f):\n             if os.path.exists(f):\n                 os.remove(f)\n+        def rm_ref(ref):\n+            if git.ref_exists(ref):\n+                git.delete_ref(ref)\n \n         # Update 0 -> 1.\n         if get_format_version() == 0:\n             mkdir(os.path.join(branch_dir, 'trash'))\n             patch_dir = os.path.join(branch_dir, 'patches')\n             mkdir(patch_dir)\n-            refs_dir = os.path.join(self._basedir(), 'refs', 'patches', self.get_name())\n-            mkdir(refs_dir)\n+            refs_base = 'refs/patches/%s' % self.get_name()\n             for patch in (file(os.path.join(branch_dir, 'unapplied')).readlines()\n                           + file(os.path.join(branch_dir, 'applied')).readlines()):\n                 patch = patch.strip()\n                 os.rename(os.path.join(branch_dir, patch),\n                           os.path.join(patch_dir, patch))\n-                Patch(patch, patch_dir, refs_dir).update_top_ref()\n+                Patch(patch, patch_dir, refs_base).update_top_ref()\n             set_format_version(1)\n \n         # Update 1 -> 2.\n@@ -441,7 +442,7 @@ class Series(PatchSet):\n                     config.set('branch.%s.description' % self.get_name(), desc)\n                 rm(desc_file)\n             rm(os.path.join(branch_dir, 'current'))\n-            rm(os.path.join(self._basedir(), 'refs', 'bases', self.get_name()))\n+            rm_ref('refs/bases/%s' % self.get_name())\n             set_format_version(2)\n \n         # Make sure we're at the latest version.\n@@ -458,7 +459,7 @@ class Series(PatchSet):\n     def get_patch(self, name):\n         \"\"\"Return a Patch object for the given name\n         \"\"\"\n-        return Patch(name, self.__patch_dir, self.__refs_dir)\n+        return Patch(name, self.__patch_dir, self.__refs_base)\n \n     def get_current_patch(self):\n         \"\"\"Return a Patch object representing the topmost patch, or\n@@ -580,7 +581,7 @@ class Series(PatchSet):\n         \"\"\"\n         if self.is_initialised():\n             raise StackException, '%s already initialized' % self.get_name()\n-        for d in [self._dir(), self.__refs_dir]:\n+        for d in [self._dir()]:\n             if os.path.exists(d):\n                 raise StackException, '%s already exists' % d\n \n@@ -593,7 +594,6 @@ class Series(PatchSet):\n \n         self.create_empty_field('applied')\n         self.create_empty_field('unapplied')\n-        os.makedirs(self.__refs_dir)\n         self._set_field('orig-base', git.get_head())\n \n         config.set(self.format_version_key(), str(FORMAT_VERSION))\n@@ -606,14 +606,18 @@ class Series(PatchSet):\n         if to_stack.is_initialised():\n             raise StackException, '\"%s\" already exists' % to_stack.get_name()\n \n+        patches = self.get_applied() + self.get_unapplied()\n+\n         git.rename_branch(self.get_name(), to_name)\n \n+        for patch in patches:\n+            git.rename_ref('refs/patches/%s/%s' % (self.get_name(), patch),\n+                           'refs/patches/%s/%s' % (to_name, patch))\n+            git.rename_ref('refs/patches/%s/%s.log' % (self.get_name(), patch),\n+                           'refs/patches/%s/%s.log' % (to_name, patch))\n         if os.path.isdir(self._dir()):\n             rename(os.path.join(self._basedir(), 'patches'),\n                    self.get_name(), to_stack.get_name())\n-        if os.path.exists(self.__refs_dir):\n-            rename(os.path.join(self._basedir(), 'refs', 'patches'),\n-                   self.get_name(), to_stack.get_name())\n \n         # Rename the config section\n         config.rename_section(\"branch.%s\" % self.get_name(),\n@@ -715,9 +719,9 @@ class Series(PatchSet):\n                                      % self._dir())\n \n             try:\n-                os.removedirs(self.__refs_dir)\n-            except OSError:\n-                out.warn('Refs directory %s is not empty' % self.__refs_dir)\n+                git.delete_branch(self.get_name())\n+            except GitException:\n+                out.warn('Could not delete branch \"%s\"' % self.get_name())\n \n         # Cleanup parent informations\n         # FIXME: should one day make use of git-config --section-remove,\ndiff --git a/t/t0001-subdir-branches.sh b/t/t0001-subdir-branches.sh\nindex fac6339..1685233 100755\n--- a/t/t0001-subdir-branches.sh\n+++ b/t/t0001-subdir-branches.sh\n@@ -50,10 +50,11 @@ test_expect_success 'Create patch in slashy branch' \\\n \n test_expect_success 'Rename branches' \\\n   'stg branch --rename master goo/gaa &&\n-   test ! -e .git/refs/heads/master &&\n+   ! git show-ref --verify --quiet refs/heads/master &&\n    stg branch --rename goo/gaa x1/x2/x3/x4 &&\n-   test ! -e .git/refs/heads/goo &&\n+   ! git show-ref --verify --quiet refs/heads/goo/gaa &&\n    stg branch --rename x1/x2/x3/x4 servant &&\n-   test ! -e .git/refs/heads/x1'\n+   ! git show-ref --verify --quiet refs/heads/x1/x2/x3/x4\n+'\n \n test_done\ndiff --git a/t/t1000-branch-create.sh b/t/t1000-branch-create.sh\nindex cca5504..e920e93 100755\n--- a/t/t1000-branch-create.sh\n+++ b/t/t1000-branch-create.sh\n@@ -13,26 +13,9 @@ Exercises the \"stg branch\" commands.\n stg init\n \n test_expect_success \\\n-    'Create a spurious refs/patches/ entry' '\n-    find .git -name foo | xargs rm -rf &&\n-    touch .git/refs/patches/foo\n-'\n-\n-test_expect_failure \\\n-    'Try to create an stgit branch with a spurious refs/patches/ entry' '\n-    stg branch -c foo\n-'\n-\n-test_expect_success \\\n-    'Check that no part of the branch was created' '\n-    test \"`find .git -name foo | tee /dev/stderr`\" = \".git/refs/patches/foo\" &&\n-    ( grep foo .git/HEAD; test $? = 1 )\n-'\n-\n-test_expect_success \\\n     'Create a spurious patches/ entry' '\n     find .git -name foo | xargs rm -rf &&\n-    touch .git/patches/foo\n+    mkdir -p .git/patches && touch .git/patches/foo\n '\n \n test_expect_failure \\\n@@ -69,15 +52,4 @@ test_expect_success \\\n     touch .git/refs/heads/foo\n '\n \n-test_expect_failure \\\n-    'Try to create an stgit branch with an invalid refs/heads/ entry' '\n-    stg branch -c foo\n-'\n-\n-test_expect_success \\\n-    'Check that no part of the branch was created' '\n-    test \"`find .git -name foo | tee /dev/stderr`\" = \".git/refs/heads/foo\" &&\n-    ( grep foo .git/HEAD; test $? = 1 )\n-'\n-\n test_done\ndiff --git a/t/t1001-branch-rename.sh b/t/t1001-branch-rename.sh\nindex 28da15c..4e1ec84 100755\n--- a/t/t1001-branch-rename.sh\n+++ b/t/t1001-branch-rename.sh\n@@ -26,7 +26,7 @@ test_expect_success \\\n     'Rename an stgit branch' \\\n     'stg branch -c buz &&\n      stg branch -r foo bar &&\n-     test -z `find .git -name foo | tee /dev/stderr`\n+     [ -z $(find .git -type f | grep foo | tee /dev/stderr) ]\n '\n \n test_done\n"},{"id":"51166","messageId":"b0943d9e0708210623h112faa42p97bba06bc9fab774@mail.gmail.com","threadId":"9463","inReplyTo":"20070810032318.19791.70483.stgit@yoghurt","subject":"Re: [StGIT PATCH 2/2] Don't touch ref files manually","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2007-08-21T13:23:29Z","receivedAt":"2007-08-21T13:23:29Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"Hi Karl,\n\nOn 10/08/07, Karl Hasselström <kha@treskal.com> wrote:\n> Messing with files manually doesn't work if the refs are packed. The\n> officially preferred way is to use git-update-ref, git-show-ref,\n> et.al. So do that.\n\nThanks for this patch. I'll apply it but I have some questions below.\n\n> @@ -294,14 +301,16 @@ def set_head_file(ref):\n>      # head cache flushing is needed since we might have a different value\n>      # in the new head\n>      __clear_head_cache()\n> -    if __run('git-symbolic-ref HEAD',\n> -             [os.path.join('refs', 'heads', ref)]) != 0:\n> +    if __run('git-symbolic-ref HEAD', ['refs/heads/%s' % ref]) != 0:\n>          raise GitException, 'Could not set head to \"%s\"' % ref\n\nWhy replacing os.path.join with a single string? I think the former is\nmore portable if, in the future, someone will try to run StGIT on\nWindows (and GIT would be compiled directly, without cygwin).\n\n> diff --git a/t/t1000-branch-create.sh b/t/t1000-branch-create.sh\n> index cca5504..e920e93 100755\n> --- a/t/t1000-branch-create.sh\n> +++ b/t/t1000-branch-create.sh\n> @@ -13,26 +13,9 @@ Exercises the \"stg branch\" commands.\n>  stg init\n>\n>  test_expect_success \\\n> -    'Create a spurious refs/patches/ entry' '\n> -    find .git -name foo | xargs rm -rf &&\n> -    touch .git/refs/patches/foo\n\nI haven't tried this test but is there any problem if we keep them?\n\nThanks.\n\n-- \nCatalin\n"},{"id":"51173","messageId":"20070821155845.GA16221@diana.vm.bytemark.co.uk","threadId":"9463","inReplyTo":"b0943d9e0708210623h112faa42p97bba06bc9fab774@mail.gmail.com","subject":"Re: [StGIT PATCH 2/2] Don't touch ref files manually","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-21T15:58:45Z","receivedAt":"2007-08-21T15:58:45Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-08-21 14:23:29 +0100, Catalin Marinas wrote:\n\n> On 10/08/07, Karl Hasselström <kha@treskal.com> wrote:\n>\n> > Messing with files manually doesn't work if the refs are packed.\n> > The officially preferred way is to use git-update-ref,\n> > git-show-ref, et.al. So do that.\n>\n> Thanks for this patch. I'll apply it but I have some questions\n> below.\n>\n> > @@ -294,14 +301,16 @@ def set_head_file(ref):\n> >      # head cache flushing is needed since we might have a different value\n> >      # in the new head\n> >      __clear_head_cache()\n> > -    if __run('git-symbolic-ref HEAD',\n> > -             [os.path.join('refs', 'heads', ref)]) != 0:\n> > +    if __run('git-symbolic-ref HEAD', ['refs/heads/%s' % ref]) != 0:\n> >          raise GitException, 'Could not set head to \"%s\"' % ref\n>\n> Why replacing os.path.join with a single string? I think the former\n> is more portable if, in the future, someone will try to run StGIT on\n> Windows (and GIT would be compiled directly, without cygwin).\n\nYes, that's the case iff it is a pathname in the file system. But I\nthink forward slashes are used even on Windows in this case --\notherwise too mmuch configuration would have to be changed. This is\nwhat I _think_, however; I haven't actually checked.\n\n> > diff --git a/t/t1000-branch-create.sh b/t/t1000-branch-create.sh\n> > index cca5504..e920e93 100755\n> > --- a/t/t1000-branch-create.sh\n> > +++ b/t/t1000-branch-create.sh\n> > @@ -13,26 +13,9 @@ Exercises the \"stg branch\" commands.\n> >  stg init\n> >\n> >  test_expect_success \\\n> > -    'Create a spurious refs/patches/ entry' '\n> > -    find .git -name foo | xargs rm -rf &&\n> > -    touch .git/refs/patches/foo\n>\n> I haven't tried this test but is there any problem if we keep them?\n\nYes.\n\nThis test started to fail after my change. I don't remember exactly,\nbut I think it was git-symbolic-ref actually reading the file, finding\nno ref there, and ignoring it. So this empty file didn't mess up the\nmachinery anymore, and since this isn't a common usecase anyway, and\nsince this change is to rely on git for all the low-level ref\nmanipulation, I just deleted the test.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"51174","messageId":"20070821160904.GB16221@diana.vm.bytemark.co.uk","threadId":"9463","inReplyTo":"20070821155845.GA16221@diana.vm.bytemark.co.uk","subject":"Re: [StGIT PATCH 2/2] Don't touch ref files manually","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-21T16:09:04Z","receivedAt":"2007-08-21T16:09:04Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-08-21 17:58:45 +0200, Karl Hasselström wrote:\n\n> On 2007-08-21 14:23:29 +0100, Catalin Marinas wrote:\n>\n> > On 10/08/07, Karl Hasselström <kha@treskal.com> wrote:\n> >\n> > > diff --git a/t/t1000-branch-create.sh b/t/t1000-branch-create.sh\n> > > index cca5504..e920e93 100755\n> > > --- a/t/t1000-branch-create.sh\n> > > +++ b/t/t1000-branch-create.sh\n> > > @@ -13,26 +13,9 @@ Exercises the \"stg branch\" commands.\n> > >  stg init\n> > >\n> > >  test_expect_success \\\n> > > -    'Create a spurious refs/patches/ entry' '\n> > > -    find .git -name foo | xargs rm -rf &&\n> > > -    touch .git/refs/patches/foo\n> >\n> > I haven't tried this test but is there any problem if we keep\n> > them?\n>\n> Yes.\n>\n> This test started to fail after my change. I don't remember exactly,\n> but I think it was git-symbolic-ref actually reading the file,\n> finding no ref there, and ignoring it. So this empty file didn't\n> mess up the machinery anymore, and since this isn't a common usecase\n> anyway, and since this change is to rely on git for all the\n> low-level ref manipulation, I just deleted the test.\n\nBTW, I just saw that this was explained by the second bullet point in\nthe commit message. :-)\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"51175","messageId":"46CB0EEE.D46769FE@eudaptics.com","threadId":"9463","inReplyTo":"20070821155845.GA16221@diana.vm.bytemark.co.uk","subject":"Re: [StGIT PATCH 2/2] Don't touch ref files manually","fromName":"Johannes Sixt","fromEmail":"j.sixt@eudaptics.com","sentAt":"2007-08-21T16:12:30Z","receivedAt":"2007-08-21T16:12:30Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Karl Hasselström wrote:\n> On 2007-08-21 14:23:29 +0100, Catalin Marinas wrote:\n> > On 10/08/07, Karl Hasselström <kha@treskal.com> wrote:\n> > > @@ -294,14 +301,16 @@ def set_head_file(ref):\n> > >      # head cache flushing is needed since we might have a different value\n> > >      # in the new head\n> > >      __clear_head_cache()\n> > > -    if __run('git-symbolic-ref HEAD',\n> > > -             [os.path.join('refs', 'heads', ref)]) != 0:\n> > > +    if __run('git-symbolic-ref HEAD', ['refs/heads/%s' % ref]) != 0:\n> > >          raise GitException, 'Could not set head to \"%s\"' % ref\n> >\n> > Why replacing os.path.join with a single string? I think the former\n> > is more portable if, in the future, someone will try to run StGIT on\n> > Windows (and GIT would be compiled directly, without cygwin).\n> \n> Yes, that's the case iff it is a pathname in the file system. But I\n> think forward slashes are used even on Windows in this case --\n> otherwise too mmuch configuration would have to be changed. This is\n> what I _think_, however; I haven't actually checked.\n\nThere are some instances where the MinGW port accepts backslashes in ref\nnames, but you are on the safe side with forward slashes.\n\n-- Hannes\n"},{"id":"51176","messageId":"20070821163843.GA17045@diana.vm.bytemark.co.uk","threadId":"9463","inReplyTo":"46CB0EEE.D46769FE@eudaptics.com","subject":"Re: [StGIT PATCH 2/2] Don't touch ref files manually","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-21T16:38:43Z","receivedAt":"2007-08-21T16:38:43Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-08-21 18:12:30 +0200, Johannes Sixt wrote:\n\n> There are some instances where the MinGW port accepts backslashes in\n> ref names, but you are on the safe side with forward slashes.\n\nThis was just what I was expecting. Thanks for the confirmation!\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"51177","messageId":"20070821164629.GB17045@diana.vm.bytemark.co.uk","threadId":"9463","inReplyTo":"b0943d9e0708210623h112faa42p97bba06bc9fab774@mail.gmail.com","subject":"Re: [StGIT PATCH 2/2] Don't touch ref files manually","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-21T16:46:29Z","receivedAt":"2007-08-21T16:46:29Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-08-21 14:23:29 +0100, Catalin Marinas wrote:\n\n> Thanks for this patch.\n\nTwo things:\n\n  * The test in your tree has the latin1 form of my name in it,\n    instead of utf8. (I noticed because I got a conflict when I\n    rebased onto your master.)\n\n  * The test suite now fails on your master (since you've applied just\n    the test and not the fix). Please don't do that; it makes\n    development needlessly hard. (To be fair, this is partly my fault,\n    since I posted the fixin a separate patch. I'll change my ways.)\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"51198","messageId":"20070821204831.GA20965@diana.vm.bytemark.co.uk","threadId":"9463","inReplyTo":"20070821164629.GB17045@diana.vm.bytemark.co.uk","subject":"Re: [StGIT PATCH 2/2] Don't touch ref files manually","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-08-21T20:48:31Z","receivedAt":"2007-08-21T20:48:31Z","isPatch":true,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-08-21 18:46:29 +0200, Karl Hasselström wrote:\n\n>   * The test in your tree has the latin1 form of my name in it,\n>     instead of utf8. (I noticed because I got a conflict when I\n>     rebased onto your master.)\n\nThat seems to have happened to the other new test too.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"51370","messageId":"b0943d9e0708230704k78281ce4jc8dcb7078fea20ed@mail.gmail.com","threadId":"9463","inReplyTo":"20070821164629.GB17045@diana.vm.bytemark.co.uk","subject":"Re: [StGIT PATCH 2/2] Don't touch ref files manually","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@gmail.com","sentAt":"2007-08-23T14:04:02Z","receivedAt":"2007-08-23T14:04:02Z","isPatch":true,"sender":{"key":"catalin.marinas@gmail.com","avatar":null},"body":"On 21/08/07, Karl Hasselström <kha@treskal.com> wrote:\n> On 2007-08-21 14:23:29 +0100, Catalin Marinas wrote:\n>\n> > Thanks for this patch.\n>\n> Two things:\n>\n>   * The test in your tree has the latin1 form of my name in it,\n>     instead of utf8. (I noticed because I got a conflict when I\n>     rebased onto your master.)\n\nI imported it on a different PC (at work) where I think the locale\nsupport isn't properly set up. Xterm accepted a copy-paste operation\nwith your name and I passed it through emacs which probably broke it.\nThe From: header had a different encoding and hence the author was\ncorrect.\n\nI'll try to do it on my home PC only (or just pull from your repo :-)).\n\n>   * The test suite now fails on your master (since you've applied just\n>     the test and not the fix). Please don't do that; it makes\n>     development needlessly hard. (To be fair, this is partly my fault,\n>     since I posted the fixin a separate patch. I'll change my ways.)\n\nI think it's OK to send separate patches and I'm OK with a test\nfailing in a development stage. The 2nd patch didn't apply cleanly and\ndidn't have time to see what was wrong. I'll push it tonight, please\nhave a look for things I might have missed.\n\nThanks.\n\n-- \nCatalin\n"}]}