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: "Bunk, Bernd" <bernd dot bunk at intel dot com>
- To: Pedro Alves <palves at redhat dot com>, Eli Zaretskii <eliz at gnu dot org>
- Cc: "tromey at redhat dot com" <tromey at redhat dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Fri, 23 Aug 2013 14:05:30 +0000
- 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> <8361uxkbi9 dot fsf at gnu dot org> <52166564 dot 90605 at redhat dot com>
> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Thursday, August 22, 2013 9:24 PM
> To: Eli Zaretskii
> Cc: Bunk, Bernd; tromey@redhat.com; gdb-patches@sourceware.org
> Subject: Re: [PATCH v2] Added file properties to windows gdb executable
> for all mingw32 builds.
>
> On 08/22/2013 04:09 PM, Eli Zaretskii wrote:
>
> > 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.
>
> I'd think this works just as well on Cygwin binaries? I suggest
> windows-*.
I did not implement this for Cygwin for several reasons:
First I have no means to test that here and did not want to submit untested features or even break other builds.
And second I detected that there is a Cygwin based build (gdbtk) that has a resource (with an icon) build and linked into a windows gdb binary. I did not want to interfere with that (again no way to test it).
I could extend my implementation to all windows host builds, but TBH setting up a Cygwin environment (inside our build environment) is a huge effort that is overkill for this little feature.
>
> "properties" sounds odd to me, given files are usually called resource
> files. IIRC, the frequent file name used in lots of projects is
> winres.rc. But then, if you have more than one program in your
> project, you'd go with gdbres.rc, foores.rc, etc. Which leads me to
> ... We already have some support for including resources (icon, etc.)
> in the binary, for gdbtk.
> See gdb/configure.ac, gdb/Makefile.in and gdbtk/gdb.rc etc.
> Isn't this going to conflict on gdbtk builds?
See above, yes it would.
>
> While at it, follows a review of the patch.
>
> > +# shell parameters
> ...
>
> > +
> > +# default option values
> > +# keep these defaults in sync with gdb/top.c print_gdb_version()
>
> ...
>
> > +# check for environment variables to replace certain file properties
>
> Please make the comments follow the GNU style. That is, Start
> sentences with upper case, and end them with a double period.
Ok, I found only one shell script in that area (common/create-version.sh) and used that as code example. But I can adapt the comment/coding style - no problem [see next update]
>
> > + Patch this header file using create-win_exe_properties.sh during
> gdb build
> > + to customize the file properties. */
>
> Likewise, double period. There may be more instances.
>
> > +# keep these defaults in sync with gdb/top.c print_gdb_version()
> ...
> > +copyright="Copyright (C) 2013 Free Software Foundation, Inc."
>
> That "keep in sync" comment is useless, as nobody will remember to look
> here when looking at top.c. Instead, remove that, and you'll need to
> update the "Start of New Year Procedure" section in the internals
> manual. Guess that means in the wiki now, once this patch is in.
Ok, I remove that command, but will need to investigate what exactly this means. But changing a wiki afterwards does not sound so difficult.
>
> > +win_exe_properties.h: Makefile version.in common/create-
> win_exe_properties.sh
> > + $(SHELL) $(srcdir)/common/create-win_exe_properties.sh $(srcdir)
> \
> > + "$(host_alias)" "$(target_alias)" win_exe_properties.h
>
> It's better to write to a temporary file, and then move to the final
> destination, so you don't end up with a half baked file if you cancel
> the build at the wrong time.
Ok, didn't think about this - good catch! So I will follow the version file approach and create a temp (header) file inside the build folder, not inside the $(srcdir) folder. [see next update]
>
> --
> Pedro Alves
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052