[PATCH v2] Added file properties to windows gdb executable for all mingw32 builds.
Pedro Alves
palves@redhat.com
Fri Aug 23 14:38:00 GMT 2013
On 08/23/2013 03:05 PM, Bunk, Bernd wrote:
>> -----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.
Well, it just looks to me that the gdbres.rc resource
and the icon are presently only used on Cygwin-hosts builds,
because nobody bothered to tweak that configure bit to make it work
on mingw-hosts too. That bit is there since at least 2000, and I
believe MinGW host support came in much after. Cygwin wanting
pretty icons, but MinGW not is kind of strange, wouldn't
you say? :-)
Do you have any hint at what one would need to do, if one
were to enable that icon resource on mingw too?
>
>>
>> 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]
I don't see any comment formatting problem in create-version.sh.
(btw, I meant "double space after period". Don't know
why I wrote "double period".)
>
>>
>>> + 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.
>
I mean this section:
http://sourceware.org/gdb/current/onlinedocs/gdbint/Start-of-New-Year-Procedure.html#Start-of-New-Year-Procedure
The internals manual has just recently all been dumped into the wiki:
http://sourceware.org/gdb/wiki/Internals%20Start-of-New-Year-Procedure
See "A new strategy for internals documentation" thread in gdb@.
>>
>>> +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]
Yes, see e.g., the copying.c rule just above the rule you added.
--
Pedro Alves
More information about the Gdb-patches
mailing list