<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Hi Aaron,</div><div><br></div><div>Thanks for looking at this!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> 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.<br>
<br>
</span>If I understand properly, I think these changes are orthogonal to the<br>
version resource patch and can be split out and commit separately<br>
since they're a strict improvement to our CMake build system?<br><span class=""><br></span></blockquote><div><br></div><div>Okay.  I'll split this part out and create a separate review.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I don't see anything incorrect with the patch, but am not a CMake<br>
expert either. Do have one minor nit though:<br>
<br>
> +#ifndef RC_ORIGINAL_FILENAME<br>
> +#define RC_ORIGINAL_FILENAME ""<br>
> +#endif<br>
> +<br>
> +#ifndef RC_INTERNAL_NAME<br>
> +#define RC_INTERNAL_NAME ""<br>
> +#endif<br>
<br>
I think those should be set to the name of the file being compiled<br>
(with extension).<br></blockquote><div><br></div><div>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.</div><div><br></div><div>Thanks again!</div><div><br></div><div>-Greg <br></div></div></div></div>