[LLVMdev] [patch] EXPORTED_SYMBOL_FILE using mingw and cmake
Mueller-Roemer, Johannes Sebastian
Johannes.Sebastian.Mueller-Roemer at igd.fraunhofer.de
Tue Jul 22 23:56:08 PDT 2014
Changing it to only apply to the MinGW-Makefiles generator is maybe the safest method. Though you would have to check if MINGW is only set in the Mingw-Makefiles generator and not In the MSYS-Makefiles or Cygwin generators (should be in the docs).
--
Johannes S. Mueller-Roemer, MSc
Wiss. Mitarbeiter - Interactive Engineering Technologies (IET)
Fraunhofer-Institut für Graphische Datenverarbeitung IGD
Fraunhoferstr. 5 | 64283 Darmstadt | Germany
Tel +49 6151 155-606 | Fax +49 6151 155-139
johannes.mueller-roemer at igd.fraunhofer.de | www.igd.fraunhofer.de
-----Original Message-----
From: Dan Liew [mailto:dan at su-root.co.uk]
Sent: Monday, July 21, 2014 12:44
To: Mueller-Roemer, Johannes Sebastian
Cc: llvmdev at cs.uiuc.edu; Brad King
Subject: Re: [LLVMdev] [patch] EXPORTED_SYMBOL_FILE using mingw and cmake
@ Brad : This might be a bug worth taking a look at
Hi Johannes,
Thanks for brining this to my attention.
> this is my first post to this list, so please excuse if submitting a
> patch without previous discussion is considered bad form or anything similar.
It's fine to submit a patch without discussion first if it's minor change like this one, but usually they should normally go to the llvm-commits mailing list instead of llvmdev. I'll try to review it anyway.
> I
> encountered a bug in the CMake build while using MinGW (non-MSYS,
> non-CYGWIN) where the LTO_export fails with a “The syntax of the
> command is incorrect” error. This error was previously fixed for
> Windows in general using TO_NATIVE_PATH, however CMake has a known bug
> (
> http://public.kitware.com/Bug/print_bug_page.php?bug_id=5939 ) where
> TO_NATIVE_PATH does not replace slashes by backslashes when the MinGW
> Makefiles generator is used. The attached patch provides a workaround.
> Considering that the bug has been open since 2007 it is unlikely that
> kitware will fix this anytime soon.
Yes considering the age of that bug report the workaround is probably the best thing to do.
At a glance this patch looks okay but I would change it slightly so
- In the comments have a link to the bug you are working around
- Quote "${export_file}" so we can handle paths with spaces in (I realise the old code didn't do that, but that is probably a mistake).
Also could you use MINGW in your conditional like this so you only workaround the bug when necessary instead of all Windows platforms?
```
if(MINGW)
# mingw workaround, as TO_NATIVE_PATH is implemented incorrectly
# See http://public.kitware.com/Bug/print_bug_page.php?bug_id=5939
string(REPLACE / \\ export_file_backslashes "${export_file}")
else()
file(TO_NATIVE_PATH "${export_file}" export_file_backslashes)
endif()
```
I may have got this slightly wrong though as I'm not familiar with MinGW.
Thanks,
--
Dan Liew
PhD Student - Imperial College London
More information about the llvm-dev
mailing list