Build and Runtime failures under MinGW

[GoogleCodeExporter]

Ar rev.356 under MinGW, there are 2 build issues and runtime error.

a. Port.h tries to include fnmatch.h.  Windows code to workaround this exists, 
but is only included under MSVC.  I changed the #if to check for _WIN32 instead 
of _MSC_VER. Additionaly, added a dep to shlwapi since gcc doest have auto link.

b. For some reason, under windows with cmake 2.8.8, I had to change how the 
clang libraries are linked in to IWYU because they simply weren't linked in.

c. '\' chars are not changed to '/'.  Again, this only occurs under MinGW as 
the code to fix this is only included under MSVC.  Changed to #if to _WIN32.

Patch attached.

Original issue reported on code.google.com by c.d.glo...@gmail.com on 30 Jun 2012 at 4:22

Attachments:

• fnmatch_and_seperator_fix.patch

[GoogleCodeExporter]

Thanks for patch. s/_MSC_VER/_WIN32/ looks reasonable, but if macro _WIN32 is 
still present when you build 64-bit version?

There are a few changes I'd like to see in CMakeLists.txt, but there is already 
patch in issue #72 which I like more, so I won't bother you with these changes.

Original comment by vsap...@gmail.com on 1 Jul 2012 at 11:45

[GoogleCodeExporter]

Yes, _WIN32 *should* be defined for 64bit as well as it really just means "I'm 
using the Windows API". Under 64bit, you usually get _WIN64 as in addition to 
_WIN32.

Original comment by c.d.glo...@gmail.com on 2 Jul 2012 at 12:09

[GoogleCodeExporter]

I can confirm that; I just created a scratch project showing that both _WIN32 
and _WIN64 are defined for projects targetting x64.

I think Microsoft did that to preserve source compatibility -- _WIN32 has been 
used for years to distinguish Windows code.

Original comment by kim.gras...@gmail.com on 2 Jul 2012 at 5:00

[GoogleCodeExporter]

Thanks for sharing Windows knowledge. I've made a few changes to the patch, 
specifically:
 - Same shlwapi linking for Visual Studio and MinGW - removed # pragma comment(lib, "Shlwapi.lib"). You can find more details about this kind of pragmas at [1].
 - Removed add_definitions( /D_POSIX_=1 ). I understand that MinGW doesn't need it, so I wonder if Visual Studio really requires it. If nobody uses this definition, let's just remove it.

Chris, I understand that MinGW has no problems with #include <getopt.h>, 
#include <unistd.h>, am I right? I am asking because these headers aren't 
included #if defined(_MSC_VER).

I've attached edited patch. Chris, Kim, can you apply it and check IWYU builds 
under MinGW and Visual Studio?

[1] http://support.microsoft.com/kb/153901

Original comment by vsap...@gmail.com on 14 Jul 2012 at 9:14

• Changed state: Accepted

Attachments:

• fnmatch_and_separator_fix_edited.patch

[GoogleCodeExporter]

> removed # pragma comment(lib, "Shlwapi.lib")

This makes sense.

> I understand that MinGW has no problems with #include <getopt.h>, #include 
<unistd.h>, am I right?

Yes, any gnulib headers and standard posix headers are available under MinGW.

I tested MinGW under GCC 4.7, which worked perfectly.  Visual Studio 2010 
failed to build due to PATH_MAX not being defined as a side effect of 
undefining _POSIX_ .  So it looks like MSVC requires _POSIX_ to be defined.

Original comment by c.d.glo...@gmail.com on 15 Jul 2012 at 5:42

[GoogleCodeExporter]

Yes, this works with my Visual Studio setup if I add _POSIX_ back to 
CMakeLists.txt per Chris' suggestion;

if( MSVC )
   add_definitions( /D_POSIX_=1 )
endif()

Nitpicking: the CMake convention seems to be spaces after parens in 
if-statements, so "if( WIN32 )" looks more typical than "if (WIN32)".

Looks good otherwise!

Original comment by kim.gras...@gmail.com on 15 Jul 2012 at 7:43

[GoogleCodeExporter]

Committed changes in r361.

I've encountered both "if (smth)" and "if( smth )" in LLVM/Clang 
CMakeLists.txt. I didn't know what the convention is so I've decided to go with 
C++ style. But if you say that "if( smth )" is more typical, let's go with that 
style.

Chris, can you verify the last commit so I can close the issue?

Original comment by vsap...@gmail.com on 15 Jul 2012 at 9:03

[GoogleCodeExporter]

Verified r361.  Looks good on my end.

Original comment by c.d.glo...@gmail.com on 15 Jul 2012 at 6:11

[GoogleCodeExporter]

Original comment by vsap...@gmail.com on 15 Jul 2012 at 6:19

• Changed state: Verified