Re: [PATCH v2] Added file properties to windows gdb executable for all mingw32 builds.

[Eli Zaretskii <eliz@gnu.org>]

> From: Bernd Bunk <bernd.bunk@intel.com>
> Cc: gdb-patches@sourceware.org, Bernd Bunk <bernd.bunk@intel.com>
> Date: Thu, 22 Aug 2013 10:56:06 +0200
> 
> On Windows OS it is customary that executables and DLL's show file properties.
> These contain product name/version, file name/version, company/copyright info.
> File properties are visible by right-click on the file, selecting properties \ details.
> This patch adds file properties to the gdb executable for all mingw builds.

Thanks, this looks OK to me, except for a couple of comments:

> 	* Makefile.in (win_exe_properties.h): Add rule to create
> 	win_exe_properties.h header file with file property information.
> 	(win_exe_properties.o): Added rule to build the resource file.
> 	* configure: Add win_exe_properties.o to mingw32 specific
> 	objects.
> 	* common/create-win_exe_properties.sh: Shell script to
> 	create a win_exe_properties.h header file containing all
> 	information/defines for the gdb executable file properties.
> 	New file.
> 	* win_exe_properties.rc: Resource file implementing
> 	gdb executable file properties for windows/mingw32 builds.
> 	It includes auto-generated win_exe_properties.h file with
> 	property information. New file.

New files you add should say "New file".  Also, please leave two
spaces between sentences, the US style.

The GNU project doesn't like calling Windows a "win", so I suggest to
rename the files and the script to use something like mingw instead.

> +# check for environment variables to replace certain file properties
> +[ -n "$WIN_EXE_VERSION" ] && version=$WIN_EXE_VERSION
> +[ -n "$WIN_EXE_COMPANY_NAME" ] && company_name=$WIN_EXE_COMPANY_NAME
> +[ -n "$WIN_EXE_FILE_DESCRIPTION" ] && file_description=$WIN_EXE_FILE_DESCRIPTION
> +[ -n "$WIN_EXE_PRODUCT_NAME" ] && product_name=$WIN_EXE_PRODUCT_NAME
> +[ -n "$WIN_EXE_INTERNAL_NAME" ] && internal_name=$WIN_EXE_INTERNAL_NAME
> +[ -n "$WIN_EXE_ORIGINAL_FILENAME" ] && original_filename=$WIN_EXE_ORIGINAL_FILENAME
> +[ -n "$WIN_EXE_COPYRIGHT" ] && copyright=$WIN_EXE_COPYRIGHT
> +[ -n "$WIN_EXE_LICENSE" ] && license=$WIN_EXE_LICENSE
> +[ -n "$WIN_EXE_CONFIGURED" ] && configured=$WIN_EXE_CONFIGURED
> +[ -n "$WIN_EXE_SUPPORT" ] && support=$WIN_EXE_SUPPORT

This looks like unnecessary featurism to me.  Is it really needed, and
if so, in what use cases?
> +#include "afxres.h"

Is this header really needed?

Thanks.