Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mingw build fail caused by c57f8a4 #421

Closed
wants to merge 1 commit into from
Closed

Fix mingw build fail caused by c57f8a4 #421

wants to merge 1 commit into from

Conversation

jonforums
Copy link
Contributor

c57f8a4 changes to src/ninja.cc break the mingw builds due to incorrect macro usage. MinGW based toolchains define _WIN32 and WIN32 so _MSC_VER should be used to determine if building with VC's cl and friends.

FYI, if you're doing Windows patches and need an easy, all-in-one MinGW toolchain to test with, I maintain a few different flavors of "DevKit's" at TheCodeShop download page


C:\Users\Jon\Documents\CDev\ninja-git>git log -3 --oneline
d95fcb0 Merge pull request #416 from nico/slide
a79de82 Merge pull request #415 from nico/getopt
48143c9 Change rate measurement code.

C:\Users\Jon\Documents\CDev\ninja-git>ninja
[19/20] AR build\libninja.a
        1 file(s) moved.
[20/20] LINK ninja.exe
FAILED: g++ -Lbuild -static -o ninja.exe build\ninja.o -lninja
build\ninja.o: In function `ToolMSVC':
C:\Users\Jon\Documents\CDev\ninja-git/src/ninja.cc:301: undefined reference to `MSVCHelperMain(int, char**)'
collect2.exe: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.

This occurs because the build.ninja rule for libninja.a (correctly) doesn't include the VC needed parts.


// build.ninja generated from `python configure.py --platform=mingw`
...
build $builddir\libninja.a: ar $builddir\build.o $builddir\build_log.o $
    $builddir\clean.o $builddir\depfile_parser.o $builddir\disk_interface.o $
    $builddir\edit_distance.o $builddir\eval_env.o $builddir\explain.o $
    $builddir\graph.o $builddir\graphviz.o $builddir\lexer.o $
    $builddir\manifest_parser.o $builddir\metrics.o $builddir\state.o $
    $builddir\util.o $builddir\subprocess-win32.o $builddir\getopt.o

# Main executable is library plus main() function.
build $builddir\ninja.o: cxx src\ninja.cc
build ninja: phony ninja.exe
build ninja.exe: link $builddir\ninja.o | $builddir\libninja.a
  libs = -lninja

This patch works for me using both mingw-based GCC 4.7.1 and WinSDK 7.1 toolchains on my Win7 32bit system. Below is the output from building using WinSDK 7.1 cmd line tools:


C:\Users\Jon\Documents\CDev\ninja-git>cl /?
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 16.00.30319.01 for 80x86

# dammit, only VC IDE sets `VCINSTALLDIR`...hack it so `bootstrap.py` works for WinSDK 7.1
C:\Users\Jon\Documents\CDev\ninja-git>set VCINSTALLDIR=C:\Program Files\Microsoft Visual Studio 10.0\VC\

C:\Users\Jon\Documents\CDev\ninja-git>python bootstrap.py
Building ninja manually...
build.cc
build_log.cc
clean.cc
depfile_parser.cc
disk_interface.cc
edit_distance.cc
eval_env.cc
explain.cc
graph.cc
graphviz.cc
includes_normalize-win32.cc
lexer.cc
manifest_parser.cc
metrics.cc
minidump-win32.cc
msvc_helper-win32.cc
msvc_helper_main-win32.cc
ninja.cc
state.cc
subprocess-win32.cc
Generating Code...
Compiling...
util.cc
Generating Code...
Compiling...
getopt.c
Generating Code...
Building ninja using itself...
warning: A compatible version of re2c (>= 0.11.3) was not found; changes to src/*.in.cc wi
ll not affect your build.
wrote build.ninja.
[24/24] LINK ninja.exe
Generating code
Finished generating code

Done!

Note: to work around Windows file locking, where you can't rebuild an
in-use binary, to run ninja after making any changes to build ninja itself
you should run ninja.bootstrap instead.  Your build is also configured to
use ninja.bootstrap.exe as the MSVC helper; see the --with-ninja flag of
the --help output of configure.py.

@buildhive
Copy link

Evan Martin » ninja #170 SUCCESS
This pull request looks good
(what's this?)

@jonforums
Copy link
Contributor Author

Looking over this again, my patch is the wrong solution. MinGW builds should have the -t msvc superpowers too.

Please reject and close this pull request as I now believe the real fix should be to configure.py so libninja.a has more goodies. Looks like mods should start at

https://github.com/martine/ninja/blob/master/configure.py#L279

If I get time later this week I'll take a run at another pull request.

That said, please do use my mingw/mingw-w64 based DevKit's from TheCodeShop to test out Windows mods 😈

@nico
Copy link
Collaborator

nico commented Sep 21, 2012

(I think you can close this pull request yourself.)

@jonforums
Copy link
Contributor Author

Closing; fix superseded by #427

@jonforums jonforums closed this Sep 21, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants