Bug 5678 - Phase of public revisions reset to draft after rebasing a draft rev. on top of the public head
Summary: Phase of public revisions reset to draft after rebasing a draft rev. on top o...
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: rebase (show other bugs)
Version: 4.3
Hardware: Macintosh Mac OS
: urgent bug
Assignee: Bugzilla
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2017-09-13 14:46 UTC by Matt N.
Modified: 2017-10-18 00:00 UTC (History)
5 users (show)

See Also:
Python Version: ---


Attachments
Log with experimental.evolution=createmarkers (1.60 KB, text/plain)
2017-09-13 17:05 UTC, Matt N.
Details
Debug log of the failure (2.76 KB, text/plain)
2017-09-13 17:12 UTC, Matt N.
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matt N. 2017-09-13 14:46 UTC
STR:

$ hg log --graph --rev=wip --template "{rev} {phase} {firstline(desc)}\n"
> o  380545 public This is the public head
> :
> : @  380260 draft This is my draft commit
> :/
> o  380259 public This is the 2nd-last public commit
> |
> ~

$ hg rebase -s 380260 -d 380545
> rebasing 380260:7bf1ca063893 "This is my draft commit"
> saved backup bundle to /Users/…/mozilla-central2/.hg/strip-backup/7bf1ca063893-565e6f60-rebase.hg


Expected result:

I can get to this state by doing a `hg pull` which seems to fix the phase info.

$ hg log --graph --rev=wip --template "{rev} {phase} {firstline(desc)}\n"
> @  380545 draft This is my draft commit
> |
> o  380544 public This is the public head
> |
> ~

Actual result: 

Note that revs that were previously public are now draft:

$ hg log --graph --rev=wip --template "{rev} {phase} {firstline(desc)}\n"                                                           
> @  380545 draft This is my draft commit
> |
> o  380544 draft This is the public head
> |
> o  380543 draft This is the 2nd-last public commit
> |
> o    380542 draft merge autoland to mozilla-central. r=merge a=merge
> |\
> | ~
> o  380541 draft Bug 1397717 - Using GenericPrinter for DEBUG-only C++ dump() APIs;r=nbp
> |
> o  380540 draft Bug 1397141 - part8 : update test for video under 48x48. r=jya
> |
> o  380539 draft Bug 1397141 - part7 : update error description in MFR. r=jya
> |
> o  380538 draft Bug 1397141 - part6 : use MediaResult to replace nsresult r=jya
> |
> ~

My coworker can also reproduce this with 4.3.1

Mercurial Distributed SCM (version 4.3.1)
Comment 1 Matt N. 2017-09-13 14:48 UTC
Marking as confirmed since my coworker independently found the same issue.
Comment 2 Jun Wu 2017-09-13 15:04 UTC
Do you mean s/380545/380544/ in the first command? I failed to create a repro using a small repo (without 3rd party extensions and with obsstore disabled).
Comment 3 Gregory Szorc 2017-09-13 15:09 UTC
The integer revision numbers aren't portable across clones. If there is continued difficulty reproducing, please use {node|short} instead of {rev} next time so actual hashes can be used.
Comment 4 Matt N. 2017-09-13 15:42 UTC
(In reply to Jun Wu from comment #2)
> Do you mean s/380545/380544/ in the first command? 

No, if you look at the first graph 38045 is the public head and I wanted to go on top of that. I think the integer revision numbers changed with the rebase so don't pay attention to them, just look at the phase and commit message.

> I failed to create a
> repro using a small repo (without 3rd party extensions and with obsstore
> disabled).

I realized after filing that I should have tried without 3rd-party extensions. Let me try again now.
Comment 5 Matt N. 2017-09-13 16:01 UTC
Reproduces with a /dev/null config + rebase:

$ HGRCPATH=/dev/null hg log --graph -l 3 --template "{node|short} {phase} {firstline(desc)}\n"
> o  e5f80a639bfe public No bug, Automated HPKP preload list update from host bld-linux64-spot-308 - a=hpkp-update
> |
> | @  75448a5baaa1 draft This is my draft commit
> |/
> o  027c9df4e1b5 public No bug, Automated HSTS preload list update from host bld-linux64-spot-308 - a=hsts-update
> |
> ~


$ HGRCPATH=/dev/null hg --config 'extensions.rebase=' rebase -s 75448a5baaa1 -d e5f80a639bfe
> rebasing 380544:75448a5baaa1 "This is my draft commit"
> saved backup bundle to /Users/…/mozilla-central2/.hg/strip-backup/75448a5baaa1-1be84303-rebase.hg

$ HGRCPATH=/dev/null hg log --graph -l 3 --template "{node|short} {phase} {firstline(desc)}\n"
> @  e97bd32c2480 draft This is my draft commit
> |
> o  e5f80a639bfe draft No bug, Automated HPKP preload list update from host bld-linux64-spot-308 - a=hpkp-update
> |
> o  027c9df4e1b5 public No bug, Automated HSTS preload list update from host bld-linux64-spot-308 - a=hsts-update
> |
> ~

e5f80a639bfe should be public
Comment 6 Jun Wu 2017-09-13 16:16 UTC
Since I still have difficulty reproducing using smaller repo. Could you try enable obsstore once and try if it repos?

  [experimental]
  evolution=createmarkers

You can disable the config, remove .hg/store/obsstore and strip manually afterwards to restore the repo state.

This will be helpful to check if it's strip / cache invalidation related.
Comment 7 Matt N. 2017-09-13 17:05 UTC
Created attachment 1973 [details]
Log with experimental.evolution=createmarkers

The problem doesn't happen with experimental.evolution=createmarkers
Comment 8 Matt N. 2017-09-13 17:12 UTC
Created attachment 1974 [details]
Debug log of the failure
Comment 9 Matt N. 2017-09-13 17:16 UTC
I'm just guessing but the following from the log may be relevant:

> removing unknown node e5f80a639bfe from 1-phase boundary
> removing unknown node f4265300ce96 from 1-phase boundary

e5f80a639bfe is the public head that I'm rebasing on top of
f4265300ce96 is my draft head that I'm trying to move on top of e5f80a639bfe
Comment 10 Martin von Zweigbergk 2017-09-13 17:50 UTC
I saw that part about the "removing unknown node", but they should be part of the bundle and be restored when the bundle is applied. Can you share the output of

  hg debugbundle .hg/strip-backup/f4265300ce96-94f982ed-rebase.hg
Comment 11 Matt N. 2017-09-13 20:09 UTC
(In reply to Martin von Zweigbergk from comment #10)
> I saw that part about the "removing unknown node", but they should be part
> of the bundle and be restored when the bundle is applied. Can you share the
> output of
> 
>   hg debugbundle .hg/strip-backup/f4265300ce96-94f982ed-rebase.hg

$ hg debugbundle .hg/strip-backup/f4265300ce96-94f982ed-rebase.hg
> f4265300ce969aa45e71960f3b669eca93406fb9
Comment 12 Martin von Zweigbergk 2017-09-14 13:41 UTC
Oh, now I think I see what's going on. That output seems to be for a changegroup v1 bundle (aka bundle1). The only reason you would be using changegroup v1 is if you're not using generaldelta, so I assume you're not. I'm not sure what the right fix is yet, but it seems like we should be using cg2 for strip even in non-generaldelta repos.
Comment 13 Martin von Zweigbergk 2017-09-14 19:13 UTC
Patches sent earlier today:
https://phab.mercurial-scm.org/D714
https://phab.mercurial-scm.org/D715

Thanks for reporting this issue.
Comment 14 Bugzilla 2017-09-29 00:00 UTC
Bug marked urgent for 10 days, bumping
Comment 15 Martin von Zweigbergk 2017-09-29 08:18 UTC
Fixed by the previously mentioned patches
Comment 16 HG Bot 2017-10-04 18:16 UTC
Fixed by https://mercurial-scm.org/repo/hg/rev/b5d7e7d5c573
Martin von Zweigbergk <martinvonz@google.com>
tests: add test for issue5678

In addition to a test case for the direct problem described in the bug
report, this also adds a test case showing how obsmarkers can also get
lost when not using generaldelta.

Differential Revision: https://phab.mercurial-scm.org/D714

(please test the fix)
Comment 17 HG Bot 2017-10-04 18:16 UTC
Fixed by https://mercurial-scm.org/repo/hg/rev/91f0677dc920
Martin von Zweigbergk <martinvonz@google.com>
repair: preserve phase also when not using generaldelta (issue5678)

It seems like we used to pick the oldest possible version of the
changegroup to use for bundles created by the repair module (used
e.g. by "hg strip" and for temporary bundles by "hg rebase"). I tried
to preserve that behavior when I created the changegroup.safeversion()
method in 3b2ac2115464 (changegroup: introduce safeversion(),
2016-01-19).

However, we have recently chagned our minds and decided that these
commands are only used locally and downgrades are unlikely. That
decicion allowed us to start adding obsmarker and phase information to
these bundles. However, as the bug report shows, it means we get
different behavior e.g. when generaldelta is not enabled (because when
it was enabled, it forced us to use bundle2). The commit that actually
caused the reported bug was 8e3021fd1a44 (strip: include phases in
bundle (BC), 2017-06-15).

So, since we now depend on having more information in the bundles,
let's make sure we instead pick the newest possible changegroup
version.

Differential Revision: https://phab.mercurial-scm.org/D715

(please test the fix)
Comment 18 Bugzilla 2017-10-18 00:00 UTC
Bug was set to TESTING for 13 days, resolving