[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