[PATCH 4 of 7] patch: drop internalpatch checks for NoHunks

Patrick Mézard pmezard at gmail.com
Wed Apr 28 10:00:46 CDT 2010


Le 28/04/10 12:51, Mads Kiilerich a écrit :
> Matt Mackall wrote, On 04/28/2010 06:33 AM:
> ...
>> 4 looks suspicious, you'll need to tell us
>> more about that.
>>    
> 
> and
> 
> Augie Fackler wrote, On 04/26/2010 06:06 PM:
>> On Mon, Apr 26, 2010 at 7:37 AM, Mads Kiilerich<mads at kiilerich.com> 
>> wrote:
>>   
>>> # HG changeset patch
>>> # User Mads Kiilerich<mads at kiilerich.com>
>>> # Date 1272280862 -7200
>>> # Node ID 50c548cc4249b964cb5012307389f57377ea8149
>>> # Parent  7e65479fe7b82026f9016fb0bfa4a2a2c4917e57
>>> patch: drop internalpatch checks for NoHunks
>>>
>>> This will change the behavior for some (slightly bogus) patches, but
>>> the new
>>> behavior is IMHO both better and good.
>>>      
>> Can you describe what the behavior change will be? That feels like it
>> belongs in the commit message.
>>    
> 
> 
> At some point during development of the other patches I got some
> failures without this patch - but I can't reproduce the case and this
> patch isn't necessary.
> 
> I tried to understand the code and the semantics of the "empty" state
> variable and clean it up, but I had to give up. I couldn't see what the
> purpose with NoHunks was.
> 
> There is apparently no tests of NoHunks. This patch introduces no errors
> in the test suite.
> 
> In my opinion we don't have to recognize all malformed patches as long
> as we handle the well-formed patches correct and don't behave too badly
> on malformed patches.

To add my two cents here, I have been really careful when touching code in patch.py. This code was magically dropped from cmason's mpatch into Mercurial, and I know he did a lot of tests with many different patch inputs with what I assume is a really good coverage. I don't expect Mercurial test suite to test, say, a tenth of all possible broken inputs. In the meantime, my strategy had been not to remove things I found weird until I can really prove them wrong (which is not easy given the lack of any kind of specification of the diff and git diff formats). I believe the only time I did that, I broke some logic related to quilt patch header which may have come back in the BTS recently.

This is not a good situation.

What would be great would be to compile a patch queue of many different inputs and cram that in a single test. Something which would be fast and tell more or less reliably if something broke in patch.py instead of running *import* *mq* and so forth.

That said, I haven't look at what you are doing right now.

--
Patrick Mézard



More information about the Mercurial-devel mailing list