Re: [RFA] Add scripts to generate ARI web pages to gdb/contrib/ari directory

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Friday, May 18 2012, Pierre Muller wrote:

>   Here is a RFA for inclusion of scripts to gdb/contrib/ari.

As Jan pointed out, the patch does not apply.

I am assuming that, if you are asking for opinions, then you are
volunteering to fix the ARI scripts :-).  Here are my opinions.  Thanks
a lot for doing this!

> Index: contrib/ari/create-web-ari-in-src.sh
> ===================================================================
> RCS file: contrib/ari/create-web-ari-in-src.sh
> diff -N contrib/ari/create-web-ari-in-src.sh
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ contrib/ari/create-web-ari-in-src.sh	18 May 2012 22:31:42 -0000

Is this script called in some cronjob, or is it intended to be called
directly by the user?  I don't see it being called anywhere in the
sources.  If it is called from a cronjob, then maybe it's worth
providing an example of a simple crontab script which would work for
this purpose.

Also, if it is supposed to be called by the user, then I think it should
accept command line arguments.

> @@ -0,0 +1,68 @@
> +#! /bin/sh
> +
> +# GDB script to create web ARI page directly from within gdb/ari directory.
> +#
> +# Copyright (C) 2012 Free Software Foundation, Inc.

Is this a new script?  If not, I believe the copyright notice should
include the previous years of existence.

> +# This file is part of GDB.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.+
> +set -x
> +
> +# Determine directory of current script.
> +scriptpath=`dirname $0`

Since we are now putting the scripts in the main tree, which IMO is an
incentive for everyone to run them and check the results, I believe we
cannot always assume that certain programs are available at the user's
machine.  For this reason, maybe it's good to check if the executables
(like `dirname') being used in these sources actually exist?

> Index: contrib/ari/gdb_ari.sh
> ===================================================================
> RCS file: contrib/ari/gdb_ari.sh
> diff -N contrib/ari/gdb_ari.sh
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ contrib/ari/gdb_ari.sh	18 May 2012 22:31:42 -0000


> +awk -- '
> +BEGIN {

What do you think of creating a new file which would contain this giant
awk script?  I see there are many "sections" in this script, so maybe we
could even separate those script into logical files and use multiple `-f
FILE' arguments to awk.  But I guess only putting this huge script into
a separate file is enough for now...

Thanks,

-- 
Sergio