Patchwork rebase: move state serialization to use unfiltered repo

login
register
mail settings
Submitter Durham Goode
Date March 12, 2017, 7:41 p.m.
Message ID <ec3731bcacca0a6c65da.1489347685@dev111.prn1.facebook.com>
Download mbox | patch
Permalink /patch/19217/
State Accepted
Headers show

Comments

Durham Goode - March 12, 2017, 7:41 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1489347215 25200
#      Sun Mar 12 12:33:35 2017 -0700
# Node ID ec3731bcacca0a6c65da08e20f7784bdd210161d
# Parent  718a57e95a897f4ac407ae3733a7d41e87354acb
rebase: move state serialization to use unfiltered repo

Now that rebasestate is serialized as part of the transaction, the repo state it
sees is the version at the end of the transaction, which may have hidden nodes.
Therefore, it's possible parts of the rebase commit set are no longer visible by
the time the transaction is closing, which causes a filtered revision error in
this code. I don't think state serialization should be blocked from accessing
commits it knows exist, especially if all it's trying to do is get the hex of
them, so let's use an unfiltered repo here.

Unfortunately, the only known repro is with the fbamend Facebook extension, so
I'm not sure how to repro it in core Mercurial for a test.
Augie Fackler - March 12, 2017, 8:02 p.m.
On Sun, Mar 12, 2017 at 12:41:25PM -0700, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1489347215 25200
> #      Sun Mar 12 12:33:35 2017 -0700
> # Node ID ec3731bcacca0a6c65da08e20f7784bdd210161d
> # Parent  718a57e95a897f4ac407ae3733a7d41e87354acb
> rebase: move state serialization to use unfiltered repo

queued

>
> Now that rebasestate is serialized as part of the transaction, the repo state it
> sees is the version at the end of the transaction, which may have hidden nodes.
> Therefore, it's possible parts of the rebase commit set are no longer visible by
> the time the transaction is closing, which causes a filtered revision error in
> this code. I don't think state serialization should be blocked from accessing
> commits it knows exist, especially if all it's trying to do is get the hex of
> them, so let's use an unfiltered repo here.
>
> Unfortunately, the only known repro is with the fbamend Facebook extension, so
> I'm not sure how to repro it in core Mercurial for a test.
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -169,7 +169,7 @@ class rebaseruntime(object):
>                  self._writestatus(f)
>
>      def _writestatus(self, f):
> -        repo = self.repo
> +        repo = self.repo.unfiltered()
>          f.write(repo[self.originalwd].hex() + '\n')
>          f.write(repo[self.target].hex() + '\n')
>          f.write(repo[self.external].hex() + '\n')
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -169,7 +169,7 @@  class rebaseruntime(object):
                 self._writestatus(f)
 
     def _writestatus(self, f):
-        repo = self.repo
+        repo = self.repo.unfiltered()
         f.write(repo[self.originalwd].hex() + '\n')
         f.write(repo[self.target].hex() + '\n')
         f.write(repo[self.external].hex() + '\n')