From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1088 invoked by alias); 27 May 2012 19:53:31 -0000 Received: (qmail 1077 invoked by uid 22791); 27 May 2012 19:53:30 -0000 X-SWARE-Spam-Status: No, hits=2.3 required=5.0 tests=AWL,BAYES_00,KHOP_DNSBL_BUMP,KHOP_THREADED,MSGID_MULTIPLE_AT,RCVD_IN_HOSTKARMA_BL,RCVD_IN_JMF_BL X-Spam-Check-By: sourceware.org Received: from mailhost.u-strasbg.fr (HELO mailhost.u-strasbg.fr) (130.79.200.152) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 27 May 2012 19:53:15 +0000 Received: from md16.u-strasbg.fr (md16.u-strasbg.fr [130.79.200.206]) by mailhost.u-strasbg.fr (8.14.3/jtpda-5.5pre1) with ESMTP id q4RJr5M3030215 ; Sun, 27 May 2012 21:53:05 +0200 (CEST) (envelope-from pierre.muller@ics-cnrs.unistra.fr) Received: from mailserver.u-strasbg.fr (ms15.u-strasbg.fr [130.79.204.115]) by md16.u-strasbg.fr (8.14.3/jtpda-5.5pre1) with ESMTP id q4RJr4BT012586 ; Sun, 27 May 2012 21:53:05 +0200 (envelope-from pierre.muller@ics-cnrs.unistra.fr) Received: from E6510Muller (lec67-4-82-230-53-140.fbx.proxad.net [82.230.53.140]) (user=mullerp mech=LOGIN) by mailserver.u-strasbg.fr (8.14.3/jtpda-5.5pre1) with ESMTP id q4RJr2HQ021582 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO) ; Sun, 27 May 2012 21:53:03 +0200 (envelope-from pierre.muller@ics-cnrs.unistra.fr) From: "Pierre Muller" To: "'Sergio Durigan Junior'" Cc: "'Jan Kratochvil'" , References: <001201cd3547$377188b0$a6549a10$@muller@ics-cnrs.unistra.fr> <20120525194654.GA30472@host2.jankratochvil.net> <003c01cd3b3c$d85e07d0$891a1770$@muller@ics-cnrs.unistra.fr> In-Reply-To: Subject: RE: [RFA-v2] Add scripts to generate ARI web pages to gdb/contrib/ari directory Date: Sun, 27 May 2012 19:53:00 -0000 Message-ID: <002101cd3c42$5473b670$fd5b2350$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable 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 X-SW-Source: 2012-05/txt/msg00983.txt.bz2 > -----Message d'origine----- > De=A0: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] De la part de Sergio Durigan Junior > Envoy=E9=A0: dimanche 27 mai 2012 06:06 > =C0=A0: Pierre Muller > Cc=A0: 'Jan Kratochvil'; gdb-patches@sourceware.org > Objet=A0: Re: [RFA-v2] Add scripts to generate ARI web pages to > gdb/contrib/ari directory >=20 > Hi Pierre, >=20 > On Saturday, May 26 2012, Pierre Muller wrote: >=20 > >> The patch is corrupted by line wrapping, 48 lines and some are not > trivial > >> to recover. > > Sorry, > > I hope the attached patch will apply correctly. >=20 > It applies, but it creates a new directoy named src/ari, instead of > src/gdb/contrib/ari. Not sure if it was intended, but it doesn't work > out of the box as I was expecting. See below. This is strange indeed, but it seems to depend on the patch executable that you use, I just discovered, while trying to install my patch on a freebsd machine, that on that system patch uses '-p0' option instead of '-p 0' on Linux... Maybe you hit a similar problem. =20 > > Concerning the new create-web-ari-in-src.sh, > > this is indeed a new script (hence the 2012 copyright only) > > and it is just a way to be able to generate the ARI index.html web > > page without any parameters. > > > > It basically only give default parameters > > to update-web-ari.sh script, which requires four parameters. > > > > I hope this clarifies some of your questions. >=20 > Yes, it does, thank you. >=20 > > Concerning Sergio's suggestion to separate out the awk script into > > a separate file, I would like to minimize the changes relative to the > > existing ss > > cvs repository files. >=20 > Hm, do you mean that you prefer to postpone this separation, or that you > don't intend to do it at all? >=20 > If the latter, I still think it's valid to do it because it will improve > the readability of the code, IMO. But of course I won't push it if you > don't intend to do it. I am not against the idea of separating it out, but I would like to be careful to avoid problems if we later decide that the scripts should be run in a build directory (especially if we call a regenerated script in the build dir using the source dir awk script...) =20 > > About the use of dirname, I think that > > direname is like basename part of > > coreutils, and basename is already use several times > > inside update-web-ari script in ss. >=20 > Yes, I agree, my only concern is that maybe some obscure system won't > have some of these binaries (like `awk', for example). Of course generating the Awk Regression Index web page without an installed awk binary will be difficult :) =20 > > I agree that being made public and thus available to > > many users, it would be nice to chack availability, and add a workaround, > > but I have no > > precise how to do it, probably using a configure or Makefile could help > here > > . >=20 > I was thinking more about a check in the shell script itself, no need > for Makefiles or configure options. OK. =20 > > Note that gdb directory cvonfigure script seems to contain both > > dirname and basename... >=20 > Yeah, good point. Maybe I'm being too paranoid. >=20 > > I hope you will be able to generate a ARI web page, > > and give more feedbacks, >=20 > I wasn't able to generate the webpage easily. I had to do several fixes > in the scripts. I am sending a new version of the patch which should > apply cleanly and create the proper directory under src/gdb/contrib. Could you tell us on which system you tried? =20 > I have taken the liberty to fix several errors that were not allowing me > to generate the web page correctly. In order to test it, I was using > the following command: >=20 > /bin/sh update-web-ari.sh ~/work/src/git/gdb-src /tmp/create-ari > /tmp/webdir-ari gdb >=20 > i.e., >=20 > /bin/sh update-web-ari.sh >=20 > Note that I did not set the executable bit in any of the scripts below. > I have chosen to leave them as regular files, just like you did in your > patch. The patch generated by cvs diff -u -p -N=20 doesn't contain any filemode information, I don't know if this is possible with CVS directly, but the files do have the executable permission set for user, because this was suggested earlier in the emails. =20=20 To discusss your changes more easily, I generated a diff between my version and yours, I will comment on this diffs below. >>diff -u -p -r ari/create-web-ari-in-src.sh sergio-ari/create-web-ari-in-src.sh >>--- ari/create-web-ari-in-src.sh 2012-05-27 12:01:02.000000000 +0200 >>+++ sergio-ari/create-web-ari-in-src.sh 2012-05-27 11:59:26.000000000 +0200 >>@@ -58,7 +58,7 @@ if [ -z "${webdir}" ] ; then >> fi >> >> # Launch update-web-ari.sh in same directory as current script. >>-${scriptpath}/update-web-ari.sh ${srcdir} ${tempdir} ${webdir} gdb >>+/bin/sh ${scriptpath}/update-web-ari.sh ${srcdir} ${tempdir} ${webdir} gdb >> >> if [ -f "${webdir}/index.html" ] ; then >> echo "ARI output can be viewed in file \"${webdir}/index.html\"" This is not needed if we keep the exec permission for users (or should it be for everyone?). >>diff -u -p -r ari/gdb_ari.sh sergio-ari/gdb_ari.sh >>--- ari/gdb_ari.sh 2012-05-27 12:01:02.000000000 +0200 >>+++ sergio-ari/gdb_ari.sh 2012-05-27 11:59:26.000000000 +0200 >>@@ -264,7 +264,7 @@ Do not use `Linux'\'', instead use `Linu >> && !/(^|[^_[:alnum:]])Linux\[sic\]([^_[:alnum:]]|$)/ \ >> && !/(^|[^_[:alnum:]])GNU\/Linux([^_[:alnum:]]|$)/ \ >> && !/(^|[^_[:alnum:]])Linux kernel([^_[:alnum:]]|$)/ \ >>-&& !/(^|[^_[:alnum:]])Linux [:digit:]\.[:digit:]+)/ { >>+&& !/(^|[^_[:alnum:]])Linux [[:digit:]]\.[[:digit:]]+)/ { >> fail("GNU/Linux") >> } >> This is one of the problems I saw, but wnted to fix only after a first commit to have a minimal difference between ss and gdb/contrib/ari =20 >>diff -u -p -r ari/update-web-ari.sh sergio-ari/update-web-ari.sh >>--- ari/update-web-ari.sh 2012-05-27 12:01:02.000000000 +0200 >>+++ sergio-ari/update-web-ari.sh 2012-05-27 11:59:26.000000000 +0200 >>@@ -85,6 +85,13 @@ else >> AWK=3Dgawk >> fi >> >>+# Checking for `doschk' binary (if `check_doschk_p' is active) >>+if [ "${check_doschk_p}" =3D=3D "true" ] && doschk > /dev/null 2>&1 >>+then >>+ have_doschk=3Dtrue >>+else >>+ have_doschk=3Dfalse >>+fi >> >> # Set up a few cleanups >> if ${delete_source_p} I am not sure this is correct: calling doschk without arguments=20 caused the program (if available) to wait for input to analyze instead of analyzing files given as parameters. Was your intent to test if the binary is available? I think we must do this without calling it. Currently, id doschk binary doesn't exist, line 302 of update-web-ari.sh will create an empty doschk_out file (with some error probably generated by the shell) but this is not a problem for me at least on cygwin or Linux >>@@ -99,7 +106,7 @@ if [ -d ${snapshot} ] >> then >> module=3D${project} >> srcdir=3D${snapshot} >>- aridir=3D${srcdir}/${module}/ari >>+ aridir=3D${srcdir}/${module}/contrib/ari >> unpack_source_p=3Dfalse >> delete_source_p=3Dfalse >> version_in=3D${srcdir}/${module}/version.in >> This is indeed a required change that=20 I forgot, sorry about that one. >>@@ -203,8 +210,8 @@ then >> oldpruned=3D${oldf1}-pruned >> newpruned=3D${newf1}-pruned >> >>- cp -f ${bugf} ${oldf} >>- cp -f ${srcf} ${oldsrcf} >>+ test -f ${bugf} && cp -f ${bugf} ${oldf} >>+ test -f ${srcf} && cp -f ${srcf} ${oldsrcf} >> rm -f ${srcf} >> node=3D`uname -n` >> echo "`date`: Using source lines ${srcf}" 1>&2 OK, I am still a bit weak on some shell basics, this means use 'cp' only if test -f ${file} returns zero, i.e. if the file exists. Is it really a problem to have a failing cp call here? >>@@ -251,10 +258,7 @@ then >> >> fi >> >>- >>- >>- >>-if ${check_doschk_p} && test -d "${srcdir}" >>+if ${check_doschk_p} && test "${have_doschk}" =3D=3D "true" && test -d "${srcdir}" >> then >> echo "`date`: Checking for doschk" 1>&2 >> rm -f "${wwwdir}"/ari.doschk.* Would=20 'if ${check_doschk_p} && ${have_doschk} && test -d ${srcdir}' be also correct here? =20=20 >>@@ -744,7 +748,9 @@ END { >> #print "tests/ok:", nr / ok >> #print "bugs/tests:", bugs / nr >> #print "bugs/ok:", bugs / ok >>- print bugs / ( oks + legacy + deprecated ) >>+ if (oks + legacy + deprecated > 0) { >>+ print bugs / ( oks + legacy + deprecated ) >>+ } >> } >> ' ${wwwdir}/ari.doc` >> This is nice, but once again, I would prefer to=20 add this only after an initial commit. >>@@ -860,7 +866,9 @@ EOF >> for a in $all; do >> alls=3D"$alls all[$a] =3D 1 ;" >> done >>- cat ari.*.doc | $AWK >> ${newari} ' >>+ if ls ${wwwdir}/ari.*.doc > /dev/null 2>&1 >>+ then >>+ cat ${wwwdir}/ari.*.doc | $AWK >> ${newari} ' >> BEGIN { >> FS =3D ":" >> '"$alls"' >>@@ -876,6 +884,7 @@ BEGIN { >> } >> } >> ' >>+ fi >> >> cat >> ${newari} <>
=20 Here also, your change improve the script quality, and I would be glad to incorporate it later. One more problem I encountered on FreeBSD is that 'awk' and 'gawk' are not completely equivalent and gawk seems to generate output while freebsd awk fails... Pierre=20