[RFC][StGit PATCH] Add support for merge-friendly branches

[RFC][StGit PATCH] Add support for merge-friendly branches

[Catalin Marinas]

The main issue with publishing StGit branches is that the Git history
represented by patches is volatile, making it difficult for people
wanting to merge such branch. One solution is for all the downstream
developers to always rebase but that's not always desirable. Another
solution is provided by tools like TopGit but the visible Git history
becomes complicated, especially with repeated reordering.

The patch proposes a new StGit command called "publish". This command
allows one to develop patches normally on a StGit branch but publish the
stack changes to a separate, merge-friendly branch whose history is not
re-writable.

More about its behaviour can be found in the command description in this
patch.

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---
 stgit/commands/common.py  |   26 ++++++++
 stgit/commands/new.py     |   22 -------
 stgit/commands/publish.py |  139 +++++++++++++++++++++++++++++++++++++++++++++
 stgit/lib/git.py          |    5 ++
 t/t4100-publish.sh        |  129 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 300 insertions(+), 21 deletions(-)
 create mode 100644 stgit/commands/publish.py
 create mode 100755 t/t4100-publish.sh

diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index e46412e..04314f3 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -447,6 +447,32 @@ def readonly_constant_property(f):
         return getattr(self, n)
     return property(new_f)
 
+def update_commit_data(cd, options, allow_edit = False):
+    """Return a new CommitData object updated according to the command line
+    options."""
+    # Set the commit message from commandline.
+    if options.message != None:
+        cd = cd.set_message(options.message)
+
+    # Modify author data.
+    cd = cd.set_author(options.author(cd.author))
+
+    # Add Signed-off-by: or similar.
+    if options.sign_str != None:
+        sign_str = options.sign_str
+    else:
+        sign_str = config.get("stgit.autosign")
+    if sign_str != None:
+        cd = cd.set_message(
+            add_sign_line(cd.message, sign_str,
+                          cd.committer.name, cd.committer.email))
+
+    # Let user edit the commit message manually.
+    if allow_edit and not options.message:
+        cd = cd.set_message(edit_string(cd.message, '.stgit-new.txt'))
+
+    return cd
+
 class DirectoryException(StgException):
     pass
 
diff --git a/stgit/commands/new.py b/stgit/commands/new.py
index 2c98431..9fd51c3 100644
--- a/stgit/commands/new.py
+++ b/stgit/commands/new.py
@@ -67,32 +67,12 @@ def func(parser, options, args):
     cd = gitlib.CommitData(
         tree = stack.head.data.tree, parents = [stack.head], message = '',
         author = gitlib.Person.author(), committer = gitlib.Person.committer())
-
-    # Set patch commit message from commandline.
-    if options.message != None:
-        cd = cd.set_message(options.message)
-
-    # Modify author data.
-    cd = cd.set_author(options.author(cd.author))
-
-    # Add Signed-off-by: or similar.
-    if options.sign_str != None:
-        sign_str = options.sign_str
-    else:
-        sign_str = config.get("stgit.autosign")
-
-    if sign_str != None:
-        cd = cd.set_message(
-            utils.add_sign_line(cd.message, sign_str,
-                                cd.committer.name, cd.committer.email))
+    cd = common.update_commit_data(cd, options, allow_edit = True)
 
     if options.save_template:
         options.save_template(cd.message)
         return utils.STGIT_SUCCESS
 
-    # Let user edit the commit message manually.
-    if not options.message:
-        cd = cd.set_message(utils.edit_string(cd.message, '.stgit-new.txt'))
     if name == None:
         name = utils.make_patch_name(cd.message,
                                      lambda name: stack.patches.exists(name))
diff --git a/stgit/commands/publish.py b/stgit/commands/publish.py
new file mode 100644
index 0000000..06c32d0
--- /dev/null
+++ b/stgit/commands/publish.py
@@ -0,0 +1,139 @@
+__copyright__ = """
+Copyright (C) 2009, Catalin Marinas <catalin.marinas@gmail.com>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License version 2 as
+published by the Free Software Foundation.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+"""
+
+from stgit import argparse
+from stgit.argparse import opt
+from stgit.commands import common
+from stgit.config import config
+from stgit.lib import git, stack
+from stgit.out import out
+
+help = 'Publish the stack changes to a merge-friendly head'
+kind = 'stack'
+usage = ['[options] [branch]']
+description = """
+This command commits a set of changes on a separate (called public) branch
+based on the modifications of the given or current stack. The history of the
+public branch is not re-written, making it merge-friendly and feasible for
+publishing. The heads of the stack and public branch may be different but the
+corresponding tree objects are always the same.
+
+If the trees of the stack and public branch are different (otherwise the
+command has no effect), StGit first checks for a rebase of the stack since the
+last publishing. If a rebase is detected, StGit creates a commit on the public
+branch corresponding to a merge between the new stack base and the latest
+public head.
+
+If no rebasing was detected, StGit checks for new patches that may have been
+created on top of the stack since the last publishing. If new patches are
+found and are not empty, they are checked into the public branch keeping the
+same commit information (e.g. log message, author, committer, date).
+
+If the above tests fail (e.g. patches modified or removed), StGit creates a
+new commit on the public branch having the same tree as the stack but the
+public head as its parent. The editor will be invoked if no "--message" option
+is given.
+
+It is recommended that stack modifications falling in different categories as
+described above are separated by a publish command in order to keep the public
+branch history cleaner (otherwise StGit would generate a big commit including
+several stack modifications).
+
+The public branch name can be set via the branch.<branch>.public configuration
+variable (defaulting to "<branch>.public").
+"""
+
+args = [argparse.all_branches]
+options = [
+    opt('-b', '--branch', args = [argparse.stg_branches],
+        short = 'Use BRANCH instead of the default branch')
+] + (argparse.author_options()
+     + argparse.message_options(save_template = False)
+     + argparse.sign_options())
+
+directory = common.DirectoryHasRepositoryLib()
+
+def __create_commit(repository, tree, parents, options):
+    """Return a new Commit object."""
+    cd = git.CommitData(
+        tree = tree, parents = parents, message = '',
+        author = git.Person.author(), committer = git.Person.committer())
+    cd = common.update_commit_data(cd, options, allow_edit = True)
+
+    return repository.commit(cd)
+
+def func(parser, options, args):
+    """Publish the stack changes."""
+    repository = directory.repository
+    stack = repository.get_stack(options.branch)
+
+    if not args:
+        public_ref = config.get('branch.%s.public' % stack.name)
+        if not public_ref:
+            public_ref = 'refs/heads/%s.public' % stack.name
+    elif len(args) == 1:
+        public_ref = args[0]
+    else:
+        parser.error('incorrect number of arguments')
+
+    # just clone the stack if the public ref does not exist
+    if not repository.refs.exists(public_ref):
+        repository.refs.set(public_ref, stack.head, 'publish')
+        out.info('Created "%s"' % public_ref)
+        return
+
+    public_head = repository.refs.get(public_ref)
+    public_tree = public_head.data.tree
+
+    # check for same tree (already up to date)
+    if public_tree.sha1 == stack.head.data.tree.sha1:
+        out.info('"%s" already up to date' % public_ref)
+        return
+
+    # check for rebased stack. In this case we emulate a merge with the stack
+    # base by setting two parents.
+    merge_base = repository.get_merge_base(public_head, stack.base)
+    if merge_base.sha1 != stack.base.sha1:
+        public_head = __create_commit(repository, stack.head.data.tree,
+                                      [public_head, stack.base], options)
+        repository.refs.set(public_ref, public_head, 'publish')
+        out.info('Merged the stack base into "%s"' % public_ref)
+        return
+
+    # check for new patches from the last publishing. This is done by checking
+    # whether the public tree is the same as the bottom of the checked patch.
+    # If older patches were modified, new patches cannot be detected. The new
+    # patches and their metadata are pushed directly to the published head.
+    for p in stack.patchorder.applied:
+        pc = stack.patches.get(p).commit
+        if public_tree.sha1 == pc.data.parent.data.tree.sha1:
+            if pc.data.is_nochange():
+                out.info('Ignored new empty patch "%s"' % p)
+                continue
+            cd = pc.data.set_parent(public_head)
+            public_head = repository.commit(cd)
+            public_tree = public_head.data.tree
+            out.start('Published new patch "%s"' % p)
+
+    # create a new commit (only happens if no new patches are detected)
+    if public_tree.sha1 != stack.head.data.tree.sha1:
+        public_head = __create_commit(repository, stack.head.data.tree,
+                                      [public_head], options)
+
+    # update the public head
+    repository.refs.set(public_ref, public_head, 'publish')
+    out.info('Updated "%s"' % public_ref)
diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 6f2c977..4a17c8a 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -609,6 +609,11 @@ class Repository(RunWithEnv):
             raise DetachedHeadException()
     def set_head_ref(self, ref, msg):
         self.run(['git', 'symbolic-ref', '-m', msg, 'HEAD', ref]).no_output()
+    def get_merge_base(self, commit1, commit2):
+        """Return the merge base of two commits."""
+        sha1 = self.run(['git', 'merge-base',
+                         commit1.sha1, commit2.sha1]).output_one_line()
+        return self.get_commit(sha1)
     def simple_merge(self, base, ours, theirs):
         index = self.temp_index()
         try:
diff --git a/t/t4100-publish.sh b/t/t4100-publish.sh
new file mode 100755
index 0000000..17e07bc
--- /dev/null
+++ b/t/t4100-publish.sh
@@ -0,0 +1,129 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Catalin Marinas
+#
+
+test_description='Exercise the publish command.
+
+Create/modify patches on the stack and publish them to a separate branch.'
+
+. ./test-lib.sh
+
+test_same_tree () {
+	stack_tree=$(git rev-parse master^{tree})
+	public_tree=$(git rev-parse master.public^{tree})
+	test "$stack_tree" = "$public_tree"
+}
+
+test_expect_success \
+	'Initialize the StGit repository' \
+	'
+	stg init
+	'
+
+test_expect_success \
+	'Create some patches' \
+	'
+	stg new p1 -m p1 &&
+	echo foo1 > foo1.txt &&
+	git add foo1.txt &&
+	stg refresh &&
+	stg new p2 -m p2 &&
+	echo foo2 > foo2.txt &&
+	git add foo2.txt &&
+	stg refresh &&
+	stg new p3 -m p3 &&
+	echo foo3 > foo3.txt &&
+	git add foo3.txt &&
+	stg refresh
+	'
+
+test_expect_success \
+	'Publish the stack for the first time' \
+	'
+	stg publish &&
+	test "$(stg id)" = "$(stg id master.public)"
+	'
+
+test_expect_success \
+	'Modify a patch and publish the changes' \
+	'
+	stg pop &&
+	echo foo2 >> foo2.txt &&
+	stg refresh &&
+	stg push &&
+	old_public=$(stg id master.public) &&
+	stg publish -m "p2 updated" &&
+	test_same_tree &&
+	new_public=$(stg id master.public) &&
+	test "$(git rev-list $old_public..$new_public | wc -l)" = "1"
+	'
+
+test_expect_success \
+	'Create new patches and publish them' \
+	'
+	stg new p4 -m p4 &&
+	echo foo4 > foo4.txt &&
+	git add foo4.txt &&
+	stg refresh &&
+	stg new p5 -m p5 &&
+	echo foo5 > foo5.txt &&
+	git add foo5.txt &&
+	stg refresh &&
+	stg new empty -m empty &&
+	old_public=$(stg id master.public) &&
+	stg publish -m "Ignored message" &&
+	test_same_tree &&
+	new_public=$(stg id master.public) &&
+	test "$(git rev-list $old_public..$new_public | wc -l)" = "2"
+	'
+
+test_expect_success \
+	'Rebase the current stack and publish a merge' \
+	'
+	stg pop -a &&
+	echo foo0 > foo0.txt &&
+	git add foo0.txt &&
+	git commit -m "foo0.txt added" &&
+	stg push -a &&
+	old_public=$(stg id master.public) &&
+	stg publish -m "Merge with base" &&
+	test_same_tree &&
+	new_public=$(stg id master.public) &&
+	test "$(git rev-list $old_public..$new_public | wc -l)" = "2" &&
+	test "$(git merge-base master.public master)" = "$(stg id {base})"
+	'
+
+test_expect_success \
+	'Re-publish without any changes' \
+	'
+	old_public=$(stg id master.public) &&
+	stg publish -m "Ignored message" &&
+	test_same_tree &&
+	new_public=$(stg id master.public) &&
+	test "$old_public" = "$new_public"
+	'
+
+test_expect_success \
+	'Reorder patches and publish the changes' \
+	'
+	stg float p5 p4 p3 p2 p1 &&
+	old_public=$(stg id master.public) &&
+	stg publish -m "Ignored message" &&
+	test_same_tree &&
+	new_public=$(stg id master.public) &&
+	test "$old_public" = "$new_public"
+	'
+
+test_expect_success \
+	'Pop a patch and publish the changes' \
+	'
+	stg pop p3 &&
+	old_public=$(stg id master.public) &&
+	stg publish -m "p3 removed" &&
+	test_same_tree &&
+	new_public=$(stg id master.public) &&
+	test "$(git rev-list $old_public..$new_public | wc -l)" = "1"
+	'
+
+test_done

Fwd: [RFC][StGit PATCH] Add support for merge-friendly branches

[martin f krafft]

[-- Attachment #1.1: Type: text/plain, Size: 15655 bytes --]

[Forwarded to vcs-pkg-discuss]

Hey vcs-pkg people,

This may be of interest, I don't have any time right now to check
this out and don't remember StGit too well either.

I suggest that comments be fed back to the git mailing list, which
has a Cc policy, so just reply to all.

I have put the list and Catalin and Karl on Cc so that they know
that http://vcs-pkg.org has sighted this stuff. ;)

----- Forwarded message from Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org> -----

Date: Thu, 28 May 2009 12:12:42 +0100
From: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
To: git-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Karl Hasselström <kha-bX70CJONkLNBDgjK7y7TUQ@public.gmane.org>
Subject: [RFC][StGit PATCH] Add support for merge-friendly branches
Message-ID: <20090528111212.21925.45527.stgit-hhZApKj8DF/YkXV2EHHjLW3o5bpOHsLO@public.gmane.org>

The main issue with publishing StGit branches is that the Git history
represented by patches is volatile, making it difficult for people
wanting to merge such branch. One solution is for all the downstream
developers to always rebase but that's not always desirable. Another
solution is provided by tools like TopGit but the visible Git history
becomes complicated, especially with repeated reordering.

The patch proposes a new StGit command called "publish". This command
allows one to develop patches normally on a StGit branch but publish the
stack changes to a separate, merge-friendly branch whose history is not
re-writable.

More about its behaviour can be found in the command description in this
patch.

Signed-off-by: Catalin Marinas <catalin.marinas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 stgit/commands/common.py  |   26 ++++++++
 stgit/commands/new.py     |   22 -------
 stgit/commands/publish.py |  139 +++++++++++++++++++++++++++++++++++++++++++++
 stgit/lib/git.py          |    5 ++
 t/t4100-publish.sh        |  129 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 300 insertions(+), 21 deletions(-)
 create mode 100644 stgit/commands/publish.py
 create mode 100755 t/t4100-publish.sh

diff --git a/stgit/commands/common.py b/stgit/commands/common.py
index e46412e..04314f3 100644
--- a/stgit/commands/common.py
+++ b/stgit/commands/common.py
@@ -447,6 +447,32 @@ def readonly_constant_property(f):
         return getattr(self, n)
     return property(new_f)
 
+def update_commit_data(cd, options, allow_edit = False):
+    """Return a new CommitData object updated according to the command line
+    options."""
+    # Set the commit message from commandline.
+    if options.message != None:
+        cd = cd.set_message(options.message)
+
+    # Modify author data.
+    cd = cd.set_author(options.author(cd.author))
+
+    # Add Signed-off-by: or similar.
+    if options.sign_str != None:
+        sign_str = options.sign_str
+    else:
+        sign_str = config.get("stgit.autosign")
+    if sign_str != None:
+        cd = cd.set_message(
+            add_sign_line(cd.message, sign_str,
+                          cd.committer.name, cd.committer.email))
+
+    # Let user edit the commit message manually.
+    if allow_edit and not options.message:
+        cd = cd.set_message(edit_string(cd.message, '.stgit-new.txt'))
+
+    return cd
+
 class DirectoryException(StgException):
     pass
 
diff --git a/stgit/commands/new.py b/stgit/commands/new.py
index 2c98431..9fd51c3 100644
--- a/stgit/commands/new.py
+++ b/stgit/commands/new.py
@@ -67,32 +67,12 @@ def func(parser, options, args):
     cd = gitlib.CommitData(
         tree = stack.head.data.tree, parents = [stack.head], message = '',
         author = gitlib.Person.author(), committer = gitlib.Person.committer())
-
-    # Set patch commit message from commandline.
-    if options.message != None:
-        cd = cd.set_message(options.message)
-
-    # Modify author data.
-    cd = cd.set_author(options.author(cd.author))
-
-    # Add Signed-off-by: or similar.
-    if options.sign_str != None:
-        sign_str = options.sign_str
-    else:
-        sign_str = config.get("stgit.autosign")
-
-    if sign_str != None:
-        cd = cd.set_message(
-            utils.add_sign_line(cd.message, sign_str,
-                                cd.committer.name, cd.committer.email))
+    cd = common.update_commit_data(cd, options, allow_edit = True)
 
     if options.save_template:
         options.save_template(cd.message)
         return utils.STGIT_SUCCESS
 
-    # Let user edit the commit message manually.
-    if not options.message:
-        cd = cd.set_message(utils.edit_string(cd.message, '.stgit-new.txt'))
     if name == None:
         name = utils.make_patch_name(cd.message,
                                      lambda name: stack.patches.exists(name))
diff --git a/stgit/commands/publish.py b/stgit/commands/publish.py
new file mode 100644
index 0000000..06c32d0
--- /dev/null
+++ b/stgit/commands/publish.py
@@ -0,0 +1,139 @@
+__copyright__ = """
+Copyright (C) 2009, Catalin Marinas <catalin.marinas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License version 2 as
+published by the Free Software Foundation.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+"""
+
+from stgit import argparse
+from stgit.argparse import opt
+from stgit.commands import common
+from stgit.config import config
+from stgit.lib import git, stack
+from stgit.out import out
+
+help = 'Publish the stack changes to a merge-friendly head'
+kind = 'stack'
+usage = ['[options] [branch]']
+description = """
+This command commits a set of changes on a separate (called public) branch
+based on the modifications of the given or current stack. The history of the
+public branch is not re-written, making it merge-friendly and feasible for
+publishing. The heads of the stack and public branch may be different but the
+corresponding tree objects are always the same.
+
+If the trees of the stack and public branch are different (otherwise the
+command has no effect), StGit first checks for a rebase of the stack since the
+last publishing. If a rebase is detected, StGit creates a commit on the public
+branch corresponding to a merge between the new stack base and the latest
+public head.
+
+If no rebasing was detected, StGit checks for new patches that may have been
+created on top of the stack since the last publishing. If new patches are
+found and are not empty, they are checked into the public branch keeping the
+same commit information (e.g. log message, author, committer, date).
+
+If the above tests fail (e.g. patches modified or removed), StGit creates a
+new commit on the public branch having the same tree as the stack but the
+public head as its parent. The editor will be invoked if no "--message" option
+is given.
+
+It is recommended that stack modifications falling in different categories as
+described above are separated by a publish command in order to keep the public
+branch history cleaner (otherwise StGit would generate a big commit including
+several stack modifications).
+
+The public branch name can be set via the branch.<branch>.public configuration
+variable (defaulting to "<branch>.public").
+"""
+
+args = [argparse.all_branches]
+options = [
+    opt('-b', '--branch', args = [argparse.stg_branches],
+        short = 'Use BRANCH instead of the default branch')
+] + (argparse.author_options()
+     + argparse.message_options(save_template = False)
+     + argparse.sign_options())
+
+directory = common.DirectoryHasRepositoryLib()
+
+def __create_commit(repository, tree, parents, options):
+    """Return a new Commit object."""
+    cd = git.CommitData(
+        tree = tree, parents = parents, message = '',
+        author = git.Person.author(), committer = git.Person.committer())
+    cd = common.update_commit_data(cd, options, allow_edit = True)
+
+    return repository.commit(cd)
+
+def func(parser, options, args):
+    """Publish the stack changes."""
+    repository = directory.repository
+    stack = repository.get_stack(options.branch)
+
+    if not args:
+        public_ref = config.get('branch.%s.public' % stack.name)
+        if not public_ref:
+            public_ref = 'refs/heads/%s.public' % stack.name
+    elif len(args) == 1:
+        public_ref = args[0]
+    else:
+        parser.error('incorrect number of arguments')
+
+    # just clone the stack if the public ref does not exist
+    if not repository.refs.exists(public_ref):
+        repository.refs.set(public_ref, stack.head, 'publish')
+        out.info('Created "%s"' % public_ref)
+        return
+
+    public_head = repository.refs.get(public_ref)
+    public_tree = public_head.data.tree
+
+    # check for same tree (already up to date)
+    if public_tree.sha1 == stack.head.data.tree.sha1:
+        out.info('"%s" already up to date' % public_ref)
+        return
+
+    # check for rebased stack. In this case we emulate a merge with the stack
+    # base by setting two parents.
+    merge_base = repository.get_merge_base(public_head, stack.base)
+    if merge_base.sha1 != stack.base.sha1:
+        public_head = __create_commit(repository, stack.head.data.tree,
+                                      [public_head, stack.base], options)
+        repository.refs.set(public_ref, public_head, 'publish')
+        out.info('Merged the stack base into "%s"' % public_ref)
+        return
+
+    # check for new patches from the last publishing. This is done by checking
+    # whether the public tree is the same as the bottom of the checked patch.
+    # If older patches were modified, new patches cannot be detected. The new
+    # patches and their metadata are pushed directly to the published head.
+    for p in stack.patchorder.applied:
+        pc = stack.patches.get(p).commit
+        if public_tree.sha1 == pc.data.parent.data.tree.sha1:
+            if pc.data.is_nochange():
+                out.info('Ignored new empty patch "%s"' % p)
+                continue
+            cd = pc.data.set_parent(public_head)
+            public_head = repository.commit(cd)
+            public_tree = public_head.data.tree
+            out.start('Published new patch "%s"' % p)
+
+    # create a new commit (only happens if no new patches are detected)
+    if public_tree.sha1 != stack.head.data.tree.sha1:
+        public_head = __create_commit(repository, stack.head.data.tree,
+                                      [public_head], options)
+
+    # update the public head
+    repository.refs.set(public_ref, public_head, 'publish')
+    out.info('Updated "%s"' % public_ref)
diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 6f2c977..4a17c8a 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -609,6 +609,11 @@ class Repository(RunWithEnv):
             raise DetachedHeadException()
     def set_head_ref(self, ref, msg):
         self.run(['git', 'symbolic-ref', '-m', msg, 'HEAD', ref]).no_output()
+    def get_merge_base(self, commit1, commit2):
+        """Return the merge base of two commits."""
+        sha1 = self.run(['git', 'merge-base',
+                         commit1.sha1, commit2.sha1]).output_one_line()
+        return self.get_commit(sha1)
     def simple_merge(self, base, ours, theirs):
         index = self.temp_index()
         try:
diff --git a/t/t4100-publish.sh b/t/t4100-publish.sh
new file mode 100755
index 0000000..17e07bc
--- /dev/null
+++ b/t/t4100-publish.sh
@@ -0,0 +1,129 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Catalin Marinas
+#
+
+test_description='Exercise the publish command.
+
+Create/modify patches on the stack and publish them to a separate branch.'
+
+. ./test-lib.sh
+
+test_same_tree () {
+	stack_tree=$(git rev-parse master^{tree})
+	public_tree=$(git rev-parse master.public^{tree})
+	test "$stack_tree" = "$public_tree"
+}
+
+test_expect_success \
+	'Initialize the StGit repository' \
+	'
+	stg init
+	'
+
+test_expect_success \
+	'Create some patches' \
+	'
+	stg new p1 -m p1 &&
+	echo foo1 > foo1.txt &&
+	git add foo1.txt &&
+	stg refresh &&
+	stg new p2 -m p2 &&
+	echo foo2 > foo2.txt &&
+	git add foo2.txt &&
+	stg refresh &&
+	stg new p3 -m p3 &&
+	echo foo3 > foo3.txt &&
+	git add foo3.txt &&
+	stg refresh
+	'
+
+test_expect_success \
+	'Publish the stack for the first time' \
+	'
+	stg publish &&
+	test "$(stg id)" = "$(stg id master.public)"
+	'
+
+test_expect_success \
+	'Modify a patch and publish the changes' \
+	'
+	stg pop &&
+	echo foo2 >> foo2.txt &&
+	stg refresh &&
+	stg push &&
+	old_public=$(stg id master.public) &&
+	stg publish -m "p2 updated" &&
+	test_same_tree &&
+	new_public=$(stg id master.public) &&
+	test "$(git rev-list $old_public..$new_public | wc -l)" = "1"
+	'
+
+test_expect_success \
+	'Create new patches and publish them' \
+	'
+	stg new p4 -m p4 &&
+	echo foo4 > foo4.txt &&
+	git add foo4.txt &&
+	stg refresh &&
+	stg new p5 -m p5 &&
+	echo foo5 > foo5.txt &&
+	git add foo5.txt &&
+	stg refresh &&
+	stg new empty -m empty &&
+	old_public=$(stg id master.public) &&
+	stg publish -m "Ignored message" &&
+	test_same_tree &&
+	new_public=$(stg id master.public) &&
+	test "$(git rev-list $old_public..$new_public | wc -l)" = "2"
+	'
+
+test_expect_success \
+	'Rebase the current stack and publish a merge' \
+	'
+	stg pop -a &&
+	echo foo0 > foo0.txt &&
+	git add foo0.txt &&
+	git commit -m "foo0.txt added" &&
+	stg push -a &&
+	old_public=$(stg id master.public) &&
+	stg publish -m "Merge with base" &&
+	test_same_tree &&
+	new_public=$(stg id master.public) &&
+	test "$(git rev-list $old_public..$new_public | wc -l)" = "2" &&
+	test "$(git merge-base master.public master)" = "$(stg id {base})"
+	'
+
+test_expect_success \
+	'Re-publish without any changes' \
+	'
+	old_public=$(stg id master.public) &&
+	stg publish -m "Ignored message" &&
+	test_same_tree &&
+	new_public=$(stg id master.public) &&
+	test "$old_public" = "$new_public"
+	'
+
+test_expect_success \
+	'Reorder patches and publish the changes' \
+	'
+	stg float p5 p4 p3 p2 p1 &&
+	old_public=$(stg id master.public) &&
+	stg publish -m "Ignored message" &&
+	test_same_tree &&
+	new_public=$(stg id master.public) &&
+	test "$old_public" = "$new_public"
+	'
+
+test_expect_success \
+	'Pop a patch and publish the changes' \
+	'
+	stg pop p3 &&
+	old_public=$(stg id master.public) &&
+	stg publish -m "p3 removed" &&
+	test_same_tree &&
+	new_public=$(stg id master.public) &&
+	test "$(git rev-list $old_public..$new_public | wc -l)" = "1"
+	'
+
+test_done

-- 
 .''`.   martin f. krafft <madduck@d.o>      Related projects:
: :'  :  proud Debian developer               http://debiansystem.info
`. `'`   http://people.debian.org/~madduck    http://vcs-pkg.org
  `-  Debian - when you have better things to do than fixing systems
 
a farmer is a man outstanding in his field.

[-- Attachment #1.2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 207 bytes --]

_______________________________________________
vcs-pkg-discuss mailing list
vcs-pkg-discuss-XbBxUvOt3X2LieD7tvxI8l/i77bcL1HB@public.gmane.org
http://lists.alioth.debian.org/mailman/listinfo/vcs-pkg-discuss

Re: [RFC][StGit PATCH] Add support for merge-friendly branches

[Karl Hasselström]

On 2009-05-28 12:12:42 +0100, Catalin Marinas wrote:

> The patch proposes a new StGit command called "publish". This
> command allows one to develop patches normally on a StGit branch but
> publish the stack changes to a separate, merge-friendly branch whose
> history is not re-writable.

Hmm, interesting. I don't think I'd want to use a command like this
myself, but I can see how it might be useful.

> +    # check for same tree (already up to date)
> +    if public_tree.sha1 == stack.head.data.tree.sha1:
> +        out.info('"%s" already up to date' % public_ref)
> +        return
> +
> +    # check for rebased stack. In this case we emulate a merge with the stack
> +    # base by setting two parents.
> +    merge_base = repository.get_merge_base(public_head, stack.base)
> +    if merge_base.sha1 != stack.base.sha1:
> +        public_head = __create_commit(repository, stack.head.data.tree,
> +                                      [public_head, stack.base], options)
> +        repository.refs.set(public_ref, public_head, 'publish')
> +        out.info('Merged the stack base into "%s"' % public_ref)
> +        return

Hmm. Couldn't the merge base conceivably be higher up in the stack?
Like, right at the beginning, don't we have public_head == stack.head?
That would be caught by the "same tree" check" a bit earlier, but
after adding another patch, don't we have public_head == stack.head^ ?
Which would give merge_base == public_head.

> +    def get_merge_base(self, commit1, commit2):
> +        """Return the merge base of two commits."""
> +        sha1 = self.run(['git', 'merge-base',
> +                         commit1.sha1, commit2.sha1]).output_one_line()
> +        return self.get_commit(sha1)

This funcion should probably return a list of zero or more merge
bases. See the --all flag to git merge-base.

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

Re: [RFC][StGit PATCH] Add support for merge-friendly branches

[Catalin Marinas]

2009/5/28 Karl Hasselström <kha@treskal.com>:
> On 2009-05-28 12:12:42 +0100, Catalin Marinas wrote:
>
>> The patch proposes a new StGit command called "publish". This
>> command allows one to develop patches normally on a StGit branch but
>> publish the stack changes to a separate, merge-friendly branch whose
>> history is not re-writable.
>
> Hmm, interesting. I don't think I'd want to use a command like this
> myself, but I can see how it might be useful.

For me it is useful. I publish a kernel tree with over 100 patches.
Later I find that one patch is buggy. The current merge-friendly
solution is to add another patch but I may want to just update the
buggy patch as it's easier when time comes to submit upstream. This,
however, rewrites the history. So with the "publish" command I just
generate another commit on top of the public branch and I always end
up with the same tree as on my stack.

>> +    # check for same tree (already up to date)
>> +    if public_tree.sha1 == stack.head.data.tree.sha1:
>> +        out.info('"%s" already up to date' % public_ref)
>> +        return
>> +
>> +    # check for rebased stack. In this case we emulate a merge with the stack
>> +    # base by setting two parents.
>> +    merge_base = repository.get_merge_base(public_head, stack.base)
>> +    if merge_base.sha1 != stack.base.sha1:
>> +        public_head = __create_commit(repository, stack.head.data.tree,
>> +                                      [public_head, stack.base], options)
>> +        repository.refs.set(public_ref, public_head, 'publish')
>> +        out.info('Merged the stack base into "%s"' % public_ref)
>> +        return
>
> Hmm. Couldn't the merge base conceivably be higher up in the stack?
> Like, right at the beginning, don't we have public_head == stack.head?
> That would be caught by the "same tree" check" a bit earlier, but
> after adding another patch, don't we have public_head == stack.head^ ?
> Which would give merge_base == public_head.

We could have public_head == stack.head^... but that's not an issue.
The merge_base above is checked against the base of the stack rather
than the top as we assume that the base isn't volatile. So even if
public_head is the same as some patch commit, the merge_base above
would always be the base of the stack. Only if the stack base was
updated, we get a different merge_base (equal to the previous stack
base).

>> +    def get_merge_base(self, commit1, commit2):
>> +        """Return the merge base of two commits."""
>> +        sha1 = self.run(['git', 'merge-base',
>> +                         commit1.sha1, commit2.sha1]).output_one_line()
>> +        return self.get_commit(sha1)
>
> This funcion should probably return a list of zero or more merge
> bases. See the --all flag to git merge-base.

OK, I'll add this and check the stack base against this set(list).

-- 
Catalin

Re: [RFC][StGit PATCH] Add support for merge-friendly branches

[Catalin Marinas]

2009/5/28 Catalin Marinas <catalin.marinas@gmail.com>:
> 2009/5/28 Karl Hasselström <kha@treskal.com>:
>> On 2009-05-28 12:12:42 +0100, Catalin Marinas wrote:
>>> +    def get_merge_base(self, commit1, commit2):
>>> +        """Return the merge base of two commits."""
>>> +        sha1 = self.run(['git', 'merge-base',
>>> +                         commit1.sha1, commit2.sha1]).output_one_line()
>>> +        return self.get_commit(sha1)
>>
>> This funcion should probably return a list of zero or more merge
>> bases. See the --all flag to git merge-base.
>
> OK, I'll add this and check the stack base against this set(list).

What about this change to the original patch (it's faster to just
return the sha1 refs than building the Commit objects):


diff --git a/stgit/commands/publish.py b/stgit/commands/publish.py
index 06c32d0..ce08a19 100644
--- a/stgit/commands/publish.py
+++ b/stgit/commands/publish.py
@@ -106,8 +106,8 @@ def func(parser, options, args):

     # check for rebased stack. In this case we emulate a merge with the stack
     # base by setting two parents.
-    merge_base = repository.get_merge_base(public_head, stack.base)
-    if merge_base.sha1 != stack.base.sha1:
+    merge_base = set(repository.get_merge_base_sha1(public_head, stack.base))
+    if not stack.base.sha1 in merge_base:
         public_head = __create_commit(repository, stack.head.data.tree,
                                       [public_head, stack.base], options)
         repository.refs.set(public_ref, public_head, 'publish')
diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 4a17c8a..5bd4e4d 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -609,11 +609,10 @@ class Repository(RunWithEnv):
             raise DetachedHeadException()
     def set_head_ref(self, ref, msg):
         self.run(['git', 'symbolic-ref', '-m', msg, 'HEAD', ref]).no_output()
-    def get_merge_base(self, commit1, commit2):
-        """Return the merge base of two commits."""
-        sha1 = self.run(['git', 'merge-base',
-                         commit1.sha1, commit2.sha1]).output_one_line()
-        return self.get_commit(sha1)
+    def get_merge_base_sha1(self, commit1, commit2):
+        """Return the merge base of two commits as a list of sha1 refs."""
+        return self.run(['git', 'merge-base', '--all',
+                         commit1.sha1, commit2.sha1]).output_lines()
     def simple_merge(self, base, ours, theirs):
         index = self.temp_index()
         try:

-- 
Catalin

Re: [RFC][StGit PATCH] Add support for merge-friendly branches

[Karl Hasselström]

On 2009-05-28 15:51:20 +0100, Catalin Marinas wrote:

> 2009/5/28 Catalin Marinas <catalin.marinas@gmail.com>:
>
> > 2009/5/28 Karl Hasselström <kha@treskal.com>:
> >
> > > This funcion should probably return a list of zero or more merge
> > > bases. See the --all flag to git merge-base.
> >
> > OK, I'll add this and check the stack base against this set(list).
>
> What about this change to the original patch (it's faster to just
> return the sha1 refs than building the Commit objects):

Creating Commit objects is really cheap---just look at the
constructor. I made them that way on purpose, so that we'd never have
to think twice about using Commit objects instead of passing sha1s
around.

Also, you said "set", and I agree---the return value of
get_mege_bases() should be a set. That's what it _is_, conceptually,
and it makes little sense to obscure that fact.

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

Re: [RFC][StGit PATCH] Add support for merge-friendly branches

[Karl Hasselström]

On 2009-05-28 15:38:44 +0100, Catalin Marinas wrote:

> 2009/5/28 Karl Hasselström <kha@treskal.com>:
>
> > On 2009-05-28 12:12:42 +0100, Catalin Marinas wrote:
> >
> > > The patch proposes a new StGit command called "publish". This
> > > command allows one to develop patches normally on a StGit branch
> > > but publish the stack changes to a separate, merge-friendly
> > > branch whose history is not re-writable.
> >
> > Hmm, interesting. I don't think I'd want to use a command like
> > this myself, but I can see how it might be useful.
>
> For me it is useful. I publish a kernel tree with over 100 patches.
> Later I find that one patch is buggy. The current merge-friendly
> solution is to add another patch but I may want to just update the
> buggy patch as it's easier when time comes to submit upstream. This,
> however, rewrites the history. So with the "publish" command I just
> generate another commit on top of the public branch and I always end
> up with the same tree as on my stack.

I think I would've kludged this by making --theirs merges from the
StGit branch to the public branch. But "stg publish" should definitely
make the kludge history less ugly.

> > Hmm. Couldn't the merge base conceivably be higher up in the
> > stack? Like, right at the beginning, don't we have public_head ==
> > stack.head? That would be caught by the "same tree" check" a bit
> > earlier, but after adding another patch, don't we have public_head
> > == stack.head^ ? Which would give merge_base == public_head.
>
> We could have public_head == stack.head^... but that's not an issue.
> The merge_base above is checked against the base of the stack rather
> than the top as we assume that the base isn't volatile. So even if
> public_head is the same as some patch commit, the merge_base above
> would always be the base of the stack. Only if the stack base was
> updated, we get a different merge_base (equal to the previous stack
> base).

The situation I described looks like this:

    B--o--o--o--o--o--P--T

Time goes from left to right. B is the stack base, P the head of the
public branch, T the stack top. merge_base(P, T) is P, and not B.

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

Re: [RFC][StGit PATCH] Add support for merge-friendly branches

[Catalin Marinas]

2009/5/29 Karl Hasselström <kha@treskal.com>:
> On 2009-05-28 15:51:20 +0100, Catalin Marinas wrote:
>
>> 2009/5/28 Catalin Marinas <catalin.marinas@gmail.com>:
>>
>> > 2009/5/28 Karl Hasselström <kha@treskal.com>:
>> >
>> > > This funcion should probably return a list of zero or more merge
>> > > bases. See the --all flag to git merge-base.
>> >
>> > OK, I'll add this and check the stack base against this set(list).
>>
>> What about this change to the original patch (it's faster to just
>> return the sha1 refs than building the Commit objects):
>
> Creating Commit objects is really cheap---just look at the
> constructor. I made them that way on purpose, so that we'd never have
> to think twice about using Commit objects instead of passing sha1s
> around.

I was worried it may invoke git to get the CommitData.

> Also, you said "set", and I agree---the return value of
> get_mege_bases() should be a set. That's what it _is_, conceptually,
> and it makes little sense to obscure that fact.

If we return a set of commits, I suspect the Repository object
guarantees that having the same sha1 value always returns the same
Commit object and the code below is valid:

    merge_bases = repository.get_merge_bases(public_head, stack.base)
    if not stack.base in merge_bases:
        public_head = __create_commit(repository, stack.head.data.tree,
                                      [public_head, stack.base], options)
        repository.refs.set(public_ref, public_head, 'publish')
        out.info('Merged the stack base into "%s"' % public_ref)
        return

...

    def get_merge_bases(self, commit1, commit2):
        """Return a set of merge bases of two commits."""
        sha1_list = self.run(['git', 'merge-base', '--all',
                              commit1.sha1, commit2.sha1]).output_lines()
        return set(self.get_commit(sha1) for sha1 in sha1_list)

-- 
Catalin

Re: [RFC][StGit PATCH] Add support for merge-friendly branches

[Karl Hasselström]

On 2009-05-29 09:40:59 +0100, Catalin Marinas wrote:

> 2009/5/29 Karl Hasselström <kha@treskal.com>:
>
> > On 2009-05-28 15:51:20 +0100, Catalin Marinas wrote:
> >
> > > 2009/5/28 Catalin Marinas <catalin.marinas@gmail.com>:
> > >
> > > What about this change to the original patch (it's faster to
> > > just return the sha1 refs than building the Commit objects):
> >
> > Creating Commit objects is really cheap---just look at the
> > constructor. I made them that way on purpose, so that we'd never
> > have to think twice about using Commit objects instead of passing
> > sha1s around.
>
> I was worried it may invoke git to get the CommitData.

Only if you try to access it---it's evaluated lazily.

> > Also, you said "set", and I agree---the return value of
> > get_mege_bases() should be a set. That's what it _is_,
> > conceptually, and it makes little sense to obscure that fact.
>
> If we return a set of commits, I suspect the Repository object
> guarantees that having the same sha1 value always returns the same
> Commit object

Yes, exactly. As long as you get all your Commit objects from the
Repository object like you're supposed to, there'll be at most one
Commit object for each sha1. In fact, the Commit objects don't have an
__eq__ method on purpose---straight object comparison already does
exactly what we want!

> and the code below is valid:

Yes, this is precisely what I meant.

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

Re: [RFC][StGit PATCH] Add support for merge-friendly branches

[Catalin Marinas]

2009/5/29 Karl Hasselström <kha@treskal.com>:
> On 2009-05-28 15:38:44 +0100, Catalin Marinas wrote:
> I think I would've kludged this by making --theirs merges from the
> StGit branch to the public branch. But "stg publish" should definitely
> make the kludge history less ugly.

That's what I'm trying to do, keep the public history clean. One
advantage of merging the full StGit branch is that people could
retrieve the latest patch version but for those interesting in
cherry-picking you can just export the volatile StGit branch.

Regarding the resulting tree, rebasing a StGit stack is equivalent, on
a linear history branch, to a merge of the new stack base into the
linear branch. Rather than having to solve conflicts twice, the pubish
command just fakes this merge and sets the resulting tree.

>> > Hmm. Couldn't the merge base conceivably be higher up in the
>> > stack? Like, right at the beginning, don't we have public_head ==
>> > stack.head? That would be caught by the "same tree" check" a bit
>> > earlier, but after adding another patch, don't we have public_head
>> > == stack.head^ ? Which would give merge_base == public_head.
>>
>> We could have public_head == stack.head^... but that's not an issue.
>> The merge_base above is checked against the base of the stack rather
>> than the top as we assume that the base isn't volatile. So even if
>> public_head is the same as some patch commit, the merge_base above
>> would always be the base of the stack. Only if the stack base was
>> updated, we get a different merge_base (equal to the previous stack
>> base).
>
> The situation I described looks like this:
>
>    B--o--o--o--o--o--P--T
>
> Time goes from left to right. B is the stack base, P the head of the
> public branch, T the stack top. merge_base(P, T) is P, and not B.

I don't check merge_base(P, T) but merge_base(P, B) to avoid the
issues you described. So that's always B.

-- 
Catalin

Re: [RFC][StGit PATCH] Add support for merge-friendly branches

[Catalin Marinas]

2009/5/29 Catalin Marinas <catalin.marinas@gmail.com>:
>    merge_bases = repository.get_merge_bases(public_head, stack.base)
>    if not stack.base in merge_bases:
>        public_head = __create_commit(repository, stack.head.data.tree,
>                                      [public_head, stack.base], options)
>        repository.refs.set(public_ref, public_head, 'publish')
>        out.info('Merged the stack base into "%s"' % public_ref)
>        return

One minor improvement here, trying to guess what was merged:

diff --git a/stgit/commands/publish.py b/stgit/commands/publish.py
index fe7f7c6..401fbdf 100644
--- a/stgit/commands/publish.py
+++ b/stgit/commands/publish.py
@@ -67,10 +67,10 @@ options = [

 directory = common.DirectoryHasRepositoryLib()

-def __create_commit(repository, tree, parents, options):
+def __create_commit(repository, tree, parents, options, message = ''):
     """Return a new Commit object."""
     cd = git.CommitData(
-        tree = tree, parents = parents, message = '',
+        tree = tree, parents = parents, message = message,
         author = git.Person.author(), committer = git.Person.committer())
     cd = common.update_commit_data(cd, options, allow_edit = True)

@@ -106,8 +106,10 @@ def func(parser, options, args):
     # base by setting two parents.
     merge_bases = repository.get_merge_bases(public_head, stack.base)
     if not stack.base in merge_bases:
+        message = 'Merge ' + repository.describe(stack.base)
         public_head = __create_commit(repository, stack.head.data.tree,
-                                      [public_head, stack.base], options)
+                                      [public_head, stack.base], options,
+                                      message)
         repository.refs.set(public_ref, public_head, 'publish')
         out.info('Merged the stack base into "%s"' % public_ref)
         return
diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index 3303eea..6e3bb4f 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -614,6 +614,10 @@ class Repository(RunWithEnv):
         sha1_list = self.run(['git', 'merge-base', '--all',
                               commit1.sha1, commit2.sha1]).output_lines()
         return set(self.get_commit(sha1) for sha1 in sha1_list)
+    def describe(self, commit):
+        """Use git describe --all on the given commit."""
+        return self.run(['git', 'describe', '--all', commit.sha1]
+                       ).discard_stderr().discard_exitcode().raw_output()
     def simple_merge(self, base, ours, theirs):
         index = self.temp_index()
         try:

-- 
Catalin

Re: [RFC][StGit PATCH] Add support for merge-friendly branches

[Karl Hasselström]

On 2009-05-29 10:16:52 +0100, Catalin Marinas wrote:

> 2009/5/29 Karl Hasselström <kha@treskal.com>:
>
> > The situation I described looks like this:
> >
> >    B--o--o--o--o--o--P--T
> >
> > Time goes from left to right. B is the stack base, P the head of
> > the public branch, T the stack top. merge_base(P, T) is P, and not
> > B.
>
> I don't check merge_base(P, T) but merge_base(P, B) to avoid the
> issues you described. So that's always B.

Ah, so that's where I got myself confused. Thanks.

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