[PATCH] memctx: fix manifest for removed files (issue4470)
FUJIWARA Katsunori
foozy at lares.dti.ne.jp
Wed Dec 17 05:52:16 CST 2014
At Wed, 17 Dec 2014 02:09:33 -0800,
Pierre-Yves David wrote:
>
>
>
> On 12/16/2014 10:58 PM, Sean Farley wrote:
> >
> > FUJIWARA Katsunori writes:
> >
> >> At Tue, 16 Dec 2014 15:33:11 -0800,
> >> Pierre-Yves David wrote:
> >>>
> >>> On 12/16/2014 11:39 AM, Jordi GutiƩrrez Hermoso wrote:
> >>>> On Tue, 2014-12-16 at 10:43 -0500, Augie Fackler wrote:
> >>>>> # HG changeset patch
> >>>>> # User Augie Fackler <augie at google.com>
> >>>>> # Date 1418673654 18000
> >>>>> # Mon Dec 15 15:00:54 2014 -0500
> >>>>> # Node ID 40bd85091fb37fc992e3af2128a8ed31f31a47ff
> >>>>> # Parent 65c854f92d6ba8861414ff3191182fba28777a82
> >>>>> memctx: fix manifest for removed files (issue4470)
> >>>>
> >>>> Nice! Thanks for the fix!
> >>>>
> >>>>> diff --git a/tests/test-commit.t b/tests/test-commit.t
> >>>>> --- a/tests/test-commit.t
> >>>>> +++ b/tests/test-commit.t
> >>>>> @@ -429,6 +429,68 @@ specific template keywords work well
> >>>> [snip]
> >>>>> + $ cd ..
> >>>>> + $ rm -r lol
> >>>>
> >>>> Maybe remove this last `rm -r` from the test? I had it in the original
> >>>> repro, but it seems out of place in the test. It's useful to keep the
> >>>> repos in the tests when doing run-tests.py --keep-tmpdir.
> >>>>
> >>>> Also, perhaps a better name than "lol" should be used here? I'm kind
> >>>> of cavalier about my metasyntactic variables.
> >>>
> >>> I've renamed "lol" to issue4470, dropped the rm and pushted the result
> >>> to the clowncopter.
> >>
> >> Oh! I also just finished patches to fix this issue !
> >>
> >> Even though it may be too late, I post my patches for reference :-)
> >>
> >> In addition to fixing issue4470, my series also fixes "missing newly
> >> added files in the manifest" issue of memctx (by #2 of series)
> >
> > Wow, this is much more robust and helps with issues when trying to use
> > memctx in the merge machinery (I have a half-baked version of what
> > Katsunori wrote). Please queue this one if not too late.
>
> The foozy series looks really good and I would be very happy to queue
> it. But I wonder how it does related with the Augie one? I feel like
> they are orthogonal as:
> - Augie make a magic value explicit (which is good),
> - Foozy make memory status more correct.
>
> If someone can confirm my diagnostic that no adjustment are needed I'll
> happily push it.
>
> --
> Pierre-Yves David
Core of Augie's patch is avoiding manifest hash calculation and
deleting a entry from manifest, if "self[file]" returns None (=
meaning "file is removed")
================
+ fctx = self[f]
+ if fctx is None:
+ # removed file
+ del man[f]
+ else:
+ man[f] = revlog.hash(fctx.data(), p1node, p2node)
================
On the other hand, my series achieves this in two steps:
- calculate "memctx._status" correctly (patch #1/3)
issue 4470 can't be resolved only with this change, but other
patches rely on correctness of "memctx._status".
- maintain manifest for "modified" and "removed" separately (patch #3/3)
At first, manifest entries for "_status.modified" files are
updated. This can avoid manifest hash calculation for removed
files, because removed files shouldn't be listed up in
"_status.modified" (and this also reduces loop cost for large
manifest).
================
- for f, fnode in man.iteritems():
+ for f in self._status.modified:
p1node = nullid
p2node = nullid
....
man[f] = revlog.hash(self[f].data(), nullid, nullid)
================
Then, manifest entries for "_status.removed" files are deleted.
================
+ for f in self._status.removed:
+ if f in man:
+ del man[f]
+
return man
================
In addition to it, "memctx._manifest" has another issue that newly
added files aren't included.
So, patch #2/3 adds manifest entries for "_status.added" files.
================
+ for f in self._status.added:
+ man[f] = revlog.hash(self[f].data(), nullid, nullid)
+
================
----------------------------------------------------------------------
[FUJIWARA Katsunori] foozy at lares.dti.ne.jp
More information about the Mercurial-devel
mailing list