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

Matt Mackall mpm at selenic.com
Tue Jun 22 15:46:50 CDT 2010


On Mon, 2010-06-21 at 22:34 +0200, Christian Boos wrote:
> 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 ;-)

Looks good, please submit a for-real copy.

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list