No subject

computation or a bug in findmissing() (ie. how does common look like
when calling _changegroup() for the fast-path).
> 
> The change to test-notify.out is correct, because it actually hits this
> bug, as can be seen by glog'ing the two repos:
> 
> @    22c88
> |\
> | o  0a184
> | |
> o |  0647d
> |/
> o  cb9a9
> 
> and
> 
> o  0647d
> |
> @  cb9a9
> 
> It used to pull 0647d again, which is unnecessary.

Indeed, so the question is: was common composed of 0647d, or cb9a9 or
both?

According to the docstring in findmissing() if common includes 0647d,
then findmissing() should return only [0a184, 22c88].

I'd actually like to have our internals based on "common" nodes instead
of "bases" nodes since it's a lot easier to reason about them (that's
why I introduced findmissing()). Ideally a new version of the discovery
algorithm should compute the "common", and send it to a new
remote changegroupcommon().

> 
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -1622,19 +1623,22 @@
>          the linkrev.
>          """
>  
> +        # Set up some initial variables
> +        # Make it easy to refer to self.changelog
> +        cl = self.changelog
> +        # msng is short for missing - compute the list of changesets in this
> +        # changegroup.

> +        if not bases:
> +            bases = [nullid]

not needed

> +        msng_cl_lst, bases, heads = cl.nodesbetween(bases, heads)
> +
>          if extranodes is None:
>              # can we go through the fast path ?
>              heads.sort()
>              allheads = self.heads()
>              allheads.sort()
>              if heads == allheads:
> -                common = []
> -                # parents of bases are known from both sides
> -                for n in bases:
> -                    for p in self.changelog.parents(n):
> -                        if p != nullid:
> -                            common.append(p)
> -                return self._changegroup(common, source)
> +                return self._changegroup(msng_cl_lst, source)
>  
>          self.hook('preoutgoing', throw=True, source=source)
> 

Can't you remove the nodesbetween() call that's right after this, since
you already computed it?


thanks,

Benoit
-- 
:wq