<div dir="ltr"><div>Hi Aaron,</div><div><br></div><div>Thanks for the reply.</div><div> </div><div>> The patch goes in the right direction, but we have a lot of other</div><div>> executables that we want this for, right? Such as all the tools that</div><div>> come with llvm? (I realize this is the Clang patch, but I figured this</div><div>> would hit all the .exe files that would be distributed to users and</div><div>> run as part of their toolchains.)</div><div>></div><div><br></div><div>To be clear the LLVM-side patch sets 'default' LLVM values for the various file properties.  This will apply to all files that inherit from the top-level LLVM CMakelists file unless they are overridden later on by another file in the CMake hierarchy which is what the clang-side patch does.  So as it is with these patches currently, everything in the LLVM tree will get LLVM values except for anything coming from tools/clang which will get the clang default values.  I've attached a small image of the properties windows of a few .exe and .dll files from both clang and LLVM to show the difference (hopefully that's okay on the lists!  the old days of usenet still make we wary of binary attachments.  I tried to make it smaller than some patch attachments though :)).</div><div><br></div><div><img src="cid:ii_14d9ac46ff17fe97" alt="Inline images 1" width="473" height="166" style="margin-right: 0px;"><br></div><div class="gmail_extra"><br></div><div class="gmail_extra">To answer your question from your other reply:</div><div class="gmail_extra"><br><div class="gmail_quote">> What about on Windows building with something like MinGW? Should we do<br>> this then as well?<br></div><div class="gmail_quote"><br></div><div class="gmail_quote">Ideally yes, although in practice it seems to be much harder as the MinGW CMake generator doesn't seem to support this out of the box from my experimentation.  I'm happy to try and make it work too but I'd rather address it in a separate commit later if that's okay.  Should I add a TODO comment to reflect this?</div><div class="gmail_quote"><br></div><div class="gmail_quote">Otherwise, do these look ready to commit?</div><div class="gmail_quote"><br></div><div class="gmail_quote">Thanks,</div><div class="gmail_quote">-Greg</div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On 1 May 2015 at 15:03, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron.ballman@gmail.com" target="_blank">aaron.ballman@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed, Apr 29, 2015 at 1:52 PM, Greg Bedwell <<a href="mailto:gregbedwell@gmail.com">gregbedwell@gmail.com</a>> wrote:<br>
> Here's a clang side patch that overrides the LLVM version numbers with their<br>
> clang equivalents for anything from tools/clang.  I couldn't figure out how<br>
> to put both patches in Phabricator at the same time without lumping them<br>
> into a single patch file but I wanted them both available to provide context<br>
> to each other.  I'm not aware of any other in-tree projects that potentially<br>
> use their own versioning scheme.<br>
<br>
</span>The patch goes in the right direction, but we have a lot of other<br>
executables that we want this for, right? Such as all the tools that<br>
come with llvm? (I realize this is the Clang patch, but I figured this<br>
would hit all the .exe files that would be distributed to users and<br>
run as part of their toolchains.)<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
><br>
> Hopefully this explains the reason for the interface of<br>
> set_windows_version_resource_properties on the LLVM side.  I went with<br>
> "$CLANG_VERSION ($BACKEND_PACKAGE_STRING)" for the VERSION_STRING fields as<br>
> it seemed to give more useful information about the origin of the build<br>
> (especially if LLVM_APPEND_VC_REV is enabled which makes it include the SVN<br>
> revision number) but I'm happy just to use $CLANG_VERSION only or whatever<br>
> else is preferred.<br>
><br>
> A further enhancement could be to populate the FileDescription field for any<br>
> files that we feel would benefit (presumably user-facing files such as<br>
> clang.exe and friends) so that it will show up in the Windows Task Manager<br>
> when running which feels generally polite to do.  I'm happy to do this<br>
> if/when these changes land :-).<br>
><br>
> Thanks,<br>
> Greg<br>
><br>
> On 29 April 2015 at 18:22, Greg Bedwell <<a href="mailto:greg_bedwell@sn.scee.net">greg_bedwell@sn.scee.net</a>> wrote:<br>
>><br>
>> New patch attached.  Apologies for the delay.  I had to work on something<br>
>> else temporarily but am now free to hack on compilers once more - hooray!.<br>
>><br>
>> For the benefit of cfe-commits who I've just added, this patch adds a<br>
>> VERSIONINFO resource to .exe and .dll files when we build LLVM with MSVC.<br>
>> The most visible place that you can find Windows version information is by<br>
>> looking at the file properties in Windows Explorer under the details tab.<br>
>> It's also embedded into minidump .dmp files (a subsequent change that I plan<br>
>> to submit will add the capability of automatically generating these files in<br>
>> the case of crashes on Windows).<br>
>><br>
>> As suggested I separated the part that fixed an issue with the Ninja<br>
>> builder into another patch which was committed at r232727.<br>
>><br>
>> I've split the CMake function from the original patch into 2 functions<br>
>> now.  The first just adds the resource script file to the current project if<br>
>> we're building with MSVC.  The second checks whether the resource script<br>
>> exists in the current project and sets preprocessor macros for that file<br>
>> containing the version information to embed if it does.  There are a couple<br>
>> of reasons for splitting it.  Firstly, we need to add the resource script<br>
>> file to the project sources before the target has been created, but in order<br>
>> to populate the 'OriginalFilename' field we need the target location which<br>
>> only exists once the target has been created.  Secondly, it means that we<br>
>> can call the 'set_windows_version_resource_properties' function from<br>
>> specific subproject CMakelists files to override the default values if<br>
>> necessary.<br>
>><br>
>> By default we're setting the FILEVERSION field with the following values<br>
>> from CMake:<br>
>><br>
>>   LLVM_VERSION_MAJOR, LLVM_VERSION_MINOR, LLVM_VERSION_PATCH, 0<br>
>><br>
>> The "FileVersion" and "ProductVersion" fields which are strings get set to<br>
>> the value of PACKAGE_VERSION.<br>
>> The "OriginalFilename" field gets set with the name of the exe or DLL file<br>
>> that the project creates.  Note that this means that files such as<br>
>> clang++.exe or clang-cl.exe will still get the string 'clang.exe' in this<br>
>> field as they are just copies of clang.exe.  "InternalName" as per the<br>
>> Microsoft documentation suggestion gets set to the filename string without<br>
>> the extension (so 'clang' in the above cases).  I've set 'ProductName' to<br>
>> 'LLVM' by default.<br>
>><br>
>> I'll followup with a separate clang patch very shortly that overrides the<br>
>> LLVM version numbers with the clang equivalent ones and sets 'ProductName'<br>
>> to 'clang' when building projects that come from tools/clang.  I'm happy to<br>
>> bikeshed on the strings that particular fields should get as long as the<br>
>> general approach is considered acceptable.<br>
>><br>
>> Thanks,<br>
>> Greg<br>
>><br>
>><br>
>> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D7828&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=ZblE_52bRcpZXO0yRH5OFug086J9WN5_Dy5akbh01rg&s=dtVM6YHFvKtUYm8LJSXzNWY9PEtDRNS_bLefjIaXnP4&e=" target="_blank">http://reviews.llvm.org/D7828</a><br>
>><br>
>> Files:<br>
>>   cmake/modules/AddLLVM.cmake<br>
>>   resources/windows_version_resource.rc<br>
>><br>
>> EMAIL PREFERENCES<br>
>>   <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=ZblE_52bRcpZXO0yRH5OFug086J9WN5_Dy5akbh01rg&s=u29zwaKlim2JMxGkVouVLyccBmvGBULfgYehT-gU3go&e=" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
>><br>
>> _______________________________________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
>><br>
><br>
</div></div></blockquote></div><br></div>