This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2] Added file properties to windows gdb executable for all mingw32 builds.
- From: Eli Zaretskii <eliz at gnu dot org>
- To: Bernd Bunk <bernd dot bunk at intel dot com>
- Cc: tromey at redhat dot com, palves at redhat dot com, gdb-patches at sourceware dot org, bernd dot bunk at intel dot com
- Date: Thu, 22 Aug 2013 18:09:50 +0300
- Subject: Re: [PATCH v2] Added file properties to windows gdb executable for all mingw32 builds.
- References: <1377161766-8318-1-git-send-email-bernd dot bunk at intel dot com>
- Reply-to: Eli Zaretskii <eliz at gnu dot 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.