[Pull Request] Remove uses of hasattr

[Augie Fackler]

On Jul 26, 2011, at 2:54 AM, Adrian Buehlmann wrote:

> On 2011-07-26 11:40, Adrian Buehlmann wrote:
>> On 2011-07-26 05:44, Augie Fackler wrote:
>>> A while ago I asked about removing hasattr because of surprising behavior, and the general consensus seemed to be that I should go ahead and make a run at it. While on a flight today I finally finished the job. It's a really long patch queue, so instead of mailing it I'm submitting it as a pull request. You can see the changesets here:
>>> 
>>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/list?num=50
>>> 
>>> and if you want to pull/incoming, the URL you want is
>>> 
>>> http://code.google.com/p/durin42-hg-hasattr-ectomy/
>>> 
>>> It's 38 patches in total. I tried to batch them for easy reviewing. Ones of note are:
>>> 
>>> Duplicate less code when checking to see if we're in a frozen binary:
>>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/detail?r=b6581ed193f374f2d38a4cad5ad25640f304ce1f
>>> 
>>> These two attack a particular hasattr() use case in the entire codebase since it's mechanically fixable:
>>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/detail?r=f4814ebb99f75812a55776ed3a306883432935dd
>>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/detail?r=235f15a351866c2c084d4e88b89f7b69fafa66da
>>> 
>>> and this last one of note adds modules to the blacklist because of those side effects of hasattr() I warned about:
>>> http://code.google.com/p/durin42-hg-hasattr-ectomy/source/detail?r=57c2982d243bd6651e1d9e3f138f46b27b32828f
>>> 
>>> Other than that, it's mostly mechanical replacement, with a few migrations to getattr() so we can avoid looking up the attribute twice. I tried to make the patches all tiny so they'd be easy to review.
>> 
>> Augie asked to pull:
>>> # HG changeset patch
>>> # User Augie Fackler <durin42 at gmail.com>
>>> # Date 1311623995 18000
>>> # Node ID 9b736a33999292cf7fe5a8e0edbd145c9dd32e19
>>> # Parent  b6581ed193f374f2d38a4cad5ad25640f304ce1f
>>> safehasattr: new function to work around hasattr being broken
>>> 
>>> diff --git a/mercurial/util.py b/mercurial/util.py
>>> --- a/mercurial/util.py
>>> +++ b/mercurial/util.py
>>> @@ -29,6 +29,10 @@
>>> def sha1(s):
>>>     return _fastsha1(s)
>>> 
>>> +_notset = object()
>>> +def safehasattr(thing, attr):
>>> +    return getattr(thing, attr, _notset) is not _notset
>>> +
>>> def _fastsha1(s):
>>>     # This function will import sha1 from hashlib or sha (whichever is
>>>     # available) and overwrite itself with it on the first call.
>> 
>> IMHO, it would have been nice to write in the change message in what
>> ways hasattr is broken.
>> 
>> Or at least put a pointer to an explanation.
> 
> Augie patched:
>> # HG changeset patch
>> # User Augie Fackler <durin42 at gmail.com>
>> # Date 1311623971 18000
>> # Node ID e7859521b23b358659ab8a918677ccaf3ed46008
>> # Parent  27f3169fd8f4affa374201af287698d5f59c920f
>> check-code: disallow use of hasattr()
> 
> Or perhaps the explanation why hasattr is bad could be given in this
> change message (it's the topmost patch)

Hm. I think I'll put a mention in the first patch.

Does anyone else (mpm?) want to take a look before I make that change and bulldoze the old changeset nodes?