[StGIT PATCH 0/5] Various updates to the new infrastructure

[StGIT PATCH 0/5] Various updates to the new infrastructure

[Catalin Marinas]

For a wider review and to avoid merging conflicts with other developer's 
tree (especially Karl's), I created a "proposed" branch in my StGIT tree
(http://homepage.ntlworld.com/cmarinas/stgit.git) containing patches that 
I intend to merge on "master". I'll also post these patches on the list.

---

Catalin Marinas (5):
      Add stack creation and deletion support to the new 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
      Allow stack.patchorder.all to return hidden patches


 stgit/commands/commit.py  |    4 +-
 stgit/commands/init.py    |   14 ++------
 stgit/commands/new.py     |    3 +-
 stgit/lib/git.py          |   55 ++++++++++++++++++++++++++++-
 stgit/lib/stack.py        |   84 ++++++++++++++++++++++++++++++++-------------
 stgit/lib/stackupgrade.py |    7 ++++
 6 files changed, 127 insertions(+), 40 deletions(-)

-- 
Catalin

[StGIT PATCH 1/5] Allow stack.patchorder.all to return hidden patches

[Catalin Marinas]

A new property, patchorder.all_visible, was added to return only the
applied + unapplied patches. This is used in the "commit" command to
avoid automatically committing the hidden patches.

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

 stgit/commands/commit.py  |    4 ++--
 stgit/lib/stack.py        |    4 ++--
 stgit/lib/stackupgrade.py |    7 +++++++
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/stgit/commands/commit.py b/stgit/commands/commit.py
index cc2f13a..1bdbeea 100644
--- a/stgit/commands/commit.py
+++ b/stgit/commands/commit.py
@@ -45,11 +45,11 @@ options = [make_option('-n', '--number', type = 'int',
 def func(parser, options, args):
     """Commit a number of patches."""
     stack = directory.repository.current_stack
-    args = common.parse_patches(args, list(stack.patchorder.all))
+    args = common.parse_patches(args, list(stack.patchorder.all_visible))
     if len([x for x in [args, options.number != None, options.all] if x]) > 1:
         parser.error('too many options')
     if args:
-        patches = [pn for pn in stack.patchorder.all if pn in args]
+        patches = [pn for pn in stack.patchorder.all_visible if pn in args]
         bad = set(args) - set(patches)
         if bad:
             raise common.CmdException('Bad patch names: %s'
diff --git a/stgit/lib/stack.py b/stgit/lib/stack.py
index f9e750e..bdd21b1 100644
--- a/stgit/lib/stack.py
+++ b/stgit/lib/stack.py
@@ -102,8 +102,8 @@ class PatchOrder(object):
                          lambda self, val: self.__set_list('unapplied', val))
     hidden = property(lambda self: self.__get_list('hidden'),
                       lambda self, val: self.__set_list('hidden', val))
-    # don't return the hidden patches, these have to be returned explicitly
-    all = property(lambda self: self.applied + self.unapplied)
+    all = property(lambda self: self.applied + self.unapplied + self.hidden)
+    all_visible = property(lambda self: self.applied + self.unapplied)
 
 class Patches(object):
     """Creates L{Patch} objects. Makes sure there is only one such object
diff --git a/stgit/lib/stackupgrade.py b/stgit/lib/stackupgrade.py
index 96ccb79..4b437dc 100644
--- a/stgit/lib/stackupgrade.py
+++ b/stgit/lib/stackupgrade.py
@@ -90,6 +90,13 @@ def update_to_current_format_version(repository, branch):
         rm_ref('refs/bases/%s' % branch)
         set_format_version(2)
 
+    # compatibility with the new infrastructure. The changes here do not
+    # affect the compatibility with the old infrastructure (format version 2)
+    if get_format_version() == 2:
+        hidden_file = os.path.join(branch_dir, 'hidden')
+        if not os.path.isfile(hidden_file):
+            utils.create_empty_file(hidden_file)
+
     # Make sure we're at the latest version.
     fv = get_format_version()
     if not fv in [None, FORMAT_VERSION]:

Re: [StGIT PATCH 1/5] Allow stack.patchorder.all to return hidden patches

[Karl Hasselström]

On 2008-06-04 22:13:17 +0100, Catalin Marinas wrote:

> A new property, patchorder.all_visible, was added to return only the
> applied + unapplied patches. This is used in the "commit" command to
> avoid automatically committing the hidden patches.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>

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

Provided you improve the error message (see below).

> diff --git a/stgit/commands/commit.py b/stgit/commands/commit.py
> index cc2f13a..1bdbeea 100644
> --- a/stgit/commands/commit.py
> +++ b/stgit/commands/commit.py
> @@ -45,11 +45,11 @@ options = [make_option('-n', '--number', type = 'int',
>  def func(parser, options, args):
>      """Commit a number of patches."""
>      stack = directory.repository.current_stack
> -    args = common.parse_patches(args, list(stack.patchorder.all))
> +    args = common.parse_patches(args, list(stack.patchorder.all_visible))
>      if len([x for x in [args, options.number != None, options.all] if x]) > 1:
>          parser.error('too many options')
>      if args:
> -        patches = [pn for pn in stack.patchorder.all if pn in args]
> +        patches = [pn for pn in stack.patchorder.all_visible if pn in args]
>          bad = set(args) - set(patches)
>          if bad:
>              raise common.CmdException('Bad patch names: %s'

Forbidding commit to operate on hidden patches? OK, why not? But
saying "Bad patch name" if the user gives a hidden patch is probably a
bit on the grumpy side ... :-)

> diff --git a/stgit/lib/stack.py b/stgit/lib/stack.py
> index f9e750e..bdd21b1 100644
> --- a/stgit/lib/stack.py
> +++ b/stgit/lib/stack.py
> @@ -102,8 +102,8 @@ class PatchOrder(object):
>                           lambda self, val: self.__set_list('unapplied', val))
>      hidden = property(lambda self: self.__get_list('hidden'),
>                        lambda self, val: self.__set_list('hidden', val))
> -    # don't return the hidden patches, these have to be returned explicitly
> -    all = property(lambda self: self.applied + self.unapplied)
> +    all = property(lambda self: self.applied + self.unapplied + self.hidden)
> +    all_visible = property(lambda self: self.applied + self.unapplied)
>  
>  class Patches(object):
>      """Creates L{Patch} objects. Makes sure there is only one such object

Nice compromise to the debate on whether "all" should include hidden
patches.

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

Re: [StGIT PATCH 1/5] Allow stack.patchorder.all to return hidden patches

[Catalin Marinas]

2008/6/5 Karl Hasselström <kha@treskal.com>:
> On 2008-06-04 22:13:17 +0100, Catalin Marinas wrote:
>>          if bad:
>>              raise common.CmdException('Bad patch names: %s'
>
> Forbidding commit to operate on hidden patches? OK, why not? But
> saying "Bad patch name" if the user gives a hidden patch is probably a
> bit on the grumpy side ... :-)

OK. Changed it to "Nonexistent or hidden patch names". Could be done
better but I wouldn't bother too much with this error.

-- 
Catalin

[StGIT PATCH 2/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 ++--
 2 files changed, 3 insertions(+), 4 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

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

[Karl Hasselström]

On 2008-06-04 22:13:25 +0100, Catalin Marinas wrote:

> This is to avoid confusion with the Stack.head function which
> returns a commit object rather than a file name.

A much better name. But you don't fix any callers -- aren't there any?

> The patch also changes the "new" function to use stack.head directly
> rather than via the Repository.refs... object.

Oh, here's a caller. But just one.

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

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

[Catalin Marinas]

2008/6/5 Karl Hasselström <kha@treskal.com>:
> On 2008-06-04 22:13:25 +0100, Catalin Marinas wrote:
>
>> This is to avoid confusion with the Stack.head function which
>> returns a commit object rather than a file name.
>
> A much better name. But you don't fix any callers -- aren't there any?

There was actually another caller but missed it since I worked on a
bigger initially. The stack.Repository.current_branch uses it but this
was later moved to git.Repository.current_branch_name. I'll add in
case we ever need to bisect.

-- 
Catalin

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

[Karl Hasselström]

On 2008-06-05 12:49:15 +0100, Catalin Marinas wrote:

> 2008/6/5 Karl Hasselström <kha@treskal.com>:
>
> > A much better name. But you don't fix any callers -- aren't there any?
>
> There was actually another caller but missed it since I worked on a
> bigger initially. The stack.Repository.current_branch uses it but
> this was later moved to git.Repository.current_branch_name. I'll add
> in case we ever need to bisect.

Ah, good.

( Did you run the test suite on your patches? I always do something
  like this (untested -- I have this command in my bash history on
  another computer)

    $ stg pop -a
    $ while make test; do stg push || break; done

  before sending out patches. It takes a few minutes per patch, but
  it's fully automatic. )

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

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

[Catalin Marinas]

2008/6/5 Karl Hasselström <kha@treskal.com>:
> On 2008-06-05 12:49:15 +0100, Catalin Marinas wrote:
> ( Did you run the test suite on your patches? I always do something
>  like this (untested -- I have this command in my bash history on
>  another computer)
>
>    $ stg pop -a
>    $ while make test; do stg push || break; done
>
>  before sending out patches. It takes a few minutes per patch, but
>  it's fully automatic. )

Not really. It would be good to have a Makefile target for this,
something like "test_patches". I'll add it.

-- 
Catalin

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

[Karl Hasselström]

On 2008-06-05 13:06:39 +0100, Catalin Marinas wrote:

> 2008/6/5 Karl Hasselström <kha@treskal.com>:
>
> > ( Did you run the test suite on your patches? I always do
> >   something like this (untested -- I have this command in my bash
> >   history on another computer)
> >
> >    $ stg pop -a
> >    $ while make test; do stg push || break; done
> >
> >  before sending out patches. It takes a few minutes per patch, but
> >  it's fully automatic. )
>
> Not really. It would be good to have a Makefile target for this,
> something like "test_patches". I'll add it.

Good idea. But consider the case where the user doesn't want to test
all the patches -- maybe you could give make FIRST_PATCH and/or
LAST_PATCH parameters? (Defaulting to all patches, of course.)

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

[StGIT PATCH 3/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   |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 stgit/lib/stack.py |   25 ++++---------------------
 2 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index fd66f6d..6393af2 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,47 @@ 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 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)
+        self._repository.run(['git', 'branch', create_at.sha1]).discard_output()
diff --git a/stgit/lib/stack.py b/stgit/lib/stack.py
index bdd21b1..aca7a36 100644
--- a/stgit/lib/stack.py
+++ b/stgit/lib/stack.py
@@ -130,34 +130,20 @@ 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)."""
     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 +163,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)
-    @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]

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

[Karl Hasselström]

On 2008-06-04 22:13:35 +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.

Looks good except for a few minor points.

> @@ -619,3 +626,47 @@ 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 Git branch."""

Missing "a".

> +    def __init__(self, repository, name):
> +        self._repository = repository
> +        self._name = name

Using double leading underscores will give you a class-local name. Why
just one?

> +    @classmethod
> +    def create(cls, repository, name, create_at = None):

Minor nit: in case of things that are clearly "owned" by other things
(git objects are owned by a git repository, for example), I've tried
to put the creation function in the owner, not the owned. I'm not
insisting, though.

> +        """Create a new Git branch and return the corresponding L{Branch} object."""

Long line.

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

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

[Catalin Marinas]

2008/6/5 Karl Hasselström <kha@treskal.com>:
> On 2008-06-04 22:13:35 +0100, Catalin Marinas wrote:
>> +    def __init__(self, repository, name):
>> +        self._repository = repository
>> +        self._name = name
>
> Using double leading underscores will give you a class-local name. Why
> just one?

Just a convention: double underscore - private, simple underscore -
protected (using C++ or Java naming). The _repository or _name are
meant to be made available to a derived class (stack.Stack in our
case), though we have the corresponding public properties already.

>> +    @classmethod
>> +    def create(cls, repository, name, create_at = None):
>
> Minor nit: in case of things that are clearly "owned" by other things
> (git objects are owned by a git repository, for example), I've tried
> to put the creation function in the owner, not the owned. I'm not
> insisting, though.

As I see these things, the git.Repository and the derived
stack.Repository are indeed object factories but they are mainly
responsible for creating/returning a new Python object and passing
some arguments to the class' __init__. The Repository object doesn't
need to know more than the name as the rest is handled by __init__.

The same goes for the "create" function, the repository shouldn't need
to know the directory or files structure that a Stack or Branch are
using. I think we get a clearer separation this way.

-- 
Catalin

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

[Karl Hasselström]

On 2008-06-05 13:03:55 +0100, Catalin Marinas wrote:

> 2008/6/5 Karl Hasselström <kha@treskal.com>:
>
> > On 2008-06-04 22:13:35 +0100, Catalin Marinas wrote:
> >
> > > +    def __init__(self, repository, name):
> > > +        self._repository = repository
> > > +        self._name = name
> >
> > Using double leading underscores will give you a class-local name.
> > Why just one?
>
> Just a convention: double underscore - private, simple underscore -
> protected (using C++ or Java naming). The _repository or _name are
> meant to be made available to a derived class (stack.Stack in our
> case), though we have the corresponding public properties already.

OK. Then I'd argue that these two should be private since there's no
particular reason for them to be just protected, but I think I've done
enough nitpicking for one day already!

> >> +    @classmethod
> >> +    def create(cls, repository, name, create_at = None):
> >
> > Minor nit: in case of things that are clearly "owned" by other
> > things (git objects are owned by a git repository, for example),
> > I've tried to put the creation function in the owner, not the
> > owned. I'm not insisting, though.
>
> As I see these things, the git.Repository and the derived
> stack.Repository are indeed object factories but they are mainly
> responsible for creating/returning a new Python object and passing
> some arguments to the class' __init__. The Repository object doesn't
> need to know more than the name as the rest is handled by __init__.

Yes. This is exactly how it works e.g. when a Repository creates Blob,
Tree, or Commit objects.

> The same goes for the "create" function, the repository shouldn't
> need to know the directory or files structure that a Stack or Branch
> are using. I think we get a clearer separation this way.

Yes, you're right. Knowledge of internals belong in the class itself.

We'll have to be watchful against attempts to create objects other
than via the officially designated factories, though. Python doesn't
really have any mechanisms that help us here.

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

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

[Catalin Marinas]

2008/6/5 Karl Hasselström <kha@treskal.com>:
> On 2008-06-05 13:03:55 +0100, Catalin Marinas wrote:
>> 2008/6/5 Karl Hasselström <kha@treskal.com>:
>> The same goes for the "create" function, the repository shouldn't
>> need to know the directory or files structure that a Stack or Branch
>> are using. I think we get a clearer separation this way.
>
> Yes, you're right. Knowledge of internals belong in the class itself.
>
> We'll have to be watchful against attempts to create objects other
> than via the officially designated factories, though. Python doesn't
> really have any mechanisms that help us here.

Doesn't need to be enforced. The Stack and Branch objects can be
initialised by themselves provided that the repository is passed as
argument at construction. The factory is more for convenience, passing
the repository to the newly created object.

If you really want enforcing this, it can probably be done by hacking
__new__ and __init__.

-- 
Catalin

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

[Karl Hasselström]

On 2008-06-06 09:44:37 +0100, Catalin Marinas wrote:

> 2008/6/5 Karl Hasselström <kha@treskal.com>:
>
> > We'll have to be watchful against attempts to create objects other
> > than via the officially designated factories, though. Python
> > doesn't really have any mechanisms that help us here.
>
> Doesn't need to be enforced. The Stack and Branch objects can be
> initialised by themselves provided that the repository is passed as
> argument at construction. The factory is more for convenience,
> passing the repository to the newly created object.

Those classes cache stuff. So if you have, say, two PatchOrder objects
representing the same underlying files, an update in one will not be
reflected in the other. An important role of the factory is to make
sure that if an object is asked for twice, the _same_ object is
returned to both callers.

The Commit, Tree, and Blob objects produced by Repository are another
example where we rely on there being no more than one Python object
representing the same underlying data.

> If you really want enforcing this, it can probably be done by
> hacking __new__ and __init__.

No, I reckon you're right -- it's overkill. StGit is small enough that
we can probably enforce the relevant restrictions by hand.

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

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

[Catalin Marinas]

On 07/06/2008, Karl Hasselström <kha@treskal.com> wrote:
> On 2008-06-06 09:44:37 +0100, Catalin Marinas wrote:
>  > 2008/6/5 Karl Hasselström <kha@treskal.com>:
> > > We'll have to be watchful against attempts to create objects other
> > > than via the officially designated factories, though. Python
> > > doesn't really have any mechanisms that help us here.
> >
> > If you really want enforcing this, it can probably be done by
> > hacking __new__ and __init__.
>
> No, I reckon you're right -- it's overkill. StGit is small enough that
>  we can probably enforce the relevant restrictions by hand.

Hidden plan - once the new stgit.lib.git module is complete, we can
advertise it as a stand-alone object-oriented Git wrapper for Python
:-). At that point, we can think about enforcing the factory
mechanism.

-- 
Catalin

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

[David Aguilar]

On Sun, Jun 8, 2008 at 3:16 PM, Catalin Marinas
<catalin.marinas@gmail.com> wrote:
> On 07/06/2008, Karl Hasselström <kha@treskal.com> wrote:
>> On 2008-06-06 09:44:37 +0100, Catalin Marinas wrote:
>>  > 2008/6/5 Karl Hasselström <kha@treskal.com>:
>> > > We'll have to be watchful against attempts to create objects other
>> > > than via the officially designated factories, though. Python
>> > > doesn't really have any mechanisms that help us here.
>> >
>> > If you really want enforcing this, it can probably be done by
>> > hacking __new__ and __init__.
>>
>> No, I reckon you're right -- it's overkill. StGit is small enough that
>>  we can probably enforce the relevant restrictions by hand.
>
> Hidden plan - once the new stgit.lib.git module is complete, we can
> advertise it as a stand-alone object-oriented Git wrapper for Python
> :-). At that point, we can think about enforcing the factory
> mechanism.
>
> --
> Catalin

Surely you don't want people to have to install stgit just to "import git"?
(or perhaps you do, since that'd give people incentive to install stgit ;))

Back to the point...

A few folks are working towards an OO git interface for python.
Perhaps joining forces would be a big win for everyone?

http://gitorious.org/projects/git-python

It would be really cool if we could wrangle in all interested parties
towards a common goal for python.  You guys (the stgit team) have a
lot of experience in this domain so your ideas and opinions would be
greatly appreciated.

-- 
 David

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

[Sverre Rabbelier]

On Mon, Jun 9, 2008 at 2:07 AM, David Aguilar <davvid@gmail.com> wrote:
> A few folks are working towards an OO git interface for python.
> Perhaps joining forces would be a big win for everyone?
>
> http://gitorious.org/projects/git-python
>
> It would be really cool if we could wrangle in all interested parties
> towards a common goal for python.  You guys (the stgit team) have a
> lot of experience in this domain so your ideas and opinions would be
> greatly appreciated.

I very much agree with you. I'm using git-python for my GSoC project
myself (although currently I'm only using the Git object really). I
think it would be awesome if git-python and stgit.lib.git would merge.
I'd be happy to help with that in any way I can :).

-- 
Cheers,

Sverre Rabbelier

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

[Karl Hasselström]

On 2008-06-08 23:16:02 +0100, Catalin Marinas wrote:

> Hidden plan - once the new stgit.lib.git module is complete, we can
> advertise it as a stand-alone object-oriented Git wrapper for Python
> :-).

Aren't there already a couple of those? :-)

Though I agree that having such a future separation in mind is useful
for making StGit nice and modular. Whether we actually carry it out in
the end would depend on whether someone comes along and actually wants
to use it, I guess.

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

[StGIT PATCH 4/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 |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/stgit/lib/stack.py b/stgit/lib/stack.py
index aca7a36..7375d41 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 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."""
@@ -133,12 +145,14 @@ class Patches(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):
         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)
+            raise StackException('%s: branch not initialized' % name)
     patchorder = property(lambda self: self.__patchorder)
     patches = property(lambda self: self.__patches)
     @property
@@ -156,6 +170,45 @@ class Stack(git.Branch):
             return True
         return self.head == self.patches.get(self.patchorder.applied[-1]).commit
 
+    def set_parents(self, remote, localbranch):
+        if not localbranch:
+            return
+        if remote:
+            self.set_parent_remote(remote)
+        self.set_parent_branch(localbranch)
+        config.set('branch.%s.stgit.parentbranch' % self._name, localbranch)
+
+    @classmethod
+    def initialise(cls, repository, name = None):
+        """Initialise a Git branch to handle patch series."""
+        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."""
+        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."""

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

[Karl Hasselström]

On 2008-06-04 22:13:43 +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.

> diff --git a/stgit/lib/stack.py b/stgit/lib/stack.py
> index aca7a36..7375d41 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 stack objects."""

s/stack/L{Stack}/, perhaps?

> @@ -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."""

Wouldn't it be more consistent if the create function actually
returned a PatchOrder object, like other creation functions? (You
might even consider having these files auto-created whenever you
instantiate a PatchOrder object and they don't yet exist.)

Also, the creation function might instead live in the Stack class,
since it owns the patch order.

> @@ -133,12 +145,14 @@ class Patches(object):
>  class Stack(git.Branch):
>      """Represents an StGit stack (that is, a git branch with some extra
>      metadata)."""
> +    __repo_subdir = 'patches'
> +

This needs to be in the previous patch, I think, since you use it
there.

> +    def set_parents(self, remote, localbranch):
> +        if not localbranch:
> +            return
> +        if remote:
> +            self.set_parent_remote(remote)
> +        self.set_parent_branch(localbranch)
> +        config.set('branch.%s.stgit.parentbranch' % self._name, localbranch)

Hmm, I don't quite follow. Why is this a no-op if you give a false
localbranch? And why is branch.<branchname>.stgit.parentbranch needed,
when it's always the same as branch.<branchname>.merge? (Backwards
compatibility? Would you mind making a comment about that, in that
case?)

> +    @classmethod
> +    def initialise(cls, repository, name = None):
> +        """Initialise a Git branch to handle patch series."""
> +        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)

This is not quite like the other "create" functions, since it just
promotes a branch, without really creating it.

What I'd really like to see here, I think, is something like this:

  1. You get a Stack object from some stack.Repository method.

  2. The Stack object works without having to be initialized, but the
     operations that need initialization throw an exception.

  3. The Stack object has an initialize() method -- just a normal
     method, not a class method.

This will pave the way for automatic initialization -- just call
self.initialize() instead of throwing an exception in step (2).

What do you think?

> +
> +    @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."""
> +        git.Branch.create(repository, name, create_at = create_at)
> +        stack = cls.initialise(repository, name)
> +        stack.set_parents(parent_remote, parent_branch)
> +        return stack

Same point as with the other creation functions.

And I'd appreciate some documentation on what the parameters mean --
either here, or in the methods you call from here.

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

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

[Catalin Marinas]

2008/6/5 Karl Hasselström <kha@treskal.com>:
> On 2008-06-04 22:13:43 +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.
>
>> diff --git a/stgit/lib/stack.py b/stgit/lib/stack.py
>> index aca7a36..7375d41 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 stack objects."""
>
> s/stack/L{Stack}/, perhaps?

I was more referring to objects in the lib.stack module, not only the
Stack class. Can this be expressed in any way with the epydoc format?

>> @@ -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."""
>
> Wouldn't it be more consistent if the create function actually
> returned a PatchOrder object, like other creation functions?

It is a bit more complicated. PatchOrder requires a Stack object in
__init__ but the stack is not fully set up at this point.

> (You
> might even consider having these files auto-created whenever you
> instantiate a PatchOrder object and they don't yet exist.)

This is definitely a solution but at the moment we have to support the
old infrastructure which fails if the files aren't present. Once we
don't have any reference to the stgit.stack module, we can do this
more dynamically.

> Also, the creation function might instead live in the Stack class,
> since it owns the patch order.

See my reply to a previous patch with my view on ownership and
separation of functionality.

>> @@ -133,12 +145,14 @@ class Patches(object):
>>  class Stack(git.Branch):
>>      """Represents an StGit stack (that is, a git branch with some extra
>>      metadata)."""
>> +    __repo_subdir = 'patches'
>> +
>
> This needs to be in the previous patch, I think, since you use it
> there.

Yes.

>> +    def set_parents(self, remote, localbranch):
>> +        if not localbranch:
>> +            return
>> +        if remote:
>> +            self.set_parent_remote(remote)
>> +        self.set_parent_branch(localbranch)
>> +        config.set('branch.%s.stgit.parentbranch' % self._name, localbranch)
>
> Hmm, I don't quite follow. Why is this a no-op if you give a false
> localbranch? And why is branch.<branchname>.stgit.parentbranch needed,
> when it's always the same as branch.<branchname>.merge? (Backwards
> compatibility? Would you mind making a comment about that, in that
> case?)

I don't fully follow it either :-). I just copied the code from
stgit.stack.Series but I don't make any use of it yet. I think it was
initially written by Yann Dirson for the "branch" command which I
haven't translated yet. I think branch.<branchname>.stgit.parentbranch
might be used by the current "pull" or "rebase" implementations. I'll
comment it out (but still keep it for now) and put a FIXME so that I
remember when converting the "branch" and "pull" commands.

>> +    @classmethod
>> +    def initialise(cls, repository, name = None):
>> +        """Initialise a Git branch to handle patch series."""
>> +        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)
>
> This is not quite like the other "create" functions, since it just
> promotes a branch, without really creating it.

That was the intention. For creation, there is Stack.create which
handles the Git branch creation as well as the StGIT initialisation.

> What I'd really like to see here, I think, is something like this:
>
>  1. You get a Stack object from some stack.Repository method.
>
>  2. The Stack object works without having to be initialized, but the
>     operations that need initialization throw an exception.
>
>  3. The Stack object has an initialize() method -- just a normal
>     method, not a class method.

That was the original behaviour in stgit.stack.Series but if you
happen to try to access a patch or patchorder, you get some exception
on missing file form those objects rather than "stack not initialised"
from Stack.

> This will pave the way for automatic initialization -- just call
> self.initialize() instead of throwing an exception in step (2).

I'm not fully convinced with automatic initialisation. I have some
branches where I don't use StGIT but I might type a "stg series" by
mistake. The branch will be promoted to a stack but later Git doesn't
know about extra files in .git/patches and they aren't handled
(removed, renamed etc.). I'm more in favour of the explicit
initialisation.

>> +    @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."""
>> +        git.Branch.create(repository, name, create_at = create_at)
>> +        stack = cls.initialise(repository, name)
>> +        stack.set_parents(parent_remote, parent_branch)
>> +        return stack
>
> Same point as with the other creation functions.
>
> And I'd appreciate some documentation on what the parameters mean --
> either here, or in the methods you call from here.

Yes, I'll add some description.

Thanks for your comments.

-- 
Catalin

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

[Karl Hasselström]

On 2008-06-05 13:42:03 +0100, Catalin Marinas wrote:

> 2008/6/5 Karl Hasselström <kha@treskal.com>:
>
> > On 2008-06-04 22:13:43 +0100, Catalin Marinas wrote:
> >
> > > +from stgit.config import config
> > > +
> > > +class StackException(exception.StgException):
> > > +    """Exception raised by stack objects."""
> >
> > s/stack/L{Stack}/, perhaps?
>
> I was more referring to objects in the lib.stack module, not only
> the Stack class. Can this be expressed in any way with the epydoc
> format?

I think L{lib.stack} would work.

See http://epydoc.sourceforge.net/manual-epytext.html.

> > Wouldn't it be more consistent if the create function actually
> > returned a PatchOrder object, like other creation functions?
>
> It is a bit more complicated. PatchOrder requires a Stack object in
> __init__ but the stack is not fully set up at this point.

I see. How about calling the method "create_files" or something, then,
to avoid breaking with the newly established convention of having the
create() function create new underlying data and returning a Python
object that represents it?

> > (You might even consider having these files auto-created whenever
> > you instantiate a PatchOrder object and they don't yet exist.)
>
> This is definitely a solution but at the moment we have to support
> the old infrastructure which fails if the files aren't present. Once
> we don't have any reference to the stgit.stack module, we can do
> this more dynamically.

Right.

> > Also, the creation function might instead live in the Stack class,
> > since it owns the patch order.
>
> See my reply to a previous patch with my view on ownership and
> separation of functionality.

Yes, I agree with you now.

> > > +    def set_parents(self, remote, localbranch):
> > > +        if not localbranch:
> > > +            return
> > > +        if remote:
> > > +            self.set_parent_remote(remote)
> > > +        self.set_parent_branch(localbranch)
> > > +        config.set('branch.%s.stgit.parentbranch' % self._name, localbranch)
> >
> > Hmm, I don't quite follow. Why is this a no-op if you give a false
> > localbranch? And why is branch.<branchname>.stgit.parentbranch
> > needed, when it's always the same as branch.<branchname>.merge?
> > (Backwards compatibility? Would you mind making a comment about
> > that, in that case?)
>
> I don't fully follow it either :-).

Hmmm. :-)

> I just copied the code from stgit.stack.Series but I don't make any
> use of it yet. I think it was initially written by Yann Dirson for
> the "branch" command which I haven't translated yet. I think
> branch.<branchname>.stgit.parentbranch might be used by the current
> "pull" or "rebase" implementations. I'll comment it out (but still
> keep it for now) and put a FIXME so that I remember when converting
> the "branch" and "pull" commands.

In cases like this, when one wants to keep existing behavior unchanged
but doesn't quite understand the original reasons behind it, I like to
simply insert a big comment saying that this piece of code does this
strange thing for reasons unknown to me, but I kept them that way
because I didn't want to change the behavior at the moment.

( As an aside, I've also been thinking about pull/rebase. Let's have a
  discussion about that -- in another thread. )

> > What I'd really like to see here, I think, is something like this:
> >
> >  1. You get a Stack object from some stack.Repository method.
> >
> >  2. The Stack object works without having to be initialized, but
> >     the operations that need initialization throw an exception.
> >
> >  3. The Stack object has an initialize() method -- just a normal
> >     method, not a class method.
>
> That was the original behaviour in stgit.stack.Series but if you
> happen to try to access a patch or patchorder, you get some
> exception on missing file form those objects rather than "stack not
> initialised" from Stack.

This wouldn't be a problem if all methods that needed initialization
begain with a self.make_sure_were_initialized() call, which could
throw a suitable exception. (Though I see how this could get a touch
unwieldy -- it's probably not something you'd want to do unless you
_are_ shooting for auto-initialization.)

> I'm not fully convinced with automatic initialisation. I have some
> branches where I don't use StGIT but I might type a "stg series" by
> mistake. The branch will be promoted to a stack but later Git
> doesn't know about extra files in .git/patches and they aren't
> handled (removed, renamed etc.). I'm more in favour of the explicit
> initialisation.

Automatic initialization should work such that "initialize" is a
no-op. For example, the "applied" and "unapplied" files shouldn't even
exist if there are no patch names in them. That way, StGit metadata
files (and refs) are only created when there's an actual need for them
-- meaning that read-only StGit commands would never create any such
files.

Related to this: Have you looked in any detail at the "undo" stuff in
my experimental branch? The patch stack log that makes undo possible
contains all the metadata that StGit needs, accessible from a single
ref. I currently keep writing all the old metadata files as well, but
my long-term evil plan is to have only the stack log and nothing else.
If not earlier, I'd like to propose a move to auto-init at the same
time.

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

[StGIT PATCH 5/5] Add stack creation and deletion support to the new 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)

Re: [StGIT PATCH 5/5] Add stack creation and deletion support to the new infrastructure

[Karl Hasselström]

The commit message is misleading. What this patch does is make "stg
init" _use_ the new-infrastructure stack init support added in your
previous patches.

On 2008-06-04 22:13:52 +0100, Catalin Marinas wrote:

> 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

Lots of imports gone, and the remaining ones aren't spilled directly
into the local namespace. Very nice.

> -    crt_series.init()
> +    stack.Stack.initialise(directory.repository)

This would need minor tweaking if you decide to take my advice for
4/5.

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

Re: [StGIT PATCH 5/5] Add stack creation and deletion support to the new infrastructure

[Catalin Marinas]

2008/6/5 Karl Hasselström <kha@treskal.com>:
> The commit message is misleading. What this patch does is make "stg
> init" _use_ the new-infrastructure stack init support added in your
> previous patches.

Yes, it's misleading. I had a big patch containing the previous 3-4
patches and split it afterwards. This being the last one, I forgot to
change the commit message.

> On 2008-06-04 22:13:52 +0100, Catalin Marinas wrote:
>> -    crt_series.init()
>> +    stack.Stack.initialise(directory.repository)
>
> This would need minor tweaking if you decide to take my advice for
> 4/5.

I'll reply to 4/5 separately.

-- 
Catalin

Re: [StGIT PATCH 0/5] Various updates to the new infrastructure

[Karl Hasselström]

On 2008-06-04 22:13:08 +0100, Catalin Marinas wrote:

> I'll also post these patches on the list.

I hope my copious whining doesn't make you regret it. ;-)

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