On Fri, Sep 17, 2010 at 11:58 AM, Lisandro Dalcin <[email protected]> wrote: > On 17 September 2010 11:34, Gustavo Sverzut Barbieri > <[email protected]> wrote: >> On Fri, Sep 17, 2010 at 11:07 AM, Lisandro Dalcin <[email protected]> wrote: >>> On 17 September 2010 10:07, Gustavo Sverzut Barbieri >>> <[email protected]> wrote: >>>> changeset: 3753:62e52f105bc0 >>>> tag: tip >>>> user: Gustavo Sverzut Barbieri <[email protected]> >>>> date: Fri Sep 17 09:42:57 2010 -0300 >>>> files: Cython/Compiler/ModuleNode.py >>>> description: >>>> Force GCC>=4 to export module initialization function. >>>> >>>> With GCC's -fvisibility=hidden (both CFLAGS and LDFLAGS) it is >>>> possible to have the compiler to produce binaries where all symbols >>>> are hidden (local to the binary). To force a symbol to be visible one >>>> must specify __attribute__ ((visibility("default"))). >>>> >>> >>> Why do you need to force -fvisibility=hidden ? In general, Cython >>> emits 'static' storage specifier, unless 'public' or 'api' keywords >>> are involved. >> >> I don't. But we recently moved from setuptools to autoconf due lots of >> complaints of the former and I got tired and wasted 2 days to convert >> all my bindings, and users had that in their CFLAGS as the whole >> project compiles with it... except our bindings :-/ >> >> As for autoconf, later on I'll post the link to our examples... I even >> created a cython.m4 to help finding cython, its version and already >> cythonized files. What made us run away from setuptools is the deep >> level of magic and dynamic patching: setuptools/distutils checked for >> pyrex, thus I was faking cython as pyrex in sys.__modules__, but then >> people asked for easy_install and that was importing setuptools before >> we fake pyrex and then to unpatch all the classes was being a major >> PITA. autoconf/automake is not much easier, as it requires lots of >> code to get something done, but at least the rest of our developers >> are used to it due the C libraries... :-/ >> > > >> >>>> This patch introduces pre-processor code to always force module >>>> symbols to be visible. >>>> >>>> NOTE: in theory pyport.h from Python would do the right thing, but it >>>> does not define any special cases for GCC, just Windows. As Python is >>>> not changing that easily and even if it does is just for future releases, >>>> it's better to have those in Cython. >>>> >>> >>> Did you bug Python? >> >> not yet... >> >> >>>> diff --git a/Cython/Compiler/ModuleNode.py b/Cython/Compiler/ModuleNode.py >>>> --- a/Cython/Compiler/ModuleNode.py >>>> +++ b/Cython/Compiler/ModuleNode.py >>>> @@ -1671,9 +1671,15 @@ >>>> header2 = "PyMODINIT_FUNC init%s(void)" % env.module_name >>>> header3 = "PyMODINIT_FUNC PyInit_%s(void)" % env.module_name >>>> code.putln("#if PY_MAJOR_VERSION < 3") >>>> + code.putln("# if defined(__GNUC__) && (__GNUC__ >= 4)") >>>> + code.putln("__attribute__ ((visibility(\"default\")))") >>>> + code.putln("# endif") >>>> code.putln("%s; /*proto*/" % header2) >>>> code.putln(header2) >>>> code.putln("#else") >>>> + code.putln("# if defined(__GNUC__) && (__GNUC__ >= 4)") >>>> + code.putln("__attribute__ ((visibility(\"default\")))") >>>> + code.putln("# endif") >>>> code.putln("%s; /*proto*/" % header3) >>>> code.putln(header3) >>>> code.putln("#endif") >>>> >>> >>> It looks good, I support the patch. However, >>> >>> 1) Do you know how this would work in Windows with MinGW? I would >>> MinGW to do the right thing, but we have to be sure... >> >> AFAIK it does the right thing as we have libraries with that and it >> still work. Right now I don't have people to check it, but if you wish >> we could add a !windows before the check for gcc. >> > > No, I would prefer to test your patch in Windows and see if things > still work. But that would be next week. > >> >>> 2) Do you know if this could affect full static Python builds? (I >>> mean, when you build a Python interpreter with all ext modules >>> 'built-in' in the python binary) ? I cannot think of any problem, but >>> perhaps you do? >> >> no problems other than the symbol will be visible. >> > > Well, that could be a problem... Perhaps we should define a > CYTHON_MODINIT_FUNC_VISIBILITY macro, then as a last resort you can > change it. I'm thinking on something like: > > #if !defined(CYTHON_MODINIT_FUNC_VISIBILITY) > #if defined(__GNUC__) && (__GNUC__ >= 4) > #define CYTHON_MODINIT_FUNC_VISIBILITY __attribute__ ((visibility("default"))) > #else > #define CYTHON_MODINIT_FUNC_VISIBILITY > #endif > #endif > > and then we can emit code like this: > > PyMODINIT_FUNC init<name>(void) > CYTHON_MODINIT_FUNC_VISIBILITY > { > .... > } > > What do you think?
I'm fine with that, I just wanted the patch to remain as simple as possible, but this can be done as easily. -- Gustavo Sverzut Barbieri http://profusion.mobi embedded systems -------------------------------------- MSN: [email protected] Skype: gsbarbieri Mobile: +55 (19) 9225-2202 _______________________________________________ Cython-dev mailing list [email protected] http://codespeak.net/mailman/listinfo/cython-dev
