[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