[PATCH 2 of 2] discovery: properly filter changeset in 'peer.known' (issue4982)

[Gregory Szorc]

On Sun, Dec 6, 2015 at 12:54 PM, Pierre-Yves David <
pierre-yves.david at ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at fb.com>
> # Date 1449101535 28800
> #      Wed Dec 02 16:12:15 2015 -0800
> # Node ID cce8d1dfcefa5495af7900ea5cff796691ca6d07
> # Parent  179e1f468d683a1b9e7e8df76af0690b454c4025
> # EXP-Topic fixknown
> # Available At http://hg.netv6.net/marmoute-wip/mercurial/
> #              hg pull http://hg.netv6.net/marmoute-wip/mercurial/ -r
> cce8d1dfcefa
> discovery: properly filter changeset in 'peer.known' (issue4982)
>

This should be marked BC because it changes semantics of the "known" wire
protocol command. (The new semantics make sense to me modulo the bug
described below.)


>
> The 'peer.known' call (handled at the repository level) was applying its
> own
> manual filtering (looking at phases) instead of relying on the repoview
> mechanism. This led to the discovery finding more "common" node that
> 'getbundle' was willing to recognised. From there, bad things happen,
> issue4982
> is a symptom of it. While situations like described in issue4982 can still
> happen because of race conditions, fixing 'peer.known' is important for
> consistency in all cases.
>
> We update the code to use 'repoview' filtering. This lead to small changes
> in
> the tests for exchanging obsolescence marker because the discovery yields
> different results.
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -806,16 +806,17 @@ class localrepository(object):
>
>          repo = (remote and remote.local()) and remote or self
>          return repo[key].branch()
>
>      def known(self, nodes):
> -        nm = self.changelog.nodemap
> -        pc = self._phasecache
> +        cl = self.filtered('served').changelog
>

hgweb supports specifying the filter to serve with. Therefore, I believe
manually specifying a filter here is wrong, as a server started with `hg
--hidden serve` won't expose hidden changesets even though you told it to.
(This also likely points to a gap in test coverage.)

Why can't you simply use self.changelog? wireproto.py should be operating
on a repo instance with the appropriate filter (if any) already in place.


> +        nm = cl.nodemap
> +        filtered = cl.filteredrevs
>          result = []
>          for n in nodes:
>              r = nm.get(n)
> -            resp = not (r is None or pc.phase(self, r) >= phases.secret)
> +            resp = not (r is None or r in filtered)
>              result.append(resp)
>          return result
>
>      def local(self):
>          return self
> diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
> --- a/tests/test-obsolete.t
> +++ b/tests/test-obsolete.t
> @@ -743,12 +743,11 @@ This test issue 3805
>    searching for changes
>    3:323a9c3ddd91 (draft) [tip ] A
>    $ hg outgoing
>    comparing with $TESTTMP/tmpe/repo-issue3805 (glob)
>    searching for changes
> -  no changes found
> -  [1]
> +  1:29f0c6921ddd (draft) [tip ] A
>
>  #if serve
>
>    $ hg serve -R ../repo-issue3805 -n test -p $HGPORT -d --pid-file=hg.pid
> -A access.log -E errors.log
>    $ cat hg.pid >> $DAEMON_PIDS
> @@ -758,12 +757,11 @@ This test issue 3805
>    searching for changes
>    2:323a9c3ddd91 (draft) [tip ] A
>    $ hg outgoing http://localhost:$HGPORT
>    comparing with http://localhost:$HGPORT/
>    searching for changes
> -  no changes found
> -  [1]
> +  1:29f0c6921ddd (draft) [tip ] A
>
>    $ killdaemons.py
>
>  #endif
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20151206/02709e7a/attachment.html>