[MERGE] Let find_previous_heads take a repository.

[Johan Rydberg]

Aaron Bentley <aaron.bentley at utoronto.ca> writes:

>> Robert Collins <robertc at robertcollins.net> writes:
>> Robert and I discussed this a bit on IRC.  Here's a new patch to shoot
>> down.  We both dislike the fact that find_previous_heads use either a
>> repository, or a passed repository_file, but because of how the
>> upgrade code works it is a must, I'm afraid.
>
> It looks as though VersionedFile is meant to be a type of
> RepositoryFile.  Shouldn't it subclass it?

Sounds reasonable.

> I'm not sure about RepositoryFile as part of our API.  It seems to me
> that VersionedFile is a reasonable level of abstraction, and that it can
> describe all our past and present ways of referring to versions of files
> reasonably well.
>
> When RepositoryFile is fleshed out more, how will it differ from
> VersionedFile?

RepositoryFile will be a more slimmed interface than VersionedFile.
It will for example not be possible to insert new history via the
RepositoryFile interface.  History is added using either the fetcher
or the CommitBuilder.  

VersionedFile extends RepositoryFile, so there is no need to alter the
existing VersionedFile implementations (knits and weaves).  

We need a per-file interface for retrieving history information out of
the repository.  This is what RepositoryFile is meant for.

~j

>> === modified file 'bzrlib/versionedfile.py'
>> --- bzrlib/versionedfile.py	
>> +++ bzrlib/versionedfile.py	
>> @@ -449,6 +449,38 @@
>>      def weave_merge(self, plan, a_marker=TextMerge.A_MARKER, 
>>                      b_marker=TextMerge.B_MARKER):
>>          return PlanWeaveMerge(plan, a_marker, b_marker).merge_lines()[0]
>> +
>> +    def find_heads(self, versions):
>> +        """See RepositoryFile.find_heads."""
>> +        # list of revisions, one for each head.
>> +        heads = []
>> +        # revision: ancestor list for each head
>> +        head_ancestors = {}
>> +        # eliminate ancestors amongst the available candidates:
>> +        # heads are those that are not an ancestor of any other candidate
>> +        # - this provides convergence at a per-file level.
>> +        for version_id in versions:
>> +            # may be an ancestor of a known head:
>> +            already_present = 0 != len(
>> +                [head for head in heads 
>> +                 if version_id in head_ancestors[head]])
>
> This is not very clear.  Perhaps:
> [...]

I just move the existing code around.  I'll update it with one of your
suggestions.

>      already_present = bool(len([head for head in heads if version_id in
>                                  head_ancestors[head]]))

My Python skills is somewhat, but shouldn't this work?

      already_present = bool([head for head in heads if version_id in
                                  head_ancestors[head]])



Thanks for your comments,

~j