This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8259

discovery: avoid wrong detection of multiple branch heads (issue6256)
ClosedPublic

Authored by marmoute on Mar 6 2020, 7:47 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG7d5455b988ec: discovery: avoid wrong detection of multiple branch heads (issue6256)
Summary

This fix the code using obsolescence markers to remove "to be obsoleted" heads
during the detection of new head creation from push. The code turned out to not
use the branch information at all. This lead changeset from different branch to
be detected as new head on unrelated branch.

The code fix is actually quite simple. New tests have been added to covers
these cases.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

marmoute created this revision.Mar 6 2020, 7:47 PM
pulkit accepted this revision.Mar 9 2020, 5:18 AM
This revision is now accepted and ready to land.Mar 9 2020, 5:18 AM

Revision Contents
Changeset List

Diff 20625

mercurial/discovery.py

# * We "silently" skip processing on all changeset unknown locally # * We "silently" skip processing on all changeset unknown locally
# #
# * if <nh> is public on the remote, it won't be affected by obsolete # * if <nh> is public on the remote, it won't be affected by obsolete
# marker and a new is created # marker and a new is created
# define various utilities and containers # define various utilities and containers
repo = pushop.repo repo = pushop.repo
unfi = repo.unfiltered() unfi = repo.unfiltered()
tonode = unfi.changelog.node
torev = unfi.changelog.index.get_rev torev = unfi.changelog.index.get_rev
public = phases.public public = phases.public
getphase = unfi._phasecache.phase getphase = unfi._phasecache.phase
ispublic = lambda r: getphase(unfi, r) == public ispublic = lambda r: getphase(unfi, r) == public
ispushed = lambda n: torev(n) in futurecommon ispushed = lambda n: torev(n) in futurecommon
hasoutmarker = functools.partial(pushingmarkerfor, unfi.obsstore, ispushed) hasoutmarker = functools.partial(pushingmarkerfor, unfi.obsstore, ispushed)
successorsmarkers = unfi.obsstore.successors successorsmarkers = unfi.obsstore.successors
newhs = set() # final set of new heads newhs = set() # final set of new heads
# fast path the simple case # fast path the simple case
if len(localcandidate) == 1: if len(localcandidate) == 1:
return unknownheads | set(candidate_newhs), set() return unknownheads | set(candidate_newhs), set()
# actually process branch replacement # actually process branch replacement
while localcandidate: while localcandidate:
nh = localcandidate.pop() nh = localcandidate.pop()
current_branch = unfi[nh].branch()
# run this check early to skip the evaluation of the whole branch # run this check early to skip the evaluation of the whole branch
if torev(nh) in futurecommon or ispublic(torev(nh)): if torev(nh) in futurecommon or ispublic(torev(nh)):
newhs.add(nh) newhs.add(nh)
continue continue
# Get all revs/nodes on the branch exclusive to this head # Get all revs/nodes on the branch exclusive to this head
# (already filtered heads are "ignored")) # (already filtered heads are "ignored"))
branchrevs = unfi.revs( branchrevs = unfi.revs(
b'only(%n, (%ln+%ln))', nh, localcandidate, newhs b'only(%n, (%ln+%ln))', nh, localcandidate, newhs
) )
branchnodes = [tonode(r) for r in branchrevs]
branchnodes = []
for r in branchrevs:
c = unfi[r]
if c.branch() == current_branch:
branchnodes.append(c.node())
# The branch won't be hidden on the remote if # The branch won't be hidden on the remote if
# * any part of it is public, # * any part of it is public,
# * any part of it is considered part of the result by previous logic, # * any part of it is considered part of the result by previous logic,
# * if we have no markers to push to obsolete it. # * if we have no markers to push to obsolete it.
if ( if (
any(ispublic(r) for r in branchrevs) any(ispublic(r) for r in branchrevs)
or any(torev(n) in futurecommon for n in branchnodes) or any(torev(n) in futurecommon for n in branchnodes)

tests/test-push-checkheads-multibranches-E1.t

  • This file was added.
====================================
Testing head checking code: Case E-1
====================================
Mercurial checks for the introduction of new heads on push. Evolution comes
into play to detect if existing branches on the server are being replaced by
some of the new one we push.
This case is part of a series of tests checking this behavior.
Category E: case involving changeset on multiple branch
TestCase 8: moving a branch to another location
.. old-state:
..
.. * 1-changeset on branch default
.. * 1-changeset on branch Z (above default)
..
.. new-state:
..
.. * 1-changeset on branch default
.. * 1-changeset on branch Z (rebased away from A0)
..
.. expected-result:
..
.. * push allowed
..
.. graph-summary:
..
.. B ø⇠◔ B'
.. | |
.. A ◔ |
.. |/
.. ●
$ . $TESTDIR/testlib/push-checkheads-util.sh
Test setup
----------
$ mkdir E1
$ cd E1
$ setuprepos
creating basic server and client repo
updating to branch default
2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ cd client
$ hg branch Z
marked working directory as branch Z
(branches are permanent and global, did you want a bookmark?)
$ mkcommit B0
$ hg push --new-branch
pushing to $TESTTMP/E1/server
searching for changes
adding changesets
adding manifests
adding file changes
added 1 changesets with 1 changes to 1 files
$ hg up 0
0 files updated, 0 files merged, 2 files removed, 0 files unresolved
$ hg branch --force Z
marked working directory as branch Z
$ mkcommit B1
created new head
$ hg debugobsolete `getid "desc(B0)" ` `getid "desc(B1)"`
1 new obsolescence markers
obsoleted 1 changesets
$ hg log -G --hidden
@ c98b855401e7 (draft): B1
|
| x 93e5c1321ece (draft): B0
| |
| o 8aaa48160adc (draft): A0
|/
o 1e4be0697311 (public): root
Actual testing
--------------
$ hg push
pushing to $TESTTMP/E1/server
searching for changes
adding changesets
adding manifests
adding file changes
added 1 changesets with 1 changes to 1 files (+1 heads)
1 new obsolescence markers
obsoleted 1 changesets
$ cd ../..

tests/test-push-checkheads-multibranches-E2.t

  • This file was added.
====================================
Testing head checking code: Case E-2
====================================
Mercurial checks for the introduction of new heads on push. Evolution comes
into play to detect if existing branches on the server are being replaced by
some of the new one we push.
This case is part of a series of tests checking this behavior.
Category E: case involving changeset on multiple branch
TestCase 8: moving interleaved branch away from each other
.. old-state:
..
.. * 2-changeset on branch default
.. * 1-changeset on branch Z (between the two other)
..
.. new-state:
..
.. * 2-changeset on branch default, aligned
.. * 1-changeset on branch Z (at the same location)
..
.. expected-result:
..
.. * push allowed
..
.. graph-summary:
..
.. C ø⇠◔ C'
.. | |
.. B ◔ |
.. | |
.. A ø⇠◔ A'
.. |/
.. ●
$ . $TESTDIR/testlib/push-checkheads-util.sh
Test setup
----------
$ mkdir E1
$ cd E1
$ setuprepos
creating basic server and client repo
updating to branch default
2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ cd client
$ hg branch Z
marked working directory as branch Z
(branches are permanent and global, did you want a bookmark?)
$ mkcommit B0
$ hg branch default --force
marked working directory as branch default
$ mkcommit C0
created new head
$ hg push --new-branch
pushing to $TESTTMP/E1/server
searching for changes
adding changesets
adding manifests
adding file changes
added 2 changesets with 2 changes to 2 files
$ hg up 0
0 files updated, 0 files merged, 3 files removed, 0 files unresolved
$ mkcommit A1
created new head
$ mkcommit C1
$ hg debugobsolete `getid "desc(A0)" ` `getid "desc(A1)"`
1 new obsolescence markers
obsoleted 1 changesets
2 new orphan changesets
$ hg debugobsolete `getid "desc(C0)" ` `getid "desc(C1)"`
1 new obsolescence markers
obsoleted 1 changesets
$ hg log -G --hidden
@ 0c76bc104656 (draft): C1
|
o f6082bc4ffef (draft): A1
|
| x afc55ba2ce61 (draft): C0
| |
| * 93e5c1321ece (draft): B0
| |
| x 8aaa48160adc (draft): A0
|/
o 1e4be0697311 (public): root
Actual testing
--------------
$ hg push -r 'desc("C1")'
pushing to $TESTTMP/E1/server
searching for changes
adding changesets
adding manifests
adding file changes
added 2 changesets with 2 changes to 2 files (+1 heads)
2 new obsolescence markers
obsoleted 2 changesets
1 new orphan changesets
$ cd ../..

tests/test-push-checkheads-multibranches-E3.t

  • This file was added.
====================================
Testing head checking code: Case E-3
====================================
Mercurial checks for the introduction of new heads on push. Evolution comes
into play to detect if existing branches on the server are being replaced by
some of the new one we push.
This case is part of a series of tests checking this behavior.
Category E: case involving changeset on multiple branch
TestCase 8: moving only part of the interleaved branch away, creating 2 heads
.. old-state:
..
.. * 2-changeset on branch default
.. * 1-changeset on branch Z (between the two other)
..
.. new-state:
..
.. * 2-changeset on branch default, on untouched, the other moved
.. * 1-changeset on branch Z (at the same location)
..
.. expected-result:
..
.. * push rejected
..
.. graph-summary:
..
.. C ø⇠◔ C'
.. | |
.. B ◔ |
.. | |
.. A ◔ |
.. |/
.. ●
$ . $TESTDIR/testlib/push-checkheads-util.sh
Test setup
----------
$ mkdir E1
$ cd E1
$ setuprepos
creating basic server and client repo
updating to branch default
2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ cd client
$ hg branch Z
marked working directory as branch Z
(branches are permanent and global, did you want a bookmark?)
$ mkcommit B0
$ hg branch default --force
marked working directory as branch default
$ mkcommit C0
created new head
$ hg push --new-branch
pushing to $TESTTMP/E1/server
searching for changes
adding changesets
adding manifests
adding file changes
added 2 changesets with 2 changes to 2 files
$ hg up 0
0 files updated, 0 files merged, 3 files removed, 0 files unresolved
$ mkcommit C1
created new head
$ hg debugobsolete `getid "desc(C0)" ` `getid "desc(C1)"`
1 new obsolescence markers
obsoleted 1 changesets
$ hg log -G --hidden
@ dc44c53142f0 (draft): C1
|
| x afc55ba2ce61 (draft): C0
| |
| o 93e5c1321ece (draft): B0
| |
| o 8aaa48160adc (draft): A0
|/
o 1e4be0697311 (public): root
Actual testing
--------------
$ hg push -r 'desc("C1")'
pushing to $TESTTMP/E1/server
searching for changes
abort: push creates new remote head dc44c53142f0!
(merge or see 'hg help push' for details about pushing new heads)
[255]
$ cd ../..