From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14204 invoked by alias); 23 Aug 2013 14:07:36 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 14186 invoked by uid 89); 23 Aug 2013 14:07:34 -0000 X-Spam-SWARE-Status: No, score=-7.8 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 Received: from mga03.intel.com (HELO mga03.intel.com) (143.182.124.21) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 23 Aug 2013 14:07:08 +0000 Received: from azsmga001.ch.intel.com ([10.2.17.19]) by azsmga101.ch.intel.com with ESMTP; 23 Aug 2013 07:07:04 -0700 X-ExtLoop1: 1 Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by azsmga001.ch.intel.com with ESMTP; 23 Aug 2013 07:07:02 -0700 Received: from irsmsx152.ger.corp.intel.com (163.33.192.66) by IRSMSX101.ger.corp.intel.com (163.33.3.153) with Microsoft SMTP Server (TLS) id 14.3.123.3; Fri, 23 Aug 2013 15:05:30 +0100 Received: from irsmsx106.ger.corp.intel.com ([169.254.8.233]) by IRSMSX152.ger.corp.intel.com ([169.254.6.33]) with mapi id 14.03.0123.003; Fri, 23 Aug 2013 15:05:30 +0100 From: "Bunk, Bernd" To: Pedro Alves , Eli Zaretskii CC: "tromey@redhat.com" , "gdb-patches@sourceware.org" Subject: RE: [PATCH v2] Added file properties to windows gdb executable for all mingw32 builds. Date: Fri, 23 Aug 2013 14:07:00 -0000 Message-ID: References: <1377161766-8318-1-git-send-email-bernd.bunk@intel.com> <8361uxkbi9.fsf@gnu.org> <52166564.90605@redhat.com> In-Reply-To: <52166564.90605@redhat.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2013-08/txt/msg00677.txt.bz2 > -----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. >=20 > On 08/22/2013 04:09 PM, Eli Zaretskii wrote: >=20 > > 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. >=20 > 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 settin= g up a Cygwin environment (inside our build environment) is a huge effort t= hat is overkill for this little feature. >=20 > "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. >=20 > While at it, follows a review of the patch. >=20 > > +# shell parameters > ... >=20 > > + > > +# default option values > > +# keep these defaults in sync with gdb/top.c print_gdb_version() >=20 > ... >=20 > > +# check for environment variables to replace certain file properties >=20 > 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) a= nd used that as code example. But I can adapt the comment/coding style - n= o problem [see next update] >=20 > > + Patch this header file using create-win_exe_properties.sh during > gdb build > > + to customize the file properties. */ >=20 > Likewise, double period. There may be more instances. >=20 > > +# keep these defaults in sync with gdb/top.c print_gdb_version() > ... > > +copyright=3D"Copyright (C) 2013 Free Software Foundation, Inc." >=20 > 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 m= eans. But changing a wiki afterwards does not sound so difficult. >=20 > > +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 >=20 > 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 fil= e approach and create a temp (header) file inside the build folder, not ins= ide the $(srcdir) folder. [see next update] >=20 > -- > 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