[PATCH STABLE] setup: fix build_hgexe for mingw32 compiler

[Adrian Buehlmann]

On 2012-07-26 02:35, Mads Kiilerich wrote:
> Adrian Buehlmann wrote, On 07/25/2012 04:50 PM:
>> # HG changeset patch
>> # User Adrian Buehlmann <adrian at cadifra.com>
>> # Date 1343227822 -7200
>> # Branch stable
>> # Node ID 450330d3276f34576b98fd2d940c257f4bdef73b
>> # Parent  ffc49100151b68c0bcd061b900d9993e9e1a0d7d
>> setup: fix build_hgexe for mingw32 compiler
>>
>> Fixes
>>
>>    python setup.py build_hgexe -i --compiler=mingw32
>>
>> diff --git a/setup.py b/setup.py
>> --- a/setup.py
>> +++ b/setup.py
>> @@ -342,11 +342,17 @@
>>       def build_extensions(self):
>>           if os.name != 'nt':
>>               return
>> +        if isinstance(self.compiler, HackedMingw32CCompiler):
>> +            self.compiler.compiler_so = self.compiler.compiler # no -mdll
>> +            self.compiler.dll_libraries = [] # no -lmsrvc90
> 
> Why are these lines needed / nice to have? What bad consequences did 
> these options have? It seemed to work anyway, so why mess with them?

Hmm. The resulting hg.exe crashed here (Windows 7) without that change.
Without that change, gcc is called with -mdll, which looks wrong to me:

  $ C:\Python27_x86\python.exe setup.py build_hgexe -i --compiler=mingw32
  running build_hgexe
  C:\MinGW\bin\gcc.exe -mdll -O -Wall -IC:\Python27_x86\include
-IC:\Python27_x86\PC -c mercurial/exewrapper.c -o
build\temp.win32-2.7\Release\mercurial\exewrapper.o
  C:\MinGW\bin\gcc.exe -s
build\temp.win32-2.7\Release\mercurial\exewrapper.o
-LC:\Python27_x86\libs -LC:\Python27_x86\PCbuild -lmsvcr90 -o
C:\Users\adi\hgrepos\hg-main\hg.exe

build\temp.win32-2.7\Release\mercurial\exewrapper.o:exewrapper.c:(.text+0xfe):
undefined reference to `_imp__Py_Main'

build\temp.win32-2.7\Release\mercurial\exewrapper.o:exewrapper.c:(.text+0x154):
undefined reference to `_imp__Py_Main'
  collect2: ld gab 1 als Ende-Status zurück
  error: command 'gcc' failed with exit status 1

http://gcc.gnu.org/onlinedocs/gcc-4.6.2/gcc/i386-and-x86_002d64-Windows-Options.html#index-mdll-1396

"This option is available for Cygwin and MinGW targets. It specifies
that a DLL - a dynamic link library - is to be generated".

I think we don't want a dll to be creeated here.

Same for -lmsvcr90 (which is a typo in my comment): the resulting hg.exe
crashed here with that option set.

The resulting hg.exe works fine here with the patch as Matt pushed it.

Building the hg.exe goes like this here:

  $ C:\Python27_x86\python.exe setup.py build_hgexe -i --compiler=mingw32
  running build_hgexe
  creating build
  creating build\temp.win32-2.7
  creating build\temp.win32-2.7\Release
  creating build\temp.win32-2.7\Release\mercurial
  C:\MinGW\bin\gcc.exe -O -Wall -IC:\Python27_x86\include
-IC:\Python27_x86\PC -c mercurial/exewrapper.c -o
build\temp.win32-2.7\Release\mercurial\exewrapper.o
  C:\MinGW\bin\gcc.exe -s
build\temp.win32-2.7\Release\mercurial\exewrapper.o
-LC:\Python27_x86\libs -LC:\Python27_x86\PCbuild -lpython27 -o
C:\Users\adi\hgrepos\hg-main\hg.exe

and the resulting hg.exe doesn't crash here.

> And why is it done here and not in HackedMingw32CCompiler?

Because it's only wanted when building the hg.exe, not when building the
C extensions.

Note that subclassing buildhgexe from build_ext (as I did in setup.py),
is doubtlessly an ugly hack, but I didn't see an equally quick and
convenient alternative that works equally well for both the MS toolchain
and gcc.

We could have easily done the gcc calls in the makefile for sure, but
for example, using setup.py has the advantage of inferring the library
name to be used (e.g. python27.lib) from the version of the python
interpreter that was used to invoke setup.py.

> It seems like 
> it is touching a long lived 'compiler' instance

Does it? I thought that this was a separate invocation anyway.

But it could well be that I made an error here, without beeing affected
by the (suspected) bad consequences.

> and the change is thus not necessarily local anyway.

That would be bad indeed. I'll look into that.

>>           objects = self.compiler.compile(['mercurial/exewrapper.c'],
>>                                            output_dir=self.build_temp)
>>           dir = os.path.dirname(self.get_ext_fullpath('dummy'))
>>           target = os.path.join(dir, 'hg')
>> +        pythonlib = ("python%d%d" %
>> +               (sys.hexversion >> 24, (sys.hexversion >> 16) & 0xff))
>>           self.compiler.link_executable(objects, target,
>> +                                      libraries=[pythonlib],
>>                                         output_dir=self.build_temp)
> 
> Yes, there is no doubt about this part - it fixes
>    undefined reference to `_imp__Py_Main'
> link errors.

Yes. That change is needed to let the linking step pass, but it was not
sufficient to also run the hg.exe crash free here (see above).