[PATCH 0/9] Add a testsuite to stgit (take 3), and more

[PATCH 0/9] Add a testsuite to stgit (take 3), and more

[Yann Dirson]

This new revision of my testsuite patches include:

- cleanup of the repo-initialization stuff in test-lib.sh: now
test_create_repo creates an empty commit to start with, so that HEAD
points to an existing master branch.  This makes test_stg_init
useless, rather let tests can "stg init" directly.

- steal the git t/README as well, and adapt it.  Proposes a numbering
convention for stgit testcases.

- activation and refining of the testcase which I had left commented
out in t1000, and a fix for the bug it shows

- fix for a nasty caching issue in stg clone, and associated test

- 2 testcases showing problems with detection of applied patches in
"push" after a pull when our patches were integrated upstream.  No fix
here, though, so you'll see t1200 and t1201 failing.


And not directly related to the testsuite:
- more bugs listed in TODO
- support for per-user templates for mail and export, in ~/.stgit/templates

-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>

[PATCH 1/9] Add a testsuite framework copied from git-core

[Yann Dirson]

See git's t/README for details on how to use this framework.

There is no integration yet in the toplevel Makefile, I'll let
python masters take care of this.  Use "make -C t" to run the
tests for now.

A patch-naming policy should be defined for stgit, since the
git one does not apply.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 TODO             |    2 -
 t/Makefile       |   25 ++++++
 t/README         |  210 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 t/t0000-dummy.sh |   17 ++++
 t/test-lib.sh    |  203 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 456 insertions(+), 1 deletions(-)

diff --git a/TODO b/TODO
index e5affe0..d97ffd1 100644
--- a/TODO
+++ b/TODO
@@ -6,7 +6,7 @@ The TODO list until 1.0:
 - debian package support
 - man page
 - code execution allowed from templates
-- regression tests
+- more regression tests
 - release 1.0
 
 
diff --git a/t/Makefile b/t/Makefile
new file mode 100644
index 0000000..d5d7b6f
--- /dev/null
+++ b/t/Makefile
@@ -0,0 +1,25 @@
+# Run tests
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+
+#GIT_TEST_OPTS=--verbose --debug
+SHELL_PATH ?= $(SHELL)
+TAR ?= $(TAR)
+
+# Shell quote;
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+
+T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
+
+all: $(T) clean
+
+$(T):
+	@echo "*** $@ ***"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
+
+clean:
+	rm -fr trash
+
+.PHONY: $(T) clean
+.NOPARALLEL:
+
diff --git a/t/README b/t/README
new file mode 100644
index 0000000..d88bad2
--- /dev/null
+++ b/t/README
@@ -0,0 +1,210 @@
+Core GIT Tests
+==============
+
+This directory holds many test scripts for core GIT tools.  The
+first part of this short document describes how to run the tests
+and read their output.
+
+When fixing the tools or adding enhancements, you are strongly
+encouraged to add tests in this directory to cover what you are
+trying to fix or enhance.  The later part of this short document
+describes how your test scripts should be organized.
+
+The mechanism that powers this testsuite is directly imported from the
+Core GIT Tests, in directory t/ of the git repository.  Files are base
+on Core GIT version 1.3.0.rc4.g5069.
+
+
+Running Tests
+-------------
+
+The easiest way to run tests is to say "make -C t".  This runs all
+the tests.
+
+    *** t0000-basic.sh ***
+    *   ok 1: .git/objects should be empty after git-init-db in an empty repo.
+    *   ok 2: .git/objects should have 256 subdirectories.
+    *   ok 3: git-update-index without --add should fail adding.
+    ...
+    *   ok 23: no diff after checkout and git-update-index --refresh.
+    * passed all 23 test(s)
+    *** t0100-environment-names.sh ***
+    *   ok 1: using old names should issue warnings.
+    *   ok 2: using old names but having new names should not issue warnings.
+    ...
+
+Or you can run each test individually from command line, like
+this:
+
+    $ sh ./t3001-ls-files-killed.sh
+    *   ok 1: git-update-index --add to add various paths.
+    *   ok 2: git-ls-files -k to show killed files.
+    *   ok 3: validate git-ls-files -k output.
+    * passed all 3 test(s)
+
+You can pass --verbose (or -v), --debug (or -d), and --immediate
+(or -i) command line argument to the test.
+
+--verbose::
+	This makes the test more verbose.  Specifically, the
+	command being run and their output if any are also
+	output.
+
+--debug::
+	This may help the person who is developing a new test.
+	It causes the command defined with test_debug to run.
+
+--immediate::
+	This causes the test to immediately exit upon the first
+	failed test.
+
+
+Naming Tests
+------------
+
+The test files are named as:
+
+	tNNNN-commandname-details.sh
+
+where N is a decimal digit.
+
+Here is a proposal for numbering, loosely based on the Core GIT
+numbering conventions.
+
+First two digit tells the particular command we are testing:
+
+	00 - stgit itself
+	10 - branch
+	11 - clone
+	12 - push
+
+Third and fourth digit (optionally) tells the particular switch or
+group of switches we are testing.
+
+If you create files under t/ directory (i.e. here) that is not
+the top-level test script, never name the file to match the above
+pattern.  The Makefile here considers all such files as the
+top-level test script and tries to run all of them.  A care is
+especially needed if you are creating a common test library
+file, similar to test-lib.sh, because such a library file may
+not be suitable for standalone execution.
+
+
+Writing Tests
+-------------
+
+The test script is written as a shell script.  It should start
+with the standard "#!/bin/sh" with copyright notices, and an
+assignment to variable 'test_description', like this:
+
+	#!/bin/sh
+	#
+	# Copyright (c) 2005 Junio C Hamano
+	#
+
+	test_description='xxx test (option --frotz)
+
+	This test registers the following structure in the cache
+	and tries to run git-ls-files with option --frotz.'
+
+
+Source 'test-lib.sh'
+--------------------
+
+After assigning test_description, the test script should source
+test-lib.sh like this:
+
+	. ./test-lib.sh
+
+This test harness library does the following things:
+
+ - If the script is invoked with command line argument --help
+   (or -h), it shows the test_description and exits.
+
+ - Creates an empty test directory with an empty .git/objects
+   database and chdir(2) into it.  This directory is 't/trash'
+   if you must know, but I do not think you care.
+
+ - Defines standard test helper functions for your scripts to
+   use.  These functions are designed to make all scripts behave
+   consistently when command line arguments --verbose (or -v),
+   --debug (or -d), and --immediate (or -i) is given.
+
+
+End with test_done
+------------------
+
+Your script will be a sequence of tests, using helper functions
+from the test harness library.  At the end of the script, call
+'test_done'.
+
+
+Test harness library
+--------------------
+
+There are a handful helper functions defined in the test harness
+library for your script to use.
+
+ - test_expect_success <message> <script>
+
+   This takes two strings as parameter, and evaluates the
+   <script>.  If it yields success, test is considered
+   successful.  <message> should state what it is testing.
+
+   Example:
+
+	test_expect_success \
+	    'git-write-tree should be able to write an empty tree.' \
+	    'tree=$(git-write-tree)'
+
+ - test_expect_failure <message> <script>
+
+   This is the opposite of test_expect_success.  If <script>
+   yields success, test is considered a failure.
+
+   Example:
+
+	test_expect_failure \
+	    'git-update-index without --add should fail adding.' \
+	    'git-update-index should-be-empty'
+
+ - test_debug <script>
+
+   This takes a single argument, <script>, and evaluates it only
+   when the test script is started with --debug command line
+   argument.  This is primarily meant for use during the
+   development of a new test script.
+
+ - test_done
+
+   Your test script must have test_done at the end.  Its purpose
+   is to summarize successes and failures in the test script and
+   exit with an appropriate error code.
+
+
+Tips for Writing Tests
+----------------------
+
+As with any programming projects, existing programs are the best
+source of the information.  However, do _not_ emulate
+t0000-basic.sh when writing your tests.  The test is special in
+that it tries to validate the very core of GIT.  For example, it
+knows that there will be 256 subdirectories under .git/objects/,
+and it knows that the object ID of an empty tree is a certain
+40-byte string.  This is deliberately done so in t0000-basic.sh
+because the things the very basic core test tries to achieve is
+to serve as a basis for people who are changing the GIT internal
+drastically.  For these people, after making certain changes,
+not seeing failures from the basic test _is_ a failure.  And
+such drastic changes to the core GIT that even changes these
+otherwise supposedly stable object IDs should be accompanied by
+an update to t0000-basic.sh.
+
+However, other tests that simply rely on basic parts of the core
+GIT working properly should not have that level of intimate
+knowledge of the core GIT internals.  If all the test scripts
+hardcoded the object IDs like t0000-basic.sh does, that defeats
+the purpose of t0000-basic.sh, which is to isolate that level of
+validation in one place.  Your test also ends up needing
+updating when such a change to the internal happens, so do _not_
+do it and leave the low level of validation to t0000-basic.sh.
diff --git a/t/t0000-dummy.sh b/t/t0000-dummy.sh
new file mode 100755
index 0000000..1a9bd87
--- /dev/null
+++ b/t/t0000-dummy.sh
@@ -0,0 +1,17 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Dummy test.
+
+Only to test the testing environment.
+'
+
+. ./test-lib.sh
+
+test_expect_success \
+    'check stgit can be run' \
+    'stg version'
+
+test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
new file mode 100755
index 0000000..6339c54
--- /dev/null
+++ b/t/test-lib.sh
@@ -0,0 +1,203 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+# Copyright (c) 2006 Yann Dirson - tuning for stgit
+#
+
+# For repeatability, reset the environment to known value.
+LANG=C
+LC_ALL=C
+PAGER=cat
+TZ=UTC
+export LANG LC_ALL PAGER TZ
+unset AUTHOR_DATE
+unset AUTHOR_EMAIL
+unset AUTHOR_NAME
+unset COMMIT_AUTHOR_EMAIL
+unset COMMIT_AUTHOR_NAME
+unset GIT_ALTERNATE_OBJECT_DIRECTORIES
+unset GIT_AUTHOR_DATE
+GIT_AUTHOR_EMAIL=author@example.com
+GIT_AUTHOR_NAME='A U Thor'
+unset GIT_COMMITTER_DATE
+GIT_COMMITTER_EMAIL=committer@example.com
+GIT_COMMITTER_NAME='C O Mitter'
+unset GIT_DIFF_OPTS
+unset GIT_DIR
+unset GIT_EXTERNAL_DIFF
+unset GIT_INDEX_FILE
+unset GIT_OBJECT_DIRECTORY
+unset SHA1_FILE_DIRECTORIES
+unset SHA1_FILE_DIRECTORY
+export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
+export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
+
+# Each test should start with something like this, after copyright notices:
+#
+# test_description='Description of this test...
+# This test checks if command xyzzy does the right thing...
+# '
+# . ./test-lib.sh
+
+error () {
+	echo "* error: $*"
+	trap - exit
+	exit 1
+}
+
+say () {
+	echo "* $*"
+}
+
+test "${test_description}" != "" ||
+error "Test script did not set test_description."
+
+while test "$#" -ne 0
+do
+	case "$1" in
+	-d|--d|--de|--deb|--debu|--debug)
+		debug=t; shift ;;
+	-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
+		immediate=t; shift ;;
+	-h|--h|--he|--hel|--help)
+		echo "$test_description"
+		exit 0 ;;
+	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
+		verbose=t; shift ;;
+	*)
+		break ;;
+	esac
+done
+
+exec 5>&1
+if test "$verbose" = "t"
+then
+	exec 4>&2 3>&1
+else
+	exec 4>/dev/null 3>/dev/null
+fi
+
+test_failure=0
+test_count=0
+
+trap 'echo >&5 "FATAL: Unexpected exit with code $?"; exit 1' exit
+
+
+# You are not expected to call test_ok_ and test_failure_ directly, use
+# the text_expect_* functions instead.
+
+test_ok_ () {
+	test_count=$(expr "$test_count" + 1)
+	say "  ok $test_count: $@"
+}
+
+test_failure_ () {
+	test_count=$(expr "$test_count" + 1)
+	test_failure=$(expr "$test_failure" + 1);
+	say "FAIL $test_count: $1"
+	shift
+	echo "$@" | sed -e 's/^/	/'
+	test "$immediate" = "" || { trap - exit; exit 1; }
+}
+
+
+test_debug () {
+	test "$debug" = "" || eval "$1"
+}
+
+test_run_ () {
+	eval >&3 2>&4 "$1"
+	eval_ret="$?"
+	return 0
+}
+
+test_expect_failure () {
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 parameters to test-expect-failure"
+	say >&3 "expecting failure: $2"
+	test_run_ "$2"
+	if [ "$?" = 0 -a "$eval_ret" != 0 ]
+	then
+		test_ok_ "$1"
+	else
+		test_failure_ "$@"
+	fi
+}
+
+test_expect_success () {
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 parameters to test-expect-success"
+	say >&3 "expecting success: $2"
+	test_run_ "$2"
+	if [ "$?" = 0 -a "$eval_ret" = 0 ]
+	then
+		test_ok_ "$1"
+	else
+		test_failure_ "$@"
+	fi
+}
+
+test_expect_code () {
+	test "$#" = 3 ||
+	error "bug in the test script: not 3 parameters to test-expect-code"
+	say >&3 "expecting exit code $1: $3"
+	test_run_ "$3"
+	if [ "$?" = 0 -a "$eval_ret" = "$1" ]
+	then
+		test_ok_ "$2"
+	else
+		test_failure_ "$@"
+	fi
+}
+
+# Most tests can use the created repository, but some amy need to create more.
+# Usage: test_create_repo <directory>
+test_create_repo () {
+	test "$#" = 1 ||
+	error "bug in the test script: not 1 parameter to test-create-repo"
+	owd=`pwd`
+	repo="$1"
+	mkdir "$repo"
+	cd "$repo" || error "Cannot setup test environment"
+	git-init-db 2>/dev/null ||
+	error "cannot run git-init-db -- have you installed git-core?"
+	mv .git/hooks .git/hooks-disabled
+	echo "empty start" |
+	git-commit-tree `git-write-tree` >.git/refs/heads/master 2>/dev/null ||
+	error "cannot run git-commit -- is your git-core funtionning?"
+	cd "$owd"
+}
+
+test_done () {
+	trap - exit
+	case "$test_failure" in
+	0)
+		# We could:
+		# cd .. && rm -fr trash
+		# but that means we forbid any tests that use their own
+		# subdirectory from calling test_done without coming back
+		# to where they started from.
+		# The Makefile provided will clean this test area so
+		# we will leave things as they are.
+
+		say "passed all $test_count test(s)"
+		exit 0 ;;
+
+	*)
+		say "failed $test_failure among $test_count test(s)"
+		exit 1 ;;
+
+	esac
+}
+
+# Test the binaries we have just built.  The tests are kept in
+# t/ subdirectory and are run in trash subdirectory.
+PATH=$(pwd)/..:$PATH
+export PATH
+
+
+# Test repository
+test=trash
+rm -fr "$test"
+test_create_repo $test
+cd "$test"

[PATCH 2/9] Add list of bugs to TODO

[Yann Dirson]

Since there is no formal place to register bugs, other than the git
ml, I have added a couple of them, some of them already mentionned on
the ml but still around.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 TODO |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/TODO b/TODO
index d97ffd1..a13e511 100644
--- a/TODO
+++ b/TODO
@@ -7,6 +7,7 @@ The TODO list until 1.0:
 - man page
 - code execution allowed from templates
 - more regression tests
+- stg help should probably pipe through the $PAGER
 - release 1.0
 
 
@@ -17,3 +18,17 @@ The future, when time allows or if someo
   synchronising with other patches (diff format or in other
   repositories)
 - write bash-completion script for the StGIT commands
+- support for branches with / in names
+  (ml: "Handle branch names with slashes")
+- "pull" argument should default to a sane value, "origin" is wrong in
+  many cases
+
+Bugs:
+
+- the following commands break in subdirs:
+  - refresh (ml: "Running StGIT in subdirectories")
+- "stg show" on empty patch shows previous patch
+- "stg add" is accepted when no patch is applied, then any push says
+  one must refresh first, which is obviously wrong
+- "stg add" on files already added should print a notice, so that the
+  user can catch a possible typo

[PATCH 3/9] Add a couple of safety checks to series creation

[Yann Dirson]

- 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

[PATCH 4/9] Make branch creation atomic

[Yann Dirson]

This patch adds an optional create_at parameter to Series.init(), to
pass a git.branch_create() parameter if we want it to be called.  This
parameter can be None, so the test for False has to be explicit.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/commands/branch.py |    6 +-----
 stgit/stack.py           |    8 +++++---
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index be501a8..c7561a8 100644
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -125,12 +125,8 @@ def func(parser, options, args):
         tree_id = None
         if len(args) == 2:
             tree_id = git_id(args[1])
-
-        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)
+        stack.Series(args[0]).init(create_at = tree_id)
 
         print 'Branch "%s" created.' % args[0]
         return
diff --git a/stgit/stack.py b/stgit/stack.py
index c14e029..19bb62a 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -426,7 +426,7 @@ class Series:
         """
         return os.path.isdir(self.__patch_dir)
 
-    def init(self):
+    def init(self, create_at=False):
         """Initialises the stgit series
         """
         bases_dir = os.path.join(self.__base_dir, 'refs', 'bases')
@@ -438,6 +438,9 @@ class Series:
         if os.path.exists(self.__base_file):
             raise StackException, self.__base_file + ' already exists'
 
+        if (create_at!=False):
+            git.create_branch(self.__name, create_at)
+
         os.makedirs(self.__patch_dir)
 
         if not os.path.isdir(bases_dir):
@@ -509,8 +512,7 @@ class Series:
         """Clones a series
         """
         base = read_string(self.get_base_file())
-        git.create_branch(target_series, tree_id = base)
-        Series(target_series).init()
+        Series(target_series).init(create_at = base)
         new_series = Series(target_series)
 
         # generate an artificial description file

[PATCH 5/9] Correctly handle refs/patches on series rename

[Yann Dirson]

When renaming a series, the refs/patches dir was not moved, and by
chance a new one was created by the repository-upgrade code, but that
left the old one behind as cruft (which the safety checks added in a
former patch now detects).

Also adds a regression test to assert that nothing by the old name
is left behind.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/stack.py           |    2 ++
 t/t1001-branch-rename.sh |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/stgit/stack.py b/stgit/stack.py
index 19bb62a..975ac21 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -505,6 +505,8 @@ class Series:
             os.rename(self.__series_dir, to_stack.__series_dir)
         if os.path.exists(self.__base_file):
             os.rename(self.__base_file, to_stack.__base_file)
+        if os.path.exists(self.__refs_dir):
+            os.rename(self.__refs_dir, to_stack.__refs_dir)
 
         self.__init__(to_name)
 
diff --git a/t/t1001-branch-rename.sh b/t/t1001-branch-rename.sh
new file mode 100755
index 0000000..28da15c
--- /dev/null
+++ b/t/t1001-branch-rename.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Branch renames.
+
+Exercises branch renaming commands.
+'
+
+. ./test-lib.sh
+
+test_expect_success \
+    'Create an stgit branch from scratch' \
+    'stg init &&
+     stg branch -c foo &&
+     stg new p1 -m "p1"
+'
+
+test_expect_failure \
+    'Rename the current stgit branch' \
+    'stg branch -r foo bar
+'
+
+test_expect_success \
+    'Rename an stgit branch' \
+    'stg branch -c buz &&
+     stg branch -r foo bar &&
+     test -z `find .git -name foo | tee /dev/stderr`
+'
+
+test_done

[PATCH 6/9] Fix a seriously bad interaction between .git caching and repo cloning

[Yann Dirson]

Testcase 2 exhibits a problem with caching the location of .git while
cloning a repository.  Since basedir.get() is called before the clone is
built, a value may get stored in the cache if we are within a
git-controlled tree already.  Then when constructing the object for the
clone, a bogus .git is used, which can lead, when running tests in t/trash,
to corruption of the stgit .git repository.

Testcase 1 does not show any problem by chance, because since we have a
./.git prepared for use by the testsuite, value ".git" get cached, and it
happens that this value will be still valid after chdir'ing into the
newborn clone.

Clearing the cache at the appropriate place fixes the problem.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/basedir.py        |    6 ++++++
 stgit/commands/clone.py |    3 +++
 t/t1100-clone-under.sh  |   36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/stgit/basedir.py b/stgit/basedir.py
index c394572..81f2b40 100644
--- a/stgit/basedir.py
+++ b/stgit/basedir.py
@@ -42,3 +42,9 @@ def get():
             __base_dir = __output('git-rev-parse --git-dir 2> /dev/null')
 
     return __base_dir
+
+def clear_cache():
+    """Clear the cached location of .git
+    """
+    global __base_dir
+    __base_dir = None
diff --git a/stgit/commands/clone.py b/stgit/commands/clone.py
index 9ad76a6..455dd6e 100644
--- a/stgit/commands/clone.py
+++ b/stgit/commands/clone.py
@@ -51,6 +51,9 @@ def func(parser, options, args):
     os.chdir(local_dir)
     git.checkout(tree_id = 'HEAD')
 
+    # be sure to forget any cached value for .git, since we're going
+    # to work on a brand new repository
+    basedir.clear_cache()
     stack.Series().init()
 
     print 'done'
diff --git a/t/t1100-clone-under.sh b/t/t1100-clone-under.sh
new file mode 100755
index 0000000..c86ef61
--- /dev/null
+++ b/t/t1100-clone-under.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Check cloning in a repo subdir
+
+Check that "stg clone" works in a subdir of a git tree.
+This ensures (to some point) that a clone within a tree does
+not corrupt the enclosing repo.
+
+This test must be run before any tests making use of clone.
+'
+
+. ./test-lib.sh
+
+# Here we are in a repo, we have a ./.git
+# Do not get rid of it, or a bug may bite out stgit repo hard
+
+# Need a repo to clone
+test_create_repo foo
+
+test_expect_success \
+    'stg clone right inside a git tree' \
+    "stg clone foo bar"
+
+# now work in a subdir
+mkdir sub
+mv foo sub
+cd sub
+
+test_expect_success \
+    'stg clone deeper under a git tree' \
+    "stg clone foo bar"
+
+test_done

[PATCH 7/9] Test that pulls a patch creating a file that got modified afterwards

[Yann Dirson]

This demonstrates an issue wite has bitten me more than once: the stg
branch adds a file in one patch, and modifies it in a later patch; then all
patches get integrated in upstream tree, and at "stg pull" time, stgit
believes there is a conflict, even one the patches are exactly the same.

"stg push" apparently does not consider patches one by one, or it
would have noticed that the patches were "already applied".  It reports:

 Pushing patch "p1"...Error: File "file2" added in branches but different
 The merge failed during "push". Use "refresh" after fixing the conflicts
 stg push: GIT index merging failed (possible conflicts)

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 t/t1200-push-modified.sh |   51 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/t/t1200-push-modified.sh b/t/t1200-push-modified.sh
new file mode 100755
index 0000000..18a4df2
--- /dev/null
+++ b/t/t1200-push-modified.sh
@@ -0,0 +1,51 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='Exercise pushing patches applied upstream.
+
+Especially, consider the case of a patch that adds a file,
+while a subsequent one modifies it.
+'
+
+. ./test-lib.sh
+
+# don't need this repo, but better not drop it, see t1100
+#rm -rf .git
+
+# Need a repo to clone
+test_create_repo foo
+
+test_expect_success \
+    'Clone tree and setup changes' \
+    "stg clone foo bar &&
+     (cd bar && stg new p1 -m p1
+      printf 'a\nc\n' > file && stg add file && stg refresh &&
+      stg new p2 -m p2
+      printf 'a\nb\nc\n' > file && stg refresh
+     )
+"
+
+test_expect_success \
+    'Port those patches to orig tree' \
+    "(cd foo &&
+      GIT_DIR=../bar/.git git-format-patch --stdout bases/master..HEAD |
+      git-am -3 -k
+     )
+"
+
+test_expect_success \
+    'Pull to sync with parent, preparing for the problem' \
+    "(cd bar && stg pop --all &&
+      stg pull
+     )
+"
+
+test_expect_success \
+    'Now attempt to push those patches applied upstream' \
+    "(cd bar && stg push --all
+     )
+"
+
+test_done

[PATCH 8/9] Exercise "stg pull" on patches just appending lines.

[Yann Dirson]

It indeed reveals a problem in "push": appended lines are appended
again, as the already-applied patch is not detected.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 t/t1201-pull-trailing.sh |   48 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/t/t1201-pull-trailing.sh b/t/t1201-pull-trailing.sh
new file mode 100755
index 0000000..142f894
--- /dev/null
+++ b/t/t1201-pull-trailing.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Yann Dirson
+#
+
+test_description='test
+
+'
+
+. ./test-lib.sh
+
+# don't need this repo, but better not drop it, see t1100
+#rm -rf .git
+
+# Need a repo to clone
+test_create_repo foo
+
+test_expect_success \
+    'Setup and clone tree, and setup changes' \
+    "(cd foo &&
+      printf 'a\nb\n' > file && git add file && git commit -m .
+     ) &&
+     stg clone foo bar &&
+     (cd bar && stg new p1 -m p1
+      printf 'c\n' >> file && stg refresh
+     )
+"
+
+test_expect_success \
+    'Port those patches to orig tree' \
+    "(cd foo &&
+      GIT_DIR=../bar/.git git-format-patch --stdout bases/master..HEAD |
+      git-am -3 -k
+     )
+"
+
+test_expect_success \
+    'Pull those patches applied upstream' \
+    "(cd bar && stg pull
+     )
+"
+
+test_expect_success \
+    'Check that all went well' \
+    "diff -u foo/file bar/file
+"
+
+test_done

[PATCH 9/9] Look for templates in ~/.stgit/templates as well

[Yann Dirson]

This can be quite useful to avoid adding one's sig again and again to
ever covermail, and to use a patchmail template that insert
Signed-off-by lines, for those of us who prefer this to adding them to
commits.

Also make sure to use os.path.join() instead of hardcoded slashes.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

 stgit/commands/export.py |    9 ++++++---
 stgit/commands/mail.py   |   18 ++++++++++++------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/stgit/commands/export.py b/stgit/commands/export.py
index e7de902..fbf5690 100644
--- a/stgit/commands/export.py
+++ b/stgit/commands/export.py
@@ -31,8 +31,9 @@ usage = """%prog [options] [<dir>]
 
 Export the applied patches into a given directory (defaults to
 'patches') in a standard unified GNU diff format. A template file
-(defaulting to '.git/patchexport.tmpl or
-/usr/share/stgit/templates/patchexport.tmpl') can be used for the
+(defaulting to '.git/patchexport.tmpl' or
+'~/.stgit/templates/patchexport.tmpl' or
+'/usr/share/stgit/templates/patchexport.tmpl') can be used for the
 patch format. The following variables are supported in the template
 file:
 
@@ -145,8 +146,10 @@ def func(parser, options, args):
         patch_tmpl_list = []
 
     patch_tmpl_list += [os.path.join(basedir.get(), 'patchexport.tmpl'),
+                        os.path.join(os.path.expanduser('~'), '.stgit', 'templates',
+                                     'patchexport.tmpl'),
                         os.path.join(sys.prefix,
-                                     'share/stgit/templates/patchexport.tmpl')]
+                                     'share', 'stgit', 'templates', 'patchexport.tmpl')]
     tmpl = ''
     for patch_tmpl in patch_tmpl_list:
         if os.path.isfile(patch_tmpl):
diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index 470cf65..5e01ea1 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -30,16 +30,18 @@ usage = """%prog [options] [<patch> [<pa
 Send a patch or a range of patches (defaulting to the applied patches)
 by e-mail using the 'smtpserver' configuration option. The From
 address and the e-mail format are generated from the template file
-passed as argument to '--template' (defaulting to .git/patchmail.tmpl
-or /usr/share/stgit/templates/patchmail.tmpl). The To/Cc/Bcc addresses
+passed as argument to '--template' (defaulting to
+'.git/patchmail.tmpl' or '~/.stgit/templates/patchmail.tmpl' or or
+'/usr/share/stgit/templates/patchmail.tmpl'). The To/Cc/Bcc addresses
 can either be added to the template file or passed via the
 corresponding command line options.
 
 A preamble e-mail can be sent using the '--cover' and/or '--edit'
 options. The first allows the user to specify a file to be used as a
 template. The latter option will invoke the editor on the specified
-file (defaulting to .git/covermail.tmpl or
-/usr/share/stgit/templates/covermail.tmpl).
+file (defaulting to '.git/covermail.tmpl' or
+'~/.stgit/templates/covermail.tmpl' or
+'/usr/share/stgit/templates/covermail.tmpl').
 
 All the subsequent e-mails appear as replies to the first e-mail sent
 (either the preamble or the first patch). E-mails can be seen as
@@ -444,8 +446,10 @@ def func(parser, options, args):
             tfile_list = [options.cover]
         else:
             tfile_list = [os.path.join(basedir.get(), 'covermail.tmpl'),
+                          os.path.join(os.path.expanduser('~'), '.stgit', 'templates',
+                                       'covermail.tmpl'),
                           os.path.join(sys.prefix,
-                                       'share/stgit/templates/covermail.tmpl')]
+                                       'share', 'stgit', 'templates', 'covermail.tmpl')]
 
         tmpl = None
         for tfile in tfile_list:
@@ -476,8 +480,10 @@ def func(parser, options, args):
         tfile_list = [options.template]
     else:
         tfile_list = [os.path.join(basedir.get(), 'patchmail.tmpl'),
+                      os.path.join(os.path.expanduser('~'), '.stgit', 'templates',
+                                   'patchmail.tmpl'),
                       os.path.join(sys.prefix,
-                                   'share/stgit/templates/patchmail.tmpl')]
+                                   'share', 'stgit', 'templates', 'patchmail.tmpl')]
     tmpl = None
     for tfile in tfile_list:
         if os.path.isfile(tfile):