[PATCH 3/9] Add a couple of safety checks to series creation
- we must check first whether the operation can complete, instead of
bombing out halfway. That means checking that nothing will prevent
the creation of stgit data (note that calling is_initialised() is
not enough, as it does not catch a bogus file), which is tested by
the 3 first couple of testcases, and fixed in stack.py
- creating the git branch unconditionally before knowing whether
creation of the stgit stuff can be completed is a problem as well:
being atomic would be much much better. To emulate atomicity (which
comeds in the next patch), this patch does a somewhat dirty hack to
branch.py: we first attempt to create the stgit stuff, and if that
succeeds, we create the git branch: it is much easier to do at first
a quick check that the latter will succeed. Testcase 7/8 ensure
that such a safety check has not been forgotten.
- when git already reports a problem with that head we would like to
create, we should catch it. Testcase 9/10 creates such a situation,
and the fix to git.py allows to catch the error spit out by
git-rev-parse. I cannot tell why the stderr lines were not included
by the Popen3 object.
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
stgit/commands/branch.py | 5 ++-
stgit/git.py | 4 ++
stgit/stack.py | 7 +++-
t/t1000-branch-create.sh | 82 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 95 insertions(+), 3 deletions(-)
diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index c4b5945..be501a8 100644
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -126,8 +126,11 @@ def func(parser, options, args):
if len(args) == 2:
tree_id = git_id(args[1])
- git.create_branch(args[0], tree_id)
+ if git.branch_exists(args[0]):
+ raise CmdException, 'Branch "%s" already exists' % args[0]
+
stack.Series(args[0]).init()
+ git.create_branch(args[0], tree_id)
print 'Branch "%s" created.' % args[0]
return
diff --git a/stgit/git.py b/stgit/git.py
index d75b54e..8523455 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -272,9 +272,11 @@ def rev_parse(git_id):
def branch_exists(branch):
"""Existence check for the named branch
"""
- for line in _output_lines(['git-rev-parse', '--symbolic', '--all']):
+ 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
def create_branch(new_branch, tree_id = None):
diff --git a/stgit/stack.py b/stgit/stack.py
index f4d7490..c14e029 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -431,8 +431,13 @@ class Series:
"""
bases_dir = os.path.join(self.__base_dir, 'refs', 'bases')
- if self.is_initialised():
+ if os.path.exists(self.__patch_dir):
raise StackException, self.__patch_dir + ' already exists'
+ if os.path.exists(self.__refs_dir):
+ raise StackException, self.__refs_dir + ' already exists'
+ if os.path.exists(self.__base_file):
+ raise StackException, self.__base_file + ' already exists'
+
os.makedirs(self.__patch_dir)
if not os.path.isdir(bases_dir):
diff --git a/t/t1000-branch-create.sh b/t/t1000-branch-create.sh
new file mode 100755
index 0000000..f0c2367
--- /dev/null
+++ b/t/t1000-branch-create.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Branch operations.
+
+Exercises the "stg branch" commands.
+'
+
+. ./test-lib.sh
+
+stg init
+
+test_expect_failure \
+ 'Try to create an stgit branch with a spurious refs/patches/ entry' \
+ 'find .git -name foo | xargs rm -rf &&
+ touch .git/refs/patches/foo &&
+ stg branch -c foo
+'
+
+test_expect_success \
+ 'Check 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_failure \
+ 'Try to create an stgit branch with a spurious patches/ entry' \
+ 'find .git -name foo | xargs rm -rf &&
+ touch .git/patches/foo &&
+ stg branch -c foo
+'
+
+test_expect_success \
+ 'Check no part of the branch was created' \
+ 'test "`find .git -name foo | tee /dev/stderr`" = ".git/patches/foo" &&
+ ( grep foo .git/HEAD; test $? = 1 )
+'
+
+test_expect_failure \
+ 'Try to create an stgit branch with a spurious refs/bases/ entry' \
+ 'find .git -name foo | xargs rm -rf &&
+ touch .git/refs/bases/foo &&
+ stg branch -c foo
+'
+
+test_expect_success \
+ 'Check no part of the branch was created' \
+ 'test "`find .git -name foo | tee /dev/stderr`" = ".git/refs/bases/foo" &&
+ ( grep foo .git/HEAD; test $? = 1 )
+'
+
+
+test_expect_failure \
+ 'Try to create an stgit branch with an existing git branch by that name' \
+ 'find .git -name foo | xargs rm -rf &&
+ cp .git/refs/heads/master .git/refs/heads/foo &&
+ stg branch -c foo
+'
+
+test_expect_success \
+ 'Check 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_expect_failure \
+ 'Try to create an stgit branch with an invalid refs/heads/ entry' \
+ 'find .git -name foo | xargs rm -rf &&
+ touch .git/refs/heads/foo &&
+ stg branch -c foo
+'
+
+test_expect_success \
+ 'Check 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
Yann Dirson <ydirson@altern.org>