[PATCH 0 of 2] setup: don't build inotify extension when <sys/inotify.h> is too old

Christian Boos cboos at neuf.fr
Mon Jun 21 15:34:18 CDT 2010


On 6/21/2010 9:49 PM, Renato Cunha wrote:
> On Mon, Jun 21, 2010 at 09:53:08AM +0200, Christian Boos wrote:
>    
>> Here's a new patch, which ignore failures to build extensions marked
>> as optional (so only inotify currently), in the same way distutils
>> does it in Python 3.
>> (...)
>>      
> I like the latter solution, I was willing to implement something similar. Good
> thing you checked it out before me. :)
>
>    
>> @@ -209,6 +211,20 @@
>>   Distribution.global_options.append(('pure', None, "use pure (slow)
>> Python "
>>                                       "code instead of C extensions"))
>>
>> +class hgbuildext(build_ext):
>> +
>> +    def build_extensions(self):
>> +        # First, sanity-check the 'extensions' list
>> +        self.check_extensions_list(self.extensions)
>> +
>> +        for ext in self.extensions:
>> +            try:
>> +                self.build_extension(ext)
>> +            except CCompilerError:
>> +                if not hasattr(ext, 'optional') or not ext.optional:
>> +                    raise
>> +                print>>sys.stderr, "Skipping optional '%s'" % ext.name
>>      
> That lone raise isn't very nice. Perhaps a
> raise SystemExit("Failed to build non-optional extension: %s." % ext.nam)
> would've been better...
>    

A raise without parameters in an except block re-raise the exception 
that was just caught (in this case, the CCompilerError), and re-raising 
the exception instead is what py3k does in the same situation, so I 
think it's preferable than a SystemExit.

What could perhaps be improved is to use log.warn(...) instead of 
print>>sys.stderr, as the rest of distutils does.

>> @@ -260,6 +277,7 @@
>>       if hasfunction(cc, 'inotify_add_watch'):
>>           extmodules.append(Extension('hgext.inotify.linux._inotify',
>>                                        ['hgext/inotify/linux/_inotify.c']))
>> +        extmodules[-1].optional = True
>>           packages.extend(['hgext.inotify', 'hgext.inotify.linux'])
>>      
> What about:
>
> inotify = Extension('hgext.inotify.linux._inotify', ...)
> inotify.optional = True
> extmodules.append(inotify)
>
> Don't know, but makes the thing a bit more readable.
>    

As I couldn't tell which style would be preferred, I went for the 
minimal diff ;-)

-- Christian



More information about the Mercurial-devel mailing list