Greg Ward wrote:
On 27 July 2000, Rene Liebscher said:
* msvc.patch: changes msvccompiler.py and build_ext.py (You need its changes in build_ext.py to get the other two compilers work properly after applying their patches.)
OK, I've checked that one in. One problem: I don't like the fact that Extension.ext_symbols can now be None -- you took away the "or []" in the constructor, which is inconsistent with every other attribute of Extension. Since this class is just a pile of data without much behaviour, I value convenience -- don't have to check for None, just assume everything is a list -- over completeness. And I really dislike the inconsistency you've introduced.
I can see why None-ness is needed, though: in build_ext.get_export_symbols(), you can tell if 'export_symbols' was supplied at all by looking for None. But if an empty list was supplied, isn't that wrong? If export_symbols is an empty list, then we'll go ahead and build the .pyd with nothing in the export list, not even "init" + module_name. In the general-purpose library-building code (msvccompiler.py), this would be right: but this code is geared towards building Python extensions, and if a Python extension doesn't export its init function, what's the point?
I suggest changing build_ext.get_export_symbols() from this:
if ext.export_symbols is None: return [] else: return ext.export_symbols
to this:
initfunc_name = "init" + string.split(ext.name,'.')[-1] if initfunc_name not in ext.export_symbols: ext.export_symbols.append(initfunc_name)
which of course assumes that ext.export_symbols is a list, which will be valid as long as we put back the "or []" in Extension's constructor. Sound reasonable?
OK, I think you are right, if one wants to build none Python dll's he will not use build_ext to do this.
* bcpp.patch: changes bcppcompiler.py
OK, I'm going over this one now.
ld_args.extend([',',output_filename]) # no map file ld_args.extend([',']) # start libraries ld_args.extend([','])
This code seems fishy to me. First of all, why are you using 'extend([x])' when 'append(x)' would do the same thing? And second, are those commas really supposed to be whole words in the arg list? I don't know how things work in Windows, but in Unix doing that would mean that the compiler's argv list would look like
I used 'extend' to be able to write " ld_args.extend([',','some_other_param']) ", but I never needed it in some places and simply forgot to replace it with 'append' in the final patch.
[..., ',', 'foo.pyd', ',', ',', ...]
which is really strange. I suspect that really should be coded as
ld_args.append(",%s,," % output_filename)
which would make an argv of
[..., ',foo.pyd,,', ...]
which seems a) more sensible, and b) looks like the Borland C++ command lines I remember from 7-8 years ago.
So: which arguments really have to be separate words (ie. separate elements of the argv[] array that BCPP will ultimately have to process)? Does this question even make sense on a DOS/Windows environment, ie. is the argument delineation implicit in the 'spawn()' call preserved in the child process?
The commas separate different blocks of the command line, and I used them also to separate the command line building code in several blocks (objects, output filename, libraries, ... In the patch this block building is more obvious then in the current version where you already have put some blocks together.) It is not necessary to have these commas as separate words, but they don't really belong to a output filename or a library name, this is the other reason why I made them in separate words.
- def find_library_file (self, dirs, lib): - + def find_library_file (self, dirs, lib, debug=0): + # find library file
Umm, the 'debug' flag is not part of the official CCompiler interface to 'find_library_file()'. Is this really necessary? Should MSVCCompiler do it as well? If so, then the interface should change.
Yes, it should. Python comes also with libraries 'python??.lib' and 'python??_d.lib'. So it is probably a good idea to support it this way.
Looks like 'find_library_file()' is used in BCPPCompiler to give preference to bcpp_xxx.lib; is this really important? Where does it arise? Is this a common convention, or a Python-only convention? (Or a Liebscher-only convention? ;-) go down to the next explanation, it is about the same problem.
# either exchange python15.lib in the python libs directory against # a Borland-like one, or create one with name bcpp_python15.lib # there and remove the pragmas from config.h #libraries.append ('mypylib') libraries.append ('import32') libraries.append ('cw32mt')
I don't understand. You commented out the use of 'mypylib' and added the comment. Can you explain what's going on here, please? If we need to make changes in Python (1.6 or 2.0!) to support Borland C++ better, now's the time to do it. 'mypylib' is not a Borland standard library. It is a Borland-like python.lib. MSVC and Borland use different formats for their object files (COFF and OMF.) Now there comes also a 'python??' library from build_ext it should be also called 'python??.lib'. So you had to replace the standard 'python??.lib' by a Borland-like one. But what happens if you installed Python on a network drive and some users want to use MSVC and other BORLAND C++? Then you have to give one of these libraries another name. I would suggest 'bcpp_python??.lib'.
* conventions (see above) I have never heard of some convention how to handle libraries in the same directory if you want to use different compilers which need these libraries with the same name but different format. How to resolve this? I think it is the best if every compiler(class) first looks if there is a special version for and use then this special version. In this case bcpp_python??.lib if it exists and if not python??.lib. This would be also work with other compiler classes, for example I am also testing LCC-Win32, which uses then lcc_python??.lib . So in this sense it is a 'Liebscher-only convention', but it doesn't hurt users who replace their libraries (python??.lib, ...) with versions specific to their compilers. If you know a better way to handle this problem, let me know. * About the 'pragma' in config.h: In PC/config.h of the Python source there are following lines: ------------------------------------------- #ifndef USE_DL_EXPORT /* So nobody needs to specify the .lib in their Makefile any more */ #ifdef _DEBUG #pragma comment(lib,"python20_d.lib") #else #pragma comment(lib,"python20.lib") #endif #endif /* USE_DL_EXPORT */ ------------------------------------------- If we want to use bcpp_python20.lib instead of python20.lib, we had to change the first line to: ------------------------------------------ #if !defined(USE_DL_EXPORT) && defined(_MSC_VER) ------------------------------------------ So the Borland C++ compiler wouldn't try to load this incompatible library 'python20.lib' . It would make sense to change this in any case for the next version of Python, because it comes with an import library for MSVC, so this pragma should be visible only for MSVC. (We can't change the config.h file in existing installations this is why I put the comment in there. So if a user got problems he finds a hint where this could come from.)
* cygwin.patch: Other comments...
* why are {gcc,ld,dllwrap}_version class attributes? They are derived from the particular compiler used by a particular instance of the class, and when you set them in the constructor they become instance attributes. So why even define them as class attrs? Ok, I will change this.
* 'check_config_h()' really should return meaningful strings instead of magic numbers. Comment or no, any occurence of a bare integer in non-arithmetic circumstances is a magic number in my book, and I don't like magic numbers.
* you shouldn't write to stdout or stderr in a compiler class -- use the 'announce()' method. I should really add a 'debug_print()' method; that would be handy. If you add 'debug_print()' I will use it.
* I don't like using 'list(foo)' to make a copy of a list 'foo' -- either 'foo[:]' or 'copy(foo)' (from the standard copy module) is acceptable. Ok, I will change this too.
Kind regards Rene Liebscher