Patchwork [1,of,2,STABLE] pull: fix crash when pulling changeset that get hidden locally (issue3788)

login
register
mail settings
Submitter Pierre-Yves David
Date Jan. 29, 2013, 2:27 p.m.
Message ID <4a4554ba58755595d927.1359469637@crater1.logilab.fr>
Download mbox | patch
Permalink /patch/752/
State Accepted
Commit 4d9f7dd2ac822c1e4efde421235df2a00c8c6801
Delegated to: Kevin Bullock
Headers show

Comments

Pierre-Yves David - Jan. 29, 2013, 2:27 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@logilab.fr>
# Date 1359469570 -3600
# Branch stable
# Node ID 4a4554ba58755595d927ce4d77ecd2121f2e032e
# Parent  d1c13a4dc638cefc71728da24f91a723313b049e
pull: fix crash when pulling changeset that get hidden locally (issue3788)

When you have obsolescence marker that apply to a pulled changesets, the added
changeset is immediately filtered. Then the list of added changeset needs to be
build against and unfiltered repo.
Idan Kamara - Jan. 29, 2013, 2:31 p.m.
On Tue, Jan 29, 2013 at 4:27 PM, <pierre-yves.david@logilab.fr> wrote:
>
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@logilab.fr>
> # Date 1359469570 -3600
> # Branch stable
> # Node ID 4a4554ba58755595d927ce4d77ecd2121f2e032e
> # Parent  d1c13a4dc638cefc71728da24f91a723313b049e
> pull: fix crash when pulling changeset that get hidden locally (issue3788)
>
> When you have obsolescence marker that apply to a pulled changesets, the
> added
> changeset is immediately filtered. Then the list of added changeset needs
> to be
> build against and unfiltered repo.
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -1686,14 +1686,18 @@ class localrepository(object):
>                      raise util.Abort(_("partial pull cannot be done
> because "
>                                             "other repository doesn't
> support "
>                                             "changegroupsubset."))
>                  else:
>                      cg = remote.changegroupsubset(fetch, heads, 'pull')
> -                clstart = len(self.changelog)
> +                # we use unfiltered changelog here because hidden
> revision must
> +                # be taken in account for phase synchronization. They may
> +                # becomes public and becomes visible again.
> +                cl = self.unfiltered().changelog
> +                clstart = len(cl)
>                  result = self.addchangegroup(cg, 'pull', remote.url())
> -                clend = len(self.changelog)
> -                added = [self.changelog.node(r) for r in xrange(clstart,
> clend)]
> +                clend = len(cl)
> +                added = [cl.node(r) for r in xrange(clstart, clend)]
>
>              # compute target subset
>              if heads is None:
>                  # We pulled every thing possible
>                  # sync on everything common
> diff --git a/tests/test-obsolete-changeset-exchange.t
> b/tests/test-obsolete-changeset-exchange.t
> --- a/tests/test-obsolete-changeset-exchange.t
> +++ b/tests/test-obsolete-changeset-exchange.t
> @@ -51,5 +51,25 @@ Push it. The bundle should not refer to
>    checking changesets
>    checking manifests
>    crosschecking files in changesets and manifests
>    checking files
>    2 files, 2 changesets, 2 total revisions
> +
> +Adding a changeset going extinct locally
> +------------------------------------------
> +
> +Pull a changeset that will immediatly goes extinct (because you already
> have a
> +marker to obsolete him)
> +(test resolution of issue3788)
> +
> +  $ hg phase --draft --force f89bcc95eba5
> +  $ hg phase -R ../other --draft --force f89bcc95eba5
> +  $ hg commit --amend -m "A''"
> +  $ hg --hidden --config extensions.mq= strip  --no-backup f89bcc95eba5
> +  $ hg pull ../other
> +  pulling from ../other
> +  searching for changes
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 1 changesets with 0 changes to 1 files (+1 heads)
> +  (run 'hg heads' to see heads, 'hg merge' to merge)

If the pulled changeset is filtered, shouldn't this message
reflect that and report something else?
Pierre-Yves David - Jan. 29, 2013, 3:17 p.m.
On Tue, Jan 29, 2013 at 04:31:34PM +0200, Idan Kamara wrote:
> On Tue, Jan 29, 2013 at 4:27 PM, <pierre-yves.david@logilab.fr> wrote:
> >
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david@logilab.fr>
> > # Date 1359469570 -3600
> > # Branch stable
> > # Node ID 4a4554ba58755595d927ce4d77ecd2121f2e032e
> > # Parent  d1c13a4dc638cefc71728da24f91a723313b049e
> > pull: fix crash when pulling changeset that get hidden locally (issue3788)
> >
> > When you have obsolescence marker that apply to a pulled changesets, the
> > added
> > changeset is immediately filtered. Then the list of added changeset needs
> > to be
> > build against and unfiltered repo.
> >
> > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> > --- a/mercurial/localrepo.py
> > +++ b/mercurial/localrepo.py
> > @@ -1686,14 +1686,18 @@ class localrepository(object):
> >                      raise util.Abort(_("partial pull cannot be done
> > because "
> >                                             "other repository doesn't
> > support "
> >                                             "changegroupsubset."))
> >                  else:
> >                      cg = remote.changegroupsubset(fetch, heads, 'pull')
> > -                clstart = len(self.changelog)
> > +                # we use unfiltered changelog here because hidden
> > revision must
> > +                # be taken in account for phase synchronization. They may
> > +                # becomes public and becomes visible again.
> > +                cl = self.unfiltered().changelog
> > +                clstart = len(cl)
> >                  result = self.addchangegroup(cg, 'pull', remote.url())
> > -                clend = len(self.changelog)
> > -                added = [self.changelog.node(r) for r in xrange(clstart,
> > clend)]
> > +                clend = len(cl)
> > +                added = [cl.node(r) for r in xrange(clstart, clend)]
> >
> >              # compute target subset
> >              if heads is None:
> >                  # We pulled every thing possible
> >                  # sync on everything common
> > diff --git a/tests/test-obsolete-changeset-exchange.t
> > b/tests/test-obsolete-changeset-exchange.t
> > --- a/tests/test-obsolete-changeset-exchange.t
> > +++ b/tests/test-obsolete-changeset-exchange.t
> > @@ -51,5 +51,25 @@ Push it. The bundle should not refer to
> >    checking changesets
> >    checking manifests
> >    crosschecking files in changesets and manifests
> >    checking files
> >    2 files, 2 changesets, 2 total revisions
> > +
> > +Adding a changeset going extinct locally
> > +------------------------------------------
> > +
> > +Pull a changeset that will immediatly goes extinct (because you already
> > have a
> > +marker to obsolete him)
> > +(test resolution of issue3788)
> > +
> > +  $ hg phase --draft --force f89bcc95eba5
> > +  $ hg phase -R ../other --draft --force f89bcc95eba5
> > +  $ hg commit --amend -m "A''"
> > +  $ hg --hidden --config extensions.mq= strip  --no-backup f89bcc95eba5
> > +  $ hg pull ../other
> > +  pulling from ../other
> > +  searching for changes
> > +  adding changesets
> > +  adding manifests
> > +  adding file changes
> > +  added 1 changesets with 0 changes to 1 files (+1 heads)
> > +  (run 'hg heads' to see heads, 'hg merge' to merge)
> 
> If the pulled changeset is filtered, shouldn't this message
> reflect that and report something else?

Yes it should, but
1) it already behave badly in previous version
2) there the same kind of issue in various other places
3) We are in freeze, I just fixes traceback and other fun regression.

So this is left of later adventures (yoohoo)

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1686,14 +1686,18 @@  class localrepository(object):
                     raise util.Abort(_("partial pull cannot be done because "
                                            "other repository doesn't support "
                                            "changegroupsubset."))
                 else:
                     cg = remote.changegroupsubset(fetch, heads, 'pull')
-                clstart = len(self.changelog)
+                # we use unfiltered changelog here because hidden revision must
+                # be taken in account for phase synchronization. They may
+                # becomes public and becomes visible again.
+                cl = self.unfiltered().changelog
+                clstart = len(cl)
                 result = self.addchangegroup(cg, 'pull', remote.url())
-                clend = len(self.changelog)
-                added = [self.changelog.node(r) for r in xrange(clstart, clend)]
+                clend = len(cl)
+                added = [cl.node(r) for r in xrange(clstart, clend)]
 
             # compute target subset
             if heads is None:
                 # We pulled every thing possible
                 # sync on everything common
diff --git a/tests/test-obsolete-changeset-exchange.t b/tests/test-obsolete-changeset-exchange.t
--- a/tests/test-obsolete-changeset-exchange.t
+++ b/tests/test-obsolete-changeset-exchange.t
@@ -51,5 +51,25 @@  Push it. The bundle should not refer to 
   checking changesets
   checking manifests
   crosschecking files in changesets and manifests
   checking files
   2 files, 2 changesets, 2 total revisions
+
+Adding a changeset going extinct locally
+------------------------------------------
+
+Pull a changeset that will immediatly goes extinct (because you already have a
+marker to obsolete him)
+(test resolution of issue3788)
+
+  $ hg phase --draft --force f89bcc95eba5
+  $ hg phase -R ../other --draft --force f89bcc95eba5
+  $ hg commit --amend -m "A''"
+  $ hg --hidden --config extensions.mq= strip  --no-backup f89bcc95eba5
+  $ hg pull ../other
+  pulling from ../other
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 0 changes to 1 files (+1 heads)
+  (run 'hg heads' to see heads, 'hg merge' to merge)