[Distutils] Re: CygwinCCompiler, msvc hack, BCPPCompiler
Greg Ward
gward@python.net
Sat, 8 Jul 2000 21:20:19 -0400
On 06 July 2000, Rene Liebscher said:
> It fixes my compiler class to avoid this warning on a GCC
> compiled python and it removes all the python specific stuff
> (except the above).
OK, good.
> It inserts this in build_ext (specifying the export symbol
> init{module} if no export symbols given and link the library python??)
> This means also I had to adapt the msvc compiler class and the Extension
> class (It should be possible to specify None as export symbols,
> this would mean standard python module with one export symbol,
> specifying a list would mean export what is in the list.)
Agreed on changed semantics of 'export_symbols'.
One reservation: I don't see how putting the "implib" file in the temp
dir is taken care of. I agree that it's better to do it in
msvccompiler.py using the 'build_temp' parameter to the 'link_*()'
methods, but I don't see that being done in your patch. Am I missing
something or did you forget that?
> I didn't changed bcppcompiler.py because I can't test it.
OK. Lyle, are you out there and following this? I can try to hack up
bcppcompiler.py to agree with msvccompiler.py and cygwincompiler.py, but
I can't test it either.
> The def-file will go through the export_symbols parameter.
> (as string or if given export_symbols as list)
Uh no, that's wrong: that's what 'export_symbol_file' is for. Wait,
maybe it's my fault for not supporting 'export_symbol_file' in the
CCompiler interface. Oops. I vote we drop 'export_symbol_file' from
Extension -- ie. require that the symbols to export be listed in a
Python list in your setup script, rather than in an external file -- and
see if anyone howls.
Detailed comments on the patch...
> --- distutils.orig/distutils/command/build_ext.py Fri Jun 30 17:18:01 2000
> +++ dp/distutils/command/build_ext.py Wed Jul 5 11:27:48 2000
> @@ -420,16 +420,40 @@
> objects.extend (ext.extra_objects)
> extra_args = ext.extra_link_args or []
>
> - # Bunch of fixing-up we have to do for Microsoft's linker.
> - if self.compiler.compiler_type == 'msvc':
> - self.msvc_prelink_hack(sources, ext, extra_args)
> + # For all platforms and compiler doesn't export all symbols
> + # by default, we have to specify the module's init function
> + # if the use doesn't provide any export information.
> + # All other platforms and compilers should ignore such
> + # export information.
[...]
Aiee! Too much code in one place. Please factor this out into two new
methods: I'd suggest
* get_export_symbols(ext) -> [string]
determine the list of symbols to export from the .so/.pyd
* get_libraries(ext) -> [string]
determine the list of libraries to link with when generating
the .so/.pyd
> -
> - # -- Hooks 'n hacks ------------------------------------------------
> -
> - def msvc_prelink_hack (self, sources, ext, extra_args):
> -
[...]
> - implib_file = os.path.join (
> - self.implib_dir,
> - self.get_ext_libname (ext.name))
> - extra_args.append ('/IMPLIB:' + implib_file)
> - self.mkpath (os.path.dirname (implib_file))
Again, I'm not convinced we can get rid of those yet. Convince me. ;-)
> --- distutils.orig/distutils/cygwinccompiler.py Fri Jun 30 17:17:55 2000
> +++ dp/distutils/cygwinccompiler.py Wed Jul 5 12:29:57 2000
> @@ -6,11 +6,34 @@
> cygwin in no-cygwin mode).
> """
>
> +# problems:
> +#
> +# * if you use a msvc compiled python version (1.5.2)
> +# 1. you have to insert a __GNUC__ section in its config.h
> +# 2. you have to generate a import library for its dll
> +# - create a def-file for python??.dll
> +# - create a import library using
> +# dlltool --dllname python15.dll --def python15.def \
> +# --output-lib libpython15.a
Ouch! So the Cygwin support is definitely not ready for prime time with
1.5.2. My understanding is that #1 has been fixed in the current Python
2.0 CVS code. What about #2? Should we try to convince Guido to
include libpython20.a the 2.0 distribution to save Cygwin/Mingw32 users
the bother of generating it?
Or maybe "build_ext" can grow a little bit of compiler-sensitivity for
Cygwin/Mingw32 and run the above command to generate libpython20.a the
first time the user tries to build an extension with Cygwin/Mingw32.
Come to think of it, this solves the problem nicely for 1.5.2. And hey,
we could even do the same thing for config.h, but that's rather more
dangerous -- modifying an existing file rather than generating a new
one. Worth considering, though.
> +# * cygwin gcc 2.91 works (after patching some include files)
> +# * mingw32 gcc 2.95.2 works (it doesn't support gcc -shared)
> +# * cygwin gcc 2.95.2 doesn't work now
> +# - its dllwrap doesn't work at my machine
> +# - target had to be changed to something like i686-cygwin32
> +# - mingw32-mode is specified by -mno-cygwin
> +# - using gcc -shared instead dllwrap didn't work because some problems with
> +# the import library, maybe it needs some additional parameters
AAUUGGHH!!! (What more can I say?)
> def check_config_h():
> """Checks if the GCC compiler is mentioned in config.h. If it is not,
> - compiling probably doesn't work, so print a warning to stderr.
> + compiling probably doesn't work.
> """
> -
> - # XXX the result of the check should be returned!
> + # return values
> + # 2: OK, python was compiled with GCC
> + # 1: OK, python's config.h mentions __GCC__
> + # 0: uncertain, because we couldn't check it
> + # -1: probably not OK, because we didn't found it in config.h
> + # You could check check_config_h()>0 => OK
Ick. Magic numbers bad. Strings good. Intelligent exception objects
(often) better. Think about it.
> + # if sys.version contains GCC then python was compiled with
> + # GCC, and the config.h file should be OK
> + try:
> + string.index(sys.version,"GCC")
> + return 2
> + except ValueError:
> + pass # go to the next test
> +
I don't like relying on exception-handling like this: I think I'm
superstitious about the cost of invoking an unnecessary exception.
Please recode using string.find().
> +
> UnixCCompiler.__init__ (self, verbose, dry_run, force)
>
> + if check_config_h()<=0:
> + self.warn(
> + "Python's config.h doesn't seem to support your compiler.")
Should this warning add something like, "expect compilation to die
horribly"?
> + # name of dll
> if not debug:
> - ext_name = os.path.basename(output_filename)[:-len(".pyd")]
> + dll_name = os.path.basename(output_filename)[:-len(".pyd")]
> else:
> - ext_name = os.path.basename(output_filename)[:-len("_d.pyd")]
> + dll_name = os.path.basename(output_filename)[:-len("_d.pyd")]
More Python-specific stuff. Note that all this is already done for you
in build_ext.py: see 'get_ext_filename()'. As long as it's being called
appropriately, the compiler class doesn't need to figure out what the
name of the file it's going to create.
> + # where are the object files
> + temp_dir=os.path.dirname(objects[0])
What about the 'build_temp' parameter?
> + if export_symbols != None and type(export_symbols) == StringType:
> + def_file = export_symbol_file
Minor quibble: I use "is" for comparing to None and type objects. I
know it makes no difference, but that's the Distutils Coding Standard.
So there. ;-)
This concern will go away if we stop supporting 'export_symbol_file',
but 'export_symbols' should always be a list. I hate the "if it's a
string, it's a filename, if it's a list, it's a list of symbols"
distinction. Makes the code tricky to follow.
> + # Make .def file
> + # (It would probably better to check if we really need this,
> + # but for this we had to insert some unchanged parts of
> + # UnixCCompiler, and this is not what we want.)
> + if type(export_symbols) == ListType :
> + def_file = os.path.join(temp_dir, dll_name + ".def")
> + f = open(def_file,"w")
> + f.write("EXPORTS\n") # intro
> + for sym in export_symbols:
> + f.write(sym+"\n")
> + f.close()
Is it necessary to make a .def file if there are multiple export
symbols? Or can they go on the command line? (I think I'd rather put
them on the command-line: more explicit, and one less temp file to worry
about.)
> - self.export_symbols = export_symbols or []
> + self.export_symbols = export_symbols
I understand the reason for this change, but we need to make sure there
isn't code out there that relies on export_symbols always being a list
(ie. never None).
> --- distutils.orig/distutils/msvccompiler.py Mon Jul 3 12:33:44 2000
> +++ dp/distutils/msvccompiler.py Tue Jul 4 18:06:50 2000
> @@ -374,11 +374,25 @@
[...]
> + if export_symbols != None and type(export_symbols) == StringType:
> + export_opts.append('/DEF:' + export_symbols)
"is" again, and again I express distaste for this sort of
type-sensitivity.
> + # The MSVC linker generates .lib and .exp files, which cannot be
> + # suppressed by any linker switches. The .lib files may even be
> + # needed! Make sure they are generated in the temporary build
> + # directory. Since they have different names for debug and release
> + # builds, they can go into the same directory.
> + dll_name, dll_ext = os.path.splitext(os.path.basename(output_filename))
> + implib_file = os.path.join (
> + os.path.dirname(objects[0]),
> + self.static_lib_format % (dll_name, self.static_lib_extension))
> + ld_args.append ('/IMPLIB:' + implib_file)
Ahh, is this the code that's supposed to replace the last stanza of
'msvc_prelink_hack()'? If so, why isn't it putting the "implib" file in
'build_temp'?
...oh of course, it goes there anyways, because that's where object
files go. This just respects the convention of preserving the source
tree structure under build_temp. I take it all back; I think this is
right.
Can you regenerate your patch to take the above comments into account
and resubmit it? Thanks a bunch!
Greg
--
Greg Ward - just another Python hacker gward@python.net
http://starship.python.net/~gward/
All of life is a blur of Republicans and meat!