[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