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

From: Catalin Marinas <catalin.marinas@gmail.com>
Date: May 28, 2009, 14:38 UTC
Message-ID: <b0943d9e0905280738n51476ab7vd0498ea7a236c4a7@mail.gmail.com>
In-Reply-To: <20090528124817.GA22262@diana.vm.bytemark.co.uk>

2009/5/28 Karl Hasselström <kha@treskal.com>:

Show 9 quoted lines

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

Show 20 quoted lines

>> +    # 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).

Show 8 quoted lines

>> +    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