[StGIT PATCH 0/5] Proposed patches

[StGIT PATCH 0/5] Proposed patches

[Catalin Marinas]

This series contains the patches pending in my "proposed" branch.
They've been updated following comments from Karl.


Catalin Marinas (5):
      Add test_patches target to Makefile
      Convert "init" to the new StGIT infrastructure
      Add stack creation and initialisation support to lib.Stack
      Create a git.Branch class as ancestor of stack.Stack
      Rename Repository.head to Repository.head_ref


 Makefile               |    9 ++++-
 stgit/commands/init.py |   14 ++-----
 stgit/commands/new.py  |    3 +-
 stgit/lib/git.py       |   58 ++++++++++++++++++++++++++++++-
 stgit/lib/stack.py     |   90 ++++++++++++++++++++++++++++++++++++------------
 5 files changed, 137 insertions(+), 37 deletions(-)

-- 
Catalin

[StGIT PATCH 1/5] Rename Repository.head to Repository.head_ref

[Catalin Marinas]

This is to avoid confusion with the Stack.head function which returns
a commit object rather than a file name. The patch also changes the
"new" function to use stack.head directly rather than via the
Repository.refs... object.

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---

 stgit/commands/new.py |    3 +--
 stgit/lib/git.py      |    4 ++--
 stgit/lib/stack.py    |    2 +-
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/stgit/commands/new.py b/stgit/commands/new.py
index 15bb2e0..c4ee4e1 100644
--- a/stgit/commands/new.py
+++ b/stgit/commands/new.py
@@ -58,9 +58,8 @@ def func(parser, options, args):
     else:
         parser.error('incorrect number of arguments')
 
-    head = directory.repository.refs.get(directory.repository.head)
     cd = gitlib.CommitData(
-        tree = head.data.tree, parents = [head], message = '',
+        tree = stack.head.data.tree, parents = [stack.head], message = '',
         author = gitlib.Person.author(), committer = gitlib.Person.committer())
 
     # Set patch commit message from commandline.
diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index c9f01e3..fd66f6d 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -443,13 +443,13 @@ class Repository(RunWithEnv):
                                                 ).output_one_line()
         return self.get_commit(sha1)
     @property
-    def head(self):
+    def head_ref(self):
         try:
             return self.run(['git', 'symbolic-ref', '-q', 'HEAD']
                             ).output_one_line()
         except run.RunException:
             raise DetachedHeadException()
-    def set_head(self, ref, msg):
+    def set_head_ref(self, ref, msg):
         self.run(['git', 'symbolic-ref', '-m', msg, 'HEAD', ref]).no_output()
     def simple_merge(self, base, ours, theirs):
         """Given three L{Tree}s, tries to do an in-index merge with a
diff --git a/stgit/lib/stack.py b/stgit/lib/stack.py
index bdd21b1..b1544a5 100644
--- a/stgit/lib/stack.py
+++ b/stgit/lib/stack.py
@@ -178,7 +178,7 @@ class Repository(git.Repository):
         self.__stacks = {} # name -> Stack
     @property
     def current_branch(self):
-        return utils.strip_leading('refs/heads/', self.head)
+        return utils.strip_leading('refs/heads/', self.head_ref)
     @property
     def current_stack(self):
         return self.get_stack()

[StGIT PATCH 2/5] Create a git.Branch class as ancestor of stack.Stack

[Catalin Marinas]

This class deals with Git-specific branch commands. The Stack class is a
direct child of Branch and some of its functionality was moved to the new
class.

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---

 stgit/lib/git.py   |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 stgit/lib/stack.py |   27 ++++++--------------------
 2 files changed, 60 insertions(+), 21 deletions(-)

diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index fd66f6d..0e0cccb 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -28,6 +28,9 @@ class RepositoryException(exception.StgException):
     """Base class for all exceptions due to failed L{Repository}
     operations."""
 
+class BranchException(exception.StgException):
+    """Exception raised by failed L{Branch} operations."""
+
 class DateException(exception.StgException):
     """Exception raised when a date+time string could not be parsed."""
     def __init__(self, string, type):
@@ -379,6 +382,10 @@ class Repository(RunWithEnv):
         except run.RunException:
             raise RepositoryException('Cannot find git repository')
     @property
+    def current_branch_name(self):
+        """Return the name of the current branch."""
+        return utils.strip_leading('refs/heads/', self.head_ref)
+    @property
     def default_index(self):
         """An L{Index} object representing the default index file for the
         repository."""
@@ -619,3 +626,50 @@ class IndexAndWorktree(RunWithEnvCwd):
     def update_index(self, files):
         self.run(['git', 'update-index', '--remove', '-z', '--stdin']
                  ).input_nulterm(files).discard_output()
+
+class Branch(object):
+    """Represents a Git branch."""
+    def __init__(self, repository, name):
+        self.__repository = repository
+        self.__name = name
+        try:
+            self.head
+        except KeyError:
+            raise BranchException('%s: no such branch' % name)
+
+    name = property(lambda self: self.__name)
+    repository = property(lambda self: self.__repository)
+
+    def __ref(self):
+        return 'refs/heads/%s' % self.__name
+    @property
+    def head(self):
+        return self.__repository.refs.get(self.__ref())
+    def set_head(self, commit, msg):
+        self.__repository.refs.set(self.__ref(), commit, msg)
+
+    def set_parent_remote(self, name):
+        value = config.set('branch.%s.remote' % self.__name, name)
+    def set_parent_branch(self, name):
+        if config.get('branch.%s.remote' % self.__name):
+            # Never set merge if remote is not set to avoid
+            # possibly-erroneous lookups into 'origin'
+            config.set('branch.%s.merge' % self.__name, name)
+
+    @classmethod
+    def create(cls, repository, name, create_at = None):
+        """Create a new Git branch and return the corresponding
+        L{Branch} object."""
+        try:
+            branch = cls(repository, name)
+        except BranchException:
+            branch = None
+        if branch:
+            raise BranchException('%s: branch already exists' % name)
+
+        cmd = ['git', 'branch']
+        if create_at:
+            cmd.append(create_at.sha1)
+        repository.run(['git', 'branch', create_at.sha1]).discard_output()
+
+        return cls(repository, name)
diff --git a/stgit/lib/stack.py b/stgit/lib/stack.py
index b1544a5..030c407 100644
--- a/stgit/lib/stack.py
+++ b/stgit/lib/stack.py
@@ -130,34 +130,22 @@ class Patches(object):
         self.__patches[name] = p
         return p
 
-class Stack(object):
+class Stack(git.Branch):
     """Represents an StGit stack (that is, a git branch with some extra
     metadata)."""
+    __repo_subdir = 'patches'
+
     def __init__(self, repository, name):
-        self.__repository = repository
-        self.__name = name
-        try:
-            self.head
-        except KeyError:
-            raise exception.StgException('%s: no such branch' % name)
+        git.Branch.__init__(self, repository, name)
         self.__patchorder = PatchOrder(self)
         self.__patches = Patches(self)
         if not stackupgrade.update_to_current_format_version(repository, name):
             raise exception.StgException('%s: branch not initialized' % name)
-    name = property(lambda self: self.__name)
-    repository = property(lambda self: self.__repository)
     patchorder = property(lambda self: self.__patchorder)
     patches = property(lambda self: self.__patches)
     @property
     def directory(self):
-        return os.path.join(self.__repository.directory, 'patches', self.__name)
-    def __ref(self):
-        return 'refs/heads/%s' % self.__name
-    @property
-    def head(self):
-        return self.__repository.refs.get(self.__ref())
-    def set_head(self, commit, msg):
-        self.__repository.refs.set(self.__ref(), commit, msg)
+        return os.path.join(self.repository.directory, self.__repo_subdir, self.name)
     @property
     def base(self):
         if self.patchorder.applied:
@@ -177,14 +165,11 @@ class Repository(git.Repository):
         git.Repository.__init__(self, *args, **kwargs)
         self.__stacks = {} # name -> Stack
     @property
-    def current_branch(self):
-        return utils.strip_leading('refs/heads/', self.head_ref)
-    @property
     def current_stack(self):
         return self.get_stack()
     def get_stack(self, name = None):
         if not name:
-            name = self.current_branch
+            name = self.current_branch_name
         if not name in self.__stacks:
             self.__stacks[name] = Stack(self, name)
         return self.__stacks[name]

[StGIT PATCH 3/5] Add stack creation and initialisation support to lib.Stack

[Catalin Marinas]

This patch adds the create and initialise Stack classmethods to handle
the initialisation of StGIT patch series on a Git branch.

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---

 stgit/lib/stack.py |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 62 insertions(+), 1 deletions(-)

diff --git a/stgit/lib/stack.py b/stgit/lib/stack.py
index 030c407..9cb3967 100644
--- a/stgit/lib/stack.py
+++ b/stgit/lib/stack.py
@@ -3,6 +3,10 @@
 import os.path
 from stgit import exception, utils
 from stgit.lib import git, stackupgrade
+from stgit.config import config
+
+class StackException(exception.StgException):
+    """Exception raised by L{stack} objects."""
 
 class Patch(object):
     """Represents an StGit patch. This class is mainly concerned with
@@ -105,6 +109,14 @@ class PatchOrder(object):
     all = property(lambda self: self.applied + self.unapplied + self.hidden)
     all_visible = property(lambda self: self.applied + self.unapplied)
 
+    @staticmethod
+    def create(stackdir):
+        """Create the PatchOrder specific files
+        """
+        utils.create_empty_file(os.path.join(stackdir, 'applied'))
+        utils.create_empty_file(os.path.join(stackdir, 'unapplied'))
+        utils.create_empty_file(os.path.join(stackdir, 'hidden'))
+
 class Patches(object):
     """Creates L{Patch} objects. Makes sure there is only one such object
     per patch."""
@@ -140,7 +152,7 @@ class Stack(git.Branch):
         self.__patchorder = PatchOrder(self)
         self.__patches = Patches(self)
         if not stackupgrade.update_to_current_format_version(repository, name):
-            raise exception.StgException('%s: branch not initialized' % name)
+            raise StackException('%s: branch not initialized' % name)
     patchorder = property(lambda self: self.__patchorder)
     patches = property(lambda self: self.__patches)
     @property
@@ -158,6 +170,55 @@ class Stack(git.Branch):
             return True
         return self.head == self.patches.get(self.patchorder.applied[-1]).commit
 
+    def set_parents(self, remote, branch):
+        if remote:
+            self.set_parent_remote(remote)
+        if branch:
+            self.set_parent_branch(branch)
+
+    @classmethod
+    def initialise(cls, repository, name = None):
+        """Initialise a Git branch to handle patch series.
+
+        @param repository: The L{Repository} where the L{Stack} will be created
+        @param name: The name of the L{Stack}
+        """
+        if not name:
+            name = repository.current_branch_name
+        # make sure that the corresponding Git branch exists
+        git.Branch(repository, name)
+
+        dir = os.path.join(repository.directory, cls.__repo_subdir, name)
+        compat_dir = os.path.join(dir, 'patches')
+        if os.path.exists(dir):
+            raise StackException('%s: branch already initialized' % name)
+
+        # create the stack directory and files
+        utils.create_dirs(dir)
+        utils.create_dirs(compat_dir)
+        PatchOrder.create(dir)
+        config.set(stackupgrade.format_version_key(name),
+                   str(stackupgrade.FORMAT_VERSION))
+
+        return repository.get_stack(name)
+
+    @classmethod
+    def create(cls, repository, name,
+               create_at = None, parent_remote = None, parent_branch = None):
+        """Create and initialise a Git branch returning the L{Stack} object.
+
+        @param repository: The L{Repository} where the L{Stack} will be created
+        @param name: The name of the L{Stack}
+        @param create_at: The Git id used as the base for the newly created
+            Git branch
+        @param parent_remote: The name of the remote Git branch
+        @param parent_branch: The name of the parent Git branch
+        """
+        git.Branch.create(repository, name, create_at = create_at)
+        stack = cls.initialise(repository, name)
+        stack.set_parents(parent_remote, parent_branch)
+        return stack
+
 class Repository(git.Repository):
     """A git L{Repository<git.Repository>} with some added StGit-specific
     operations."""

[StGIT PATCH 4/5] Convert "init" to the new StGIT infrastructure

[Catalin Marinas]

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---

 stgit/commands/init.py |   14 ++++----------
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/stgit/commands/init.py b/stgit/commands/init.py
index 475a4ce..b68acd7 100644
--- a/stgit/commands/init.py
+++ b/stgit/commands/init.py
@@ -16,13 +16,8 @@ along with this program; if not, write to the Free Software
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 """
 
-import sys, os
-from optparse import OptionParser, make_option
-
-from stgit.commands.common import *
-from stgit.utils import *
-from stgit import stack, git
-
+from stgit.commands import common
+from stgit.lib import stack
 
 help = 'initialise the current branch for use with StGIT'
 usage = """%prog [options]
@@ -31,14 +26,13 @@ Initialise the current GIT branch to be used as an StGIT stack. Note
 that you must already be in a GIT repository and .git/HEAD must point
 to a valid file in refs/heads/."""
 
-directory = DirectoryHasRepository()
+directory = common.DirectoryHasRepositoryLib()
 options = []
 
-
 def func(parser, options, args):
     """Performs the repository initialisation
     """
     if len(args) != 0:
         parser.error('incorrect number of arguments')
 
-    crt_series.init()
+    stack.Stack.initialise(directory.repository)

[StGIT PATCH 5/5] Add test_patches target to Makefile

[Catalin Marinas]

This target tests individual patches in the series.

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---

 Makefile |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index c890b8e..e5c002a 100644
--- a/Makefile
+++ b/Makefile
@@ -2,6 +2,8 @@ PREFIX	?= $(HOME)
 DESTDIR	?= /
 PYTHON	?= python
 
+TEST_PATCHES ?= ..
+
 all:
 	$(PYTHON) setup.py build
 
@@ -14,6 +16,11 @@ doc:
 test:
 	cd t && $(MAKE) all
 
+test_patches:
+	for patch in $$(stg series --noprefix $(TEST_PATCHES)); do \
+		stg goto $$patch && $(MAKE) test || break; \
+	done
+
 clean:
 	for dir in Documentation t; do \
 		(cd $$dir && $(MAKE) clean); \
@@ -26,4 +33,4 @@ clean:
 tags:
 	ctags -e -R stgit/*
 
-.PHONY: all install doc test clean
+.PHONY: all install doc test test_patches clean

Re: [StGIT PATCH 1/5] Rename Repository.head to Repository.head_ref

[Karl Hasselström]

On 2008-06-06 21:45:37 +0100, Catalin Marinas wrote:

> This is to avoid confusion with the Stack.head function which
> returns a commit object rather than a file name. The patch also
> changes the "new" function to use stack.head directly rather than
> via the Repository.refs... object.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>

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

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [StGIT PATCH 2/5] Create a git.Branch class as ancestor of stack.Stack

[Karl Hasselström]

On 2008-06-06 21:45:45 +0100, Catalin Marinas wrote:

> This class deals with Git-specific branch commands. The Stack class
> is a direct child of Branch and some of its functionality was moved
> to the new class.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>

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

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [StGIT PATCH 3/5] Add stack creation and initialisation support to lib.Stack

[Karl Hasselström]

On 2008-06-06 21:45:54 +0100, Catalin Marinas wrote:

> This patch adds the create and initialise Stack classmethods to handle
> the initialisation of StGIT patch series on a Git branch.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>

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

I still have some comments, bu the patch looks good as-is.

> +    @staticmethod
> +    def create(stackdir):
> +        """Create the PatchOrder specific files
> +        """
> +        utils.create_empty_file(os.path.join(stackdir, 'applied'))
> +        utils.create_empty_file(os.path.join(stackdir, 'unapplied'))
> +        utils.create_empty_file(os.path.join(stackdir, 'hidden'))

Maybe s/create/create_files/ or soething to that effect here, as I
suggested in another mail a few minutes ago.

> +    def set_parents(self, remote, branch):
> +        if remote:
> +            self.set_parent_remote(remote)
> +        if branch:
> +            self.set_parent_branch(branch)

Much clearer now. Interesting that the only reason we need this
function at all is to make sure that set_parent_remote and
set_parent_branch are called in the correct order.

Maybe set_parent_branch should throw an exception instead of silently
doing nothing when there is no parent yet? It would force the callers
to be more aware of the limitation -- though we might not actually
want that. Your call.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [StGIT PATCH 4/5] Convert "init" to the new StGIT infrastructure

[Karl Hasselström]

On 2008-06-06 21:46:02 +0100, Catalin Marinas wrote:

> Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>

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

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [StGIT PATCH 5/5] Add test_patches target to Makefile

[Karl Hasselström]

On 2008-06-06 21:46:11 +0100, Catalin Marinas wrote:

> This target tests individual patches in the series.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>

Looks good. It doesn't run the tests with all patches popped, which my
invocation did, but I gues we needn't all be paranoid. :-) (And it
would be somewhat tricky to do now that you support testing an
arbitrary subset of the patches.)

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [StGIT PATCH 3/5] Add stack creation and initialisation support to lib.Stack

[Catalin Marinas]

On 07/06/2008, Karl Hasselström <kha@treskal.com> wrote:
> On 2008-06-06 21:45:54 +0100, Catalin Marinas wrote:
>  > +    @staticmethod
>  > +    def create(stackdir):
>  > +        """Create the PatchOrder specific files
>  > +        """
>  > +        utils.create_empty_file(os.path.join(stackdir, 'applied'))
>  > +        utils.create_empty_file(os.path.join(stackdir, 'unapplied'))
>  > +        utils.create_empty_file(os.path.join(stackdir, 'hidden'))
>
>
> Maybe s/create/create_files/ or soething to that effect here, as I
>  suggested in another mail a few minutes ago.

Done, no problem with that.

>  > +    def set_parents(self, remote, branch):
>  > +        if remote:
>  > +            self.set_parent_remote(remote)
>  > +        if branch:
>  > +            self.set_parent_branch(branch)
>
>
> Much clearer now. Interesting that the only reason we need this
>  function at all is to make sure that set_parent_remote and
>  set_parent_branch are called in the correct order.
>
>  Maybe set_parent_branch should throw an exception instead of silently
>  doing nothing when there is no parent yet? It would force the callers
>  to be more aware of the limitation -- though we might not actually
>  want that. Your call.

I can't argue much. I propose to commit the patch as is and modify it
afterwards. I haven't touched this code much, it was Yann's
implementation and I forgot all the decisions at that time.

-- 
Catalin