This is an archive of the discontinued Mercurial Phabricator instance.

obsutil: fix the issue5686
ClosedPublic

Authored by khanchi97 on Dec 22 2018, 4:28 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG191fac9ff9d3: obsutil: fix the issue5686
Summary

While traversing the obsolescence graph to find the successors sets
of csets:

In its 4th case (read comments of obsutil.successorssets to see
all 4 cases) where we know successors sets of all direct successors
of CURRENT, we were just missing a condition to filter out the case
when a cset is pruned.
And without this condition (that this patch added) it was making a whole
successor set to [] just because of one pruned marker.

For e.g:if following is the successors set of a cset A:

   A -> [a, b, c]

if we prune c, we expect A's successors set to be [a, b] but
you would get:
   A -> []

So this patch make sure that we calculate the right successorsset of csets
considering the pruned cset (in split case).

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

khanchi97 created this revision.Dec 22 2018, 4:28 PM

Please take a look at tests changes, as I accepted these changes using --interactive flag thinking it as a expected change.
I think I can't add tests which will reflect the expected bahivour for this issue here in mercurial, so I will send a test patch in evolve.

av6 added a subscriber: av6.Dec 23 2018, 1:43 AM
av6 added inline comments.
tests/test-obsmarker-template.t
2511

The new output looks somewhat more correct, but not quite. This changeset definitely wasn't just pruned, but it also wasn't rewritten either: it was split, and you can see correct hg fatelog output in the block below this one. The difference between them is wdir parent is different and the fatelog below has --hidden flag.

So is it expected that this patch doesn't fully fix this particular issue?

tests/test-obsolete.t
938

This title looks like it needs to mention content-divergence after this patch, and to use the new lingo.

1099–1120

This block tests how instabilities are explained in hgweb, so it would be good to have the new instability to show up: egrep '(orphan|-divergent)'. And the block's title needs to be updated too.

@av6 Thanks for the review. I will update it.

tests/test-obsmarker-template.t
2511

wdir parent is not making a difference here. If we pass --hidden here without updating the wdir parent, it will show the same result as block below. Maybe, this changed output is correct as it including those revision only which are visible to user.
So when we pass --hidden that hidden cset is visible and it outputs that it was a split operation.

Ready for review.

This revision was automatically updated to reflect the committed changes.