[Pull Request] Remove uses of hasattr

Adrian Buehlmann adrian at cadifra.com
Tue Jul 26 04:40:28 CDT 2011


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.


More information about the Mercurial-devel mailing list