[PATCH] Embed Windows version resource information into our exe and dll files
Greg Bedwell
gregbedwell at gmail.com
Thu Feb 26 10:08:22 PST 2015
Hi Aaron,
Thanks for looking at this!
> > I've had to make small change to HandleLLVMOptions.cmake to support the
> CMake Ninja generator. Previously the MSVC-specific warning flags were
> being added from add_llvm_definitions. The Visual Studio generators use a
> filter to make sure that only valid flags are passed through to the
> resource compiler, however the Ninja generator expects that this will only
> ever contain definitions and passes everything through unfiltered leading
> to errors from the resource compiler when given invalid flags. I've
> changed the behaviour so these warning flags are now added to CMAKE_C_FLAGS
> and CMAKE_CXX_FLAGS directly.
>
> If I understand properly, I think these changes are orthogonal to the
> version resource patch and can be split out and commit separately
> since they're a strict improvement to our CMake build system?
>
>
Okay. I'll split this part out and create a separate review.
I don't see anything incorrect with the patch, but am not a CMake
> expert either. Do have one minor nit though:
>
> > +#ifndef RC_ORIGINAL_FILENAME
> > +#define RC_ORIGINAL_FILENAME ""
> > +#endif
> > +
> > +#ifndef RC_INTERNAL_NAME
> > +#define RC_INTERNAL_NAME ""
> > +#endif
>
> I think those should be set to the name of the file being compiled
> (with extension).
>
I'll look into this. I'm not convinced that the information is actually
available at that point but I've got a couple of ideas that may work. I'll
try to get an updated patch up soon.
Thanks again!
-Greg
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150226/28b7aca4/attachment.html>
More information about the llvm-commits
mailing list