[PATCH] memctx: fix manifest for removed files (issue4470)

[Pierre-Yves David]

On 12/17/2014 08:38 AM, Augie Fackler wrote:
> On Wed, Dec 17, 2014 at 02:09:33AM -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),
>
> I'm not doing much of that in my code?
>
>> - Foozy make memory status more correct.
>
> ...and then uses it to avoid iterating over the entire manifest, which
> is a huge win.
>
> My feeling is that foozy's change may be a bit big for stable, but
> that it's the better long-term fix. If we want a fix for this on
> stable (might be nice, but I didn't consider it), mine may be more
> suitable. I defer to you and mpm on that.
>
> foozy's patches should definitely replace mine for default. It's a
> better solution by far.
It seem to me that you patch are otogonal because foozy does nto replace 
your patch. And you patch still sound a net positive.

So I've pushed you patch on stable and foozy's on default.

-- 
Pierre-Yves David