[PATCH] Embed Windows version resource information into our exe and dll files

[Aaron Ballman]

On Mon, Feb 23, 2015 at 9:00 AM, Greg Bedwell <greg_bedwell at sn.scee.net> wrote:
> Hi aaron.ballman, chapuni, rafael,
>
> As part of the PlayStation(R)4 SDK we have a requirement to embed Windows version information into executables/DLLs that we ship.  I'd like to upstream our local change to enable this for the LLVM and related tools.  Assuming that the basic idea is acceptable I'm expecting that we might need to go through a few iterations on this but hopefully the attached patch can serve as a basis for any related discussions.
>
> The most visible place that you can find Windows version information is by looking at the file properties in Windows Explorer under the details tab.  It's also embedded into dmp files (a subsequent change that I plan to submit will add the capability of automatically generating these files in the case of crashes on Windows)
>
> The basis of the patch is the addition of the new file 'windows_version_resource.rc'.  This is a resource script as described here:
> http://msdn.microsoft.com/en-gb/library/windows/desktop/aa380599(v=vs.85).aspx
>
> It defines a VERSIONINFO resource as defined here:
> https://msdn.microsoft.com/en-gb/library/windows/desktop/aa381058(v=vs.85).aspx
>
> There is a new CMake function 'add_version_info_on_windows' which is called from add_llvm_executable and llvm_add_library in the case of DLLs.  This will add the resource script to these projects setting the values based on the LLVM_VERSION_* CMake values.  Note that this function is a no-op except for MSVC builds.  It should be possible, in theory, to also enable it for MinGW builds using windres.exe but the MinGW CMake generator doesn't seem to support it out of the box as the Visual Studio one does so in the interest of simplicity I've left it as MSVC only.

In general, I like the way this is going. Thank you for tackling it!

> 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?

> A few points I'd like to draw attention to:
> - the location of windows_version_resource.rc:  Currently it's at the root llvm directory level but I'm not sure if this is appropriate.  Non of the existing directories seem to apply (possibly 'cmake' at a stretch?).  Should I instead make a new one?  If so, would 'resource' be a reasonable name?

I think it should be a new directory named "resources."

> - Testing:  We test this in our internal test suite using a Python script.  The script itself is simple enough but requires the use of the win32api module which is an extension.  Although it comes with some Python distributions on Windows by default, this is not guaranteed.  Any thoughts on how best to test this would be greatly appreciated.
>
> - The Windows FILEVERSION format:  This is made of four 16-bit integers x,y,z,w.  I think it makes sense for x,y and z to be major, minor and patchlevel numbers respectively, but there is a question over w.  Many software projects on Windows will use a unique build number here, so using the SVN revision would make sense in this case but one issue is that LLVM SVN revision numbers are way larger than can be represented in a 16-bit number so get truncated.  The attached patch will embed the revision number if available, so we end up with a situation where, for example the file properties in Windows explorer show "Product Version" (a string) as "3.7.0svn-r230211" and "File Version" (a number) as "3.7.0.33603".  The alternative would be for "File Version" to be set to "3.7.0.0" (major, minor, patch, zero).  I don't have a strong preference either way, as internally we'll override all these numbers with our own PS4 SDK version number, so I'm happy to go with the consensus.

I think the file version (number) should be 3.7.0.0, but agree that
the product version (string) should include the svn revision
information.

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).

Thanks!

~Aaron