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

From: Catalin Marinas <catalin.marinas@gmail.com>
Date: May 29, 2009, 08:40 UTC
Message-ID: <b0943d9e0905290140i57aeaa33r8de34302b63ffa5e@mail.gmail.com>
In-Reply-To: <20090529072050.GA9760@diana.vm.bytemark.co.uk>

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

Show 18 quoted lines

> 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