Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: "Bunk, Bernd" <bernd.bunk@intel.com>
Cc: Eli Zaretskii <eliz@gnu.org>,
	"tromey@redhat.com" <tromey@redhat.com>,
	       "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH v2] Added file properties to windows gdb executable for all mingw32 builds.
Date: Mon, 26 Aug 2013 15:24:00 -0000	[thread overview]
Message-ID: <521B7339.5010308@redhat.com> (raw)
In-Reply-To: <E2C54CDFBA86B845B3B075E2B2042A9120256D28@IRSMSX106.ger.corp.intel.com>

On 08/26/2013 01:24 PM, Bunk, Bernd wrote:
>> -----Original Message-----
>> From: Eli Zaretskii [mailto:eliz@gnu.org]
>> Sent: Friday, August 23, 2013 4:37 PM
>> To: Bunk, Bernd
>> Cc: tromey@redhat.com; palves@redhat.com; gdb-patches@sourceware.org
>> Subject: Re: [PATCH v2] Added file properties to windows gdb executable
>> for all mingw32 builds.
>>
>>> From: "Bunk, Bernd" <bernd.bunk@intel.com>
>>> CC: "tromey@redhat.com" <tromey@redhat.com>, "palves@redhat.com"
>>> 	<palves@redhat.com>, "gdb-patches@sourceware.org"
>>> 	<gdb-patches@sourceware.org>
>>> Date: Fri, 23 Aug 2013 13:44:58 +0000
>>>
>>>>> +# 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?
>>> Yes, it is needed. Not in here, but for every company which
>> changes/adds and re-distributes gdb.
>>> I started this feature because our Product Validation does not like
>> binaries without legal information.
>>> And off course this is different depending on who ships the product.
>>> Without a way to change the strings the complete changeset would be
>> useless for me.
>>
>> You can always modify the source of these attributes, can't you?  It's
>> not like you change these strings several times a day, right?
> If I have to change/overwrite the sources for this feature just to use the feature, where would be the reason for me to upstream it?  I need parameters/env vars to customize the behavior during build, not changed sources that I need to merge for every branch I have.  The whole purpose of upstreaming a feature is to NOT have custom sources for this feature.

Environment variables are horrible for build things though.
It's easy to do "make" in one shell that has them set in one way,
then continue work in a different shell that doesn't have them
set in the same way, issue a "make", and end up with a resulting binary
that doesn't have the configuration you really wanted.  Environment
values are also invisible in build logs, which makes use of them for
these kinds of things very frowned upon.  A better solution is one that
avoids these kinds of issues, and that usually means adding a new configure
option instead.  Now, you have a bunch of variables, so it could e.g.,
be something like a comma/colon/whatever- separated list of properties
passed down with --enable-gdb-windows-file-properties=...; or
--enable-gdb-windows-file-properties=/path/to/file, which would
result in create-win_exe_properties.sh reading the values
from /path/to/file; or some other solution along these lines.

Another solution for the whole add-file-properties-to-gdb-binary idea
that crossed my mind is whether one can't just add/rewrite the file
properties in a post-link step (with windres+objcopy perhaps), instead
of having to bake this at gdb build time.  Such a mechanism/tool could
then be used for all binaries/programs in the toolchain, without having
to add resource files all around -- you're going to have to add this
at least for the whole of binutils/gdb/gcc, and probably to more places
too, right?

-- 
Pedro Alves


  reply	other threads:[~2013-08-26 15:24 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-22  8:56 Bernd Bunk
2013-08-22 15:09 ` Eli Zaretskii
2013-08-22 19:24   ` Pedro Alves
2013-08-22 19:55     ` Eli Zaretskii
2013-08-23 13:06       ` Pedro Alves
2013-08-23 13:27         ` Eli Zaretskii
2013-08-23 13:42           ` Pedro Alves
2013-08-23 14:09             ` Bunk, Bernd
2013-08-23 14:07     ` Bunk, Bernd
2013-08-23 14:38       ` Pedro Alves
2013-08-26 12:02         ` Bunk, Bernd
2013-08-26 15:42           ` Pedro Alves
2013-08-23 13:45   ` Bunk, Bernd
2013-08-23 14:20     ` asmwarrior
2013-08-23 14:56       ` Bunk, Bernd
2013-08-23 14:37     ` Eli Zaretskii
2013-08-26 12:25       ` Bunk, Bernd
2013-08-26 15:24         ` Pedro Alves [this message]
2013-08-26 12:34       ` Bunk, Bernd

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=521B7339.5010308@redhat.com \
    --to=palves@redhat.com \
    --cc=bernd.bunk@intel.com \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=tromey@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox