[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!