Re: [RFC] First scripts for ARI support inisde gdb tree

[Joel Brobecker <brobecker at adacore dot com>]

>   I do have several potential issues:
> - permissions: Is it possible to set unix-like permissions using cvs?
> I now the svn has such options, but I don't know for cvs...

I think that CVS does record the unix permissions, or enough of it
for our purposes, but I'm not entirely sure anymore. Make sure
that the scripts do have the execute permission before you commit
anything and I think we'll be good to go.

>   ./ari/create-web-ari-in-src.sh 
> is the ultimate simple script:
> It uses ../.. as a main source directory
> (and thus .. as gdb source directory).
> it create a tmp directory called /tmp/testari
> and writes results into ~/htdocs/www/local/ari directory.
>   I have no idea if those defaults are OK for most systems...

I'd like that script to work more like configure: Use the path to
itself to determine the src dir, and create the HTML pages in
the current directory.  That way, it's easy to use out-of-tree,
and does not force the user to use any specific directory name.

Here is how to do it in a mostly-portable way:

    root=`dirname $0`

    # If "root" is a relative path, then convert it to absolute.
    if [ "`echo ${root} | cut -b1`" != '/' ]; then
       root="`pwd`/${root}"
    fi

(works everywhere except perhaps MinGW).

> I tried to add a GDB copyright header to all scripts
> and to use the copyright years corresponding to the
> years listed by a cvs log script_file
> in the ss cvs directory.

Good idea!

> 2012-05-17  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	* ari/: New directory.
> 	* ari/create-web-ari-in-src.sh: New file.
> 	* ari/gdb_ari.sh: New file.
> 	* ari/gdb_find.sh: New file.
> 	* ari/update-web-ari.sh: New file.

You don't need to list the creation of the ari/ directory, I think.

> +# GDB script to list of problems using awk.
> +#
> +# Copyright (C) 2002-2005,2007,2009-2012 Free Software Foundation, Inc.

You can change that to 2002-2012. Same for all the other files.

> +# Permenant checks take the form

Permanent

> +# Multi-line string
> +string_p && 

Trailing space...

> +    # Closing brace found? 

Ditto.

> +    # Closing brace found? 

Ditto.

> +# Only function implemenation should be on first column

implementation

> +	    if (char == "(") { if_brace_level++; }
> +	    if (char == ")") { 
> +		if_brace_level--;
> +		if (!if_brace_level) {
> +		    if_brace_end_pos = i; 
> +		    after_if = substr($0,i+1,length($0));
> +		    # Do not parse what is following
> +		    break;
> +		}
> +	    }
> +	}
> +	if (if_brace_level == 0) {
> +	    $0 = substr($0,1,i);
> +	    in_if = 0;
> +	} else {
> +	    if_full_line = if_full_line $0; 
> +	    if_cont_p = 1;
> +	    next; 
> +	}
> +    }
> +}
> +# if we arrive here, we need to concatenate, but we are at brace level 0
> +
> +(if_brace_end_pos) { 
> +    $0 = if_full_line substr($0,1,if_brace_end_pos);
> +    if (if_count > 1) {
> +	# print "IF: multi line " if_count " found at " FILENAME ":" FNR "
> \"" $0 "\""
> +    }
> +    if_cont_p = 0; 
> +    if_full_line = ""; 

Lots of trailing spaces in this section of the code...

> +# A find that prunes files that GDB users shouldn't be interested in.
> +# Use sort to order files alphabetically.
> +
> +find "$@" \
> +    -name testsuite -prune -o \
> +    -name gdbserver -prune -o \
> +    -name gnulib -prune -o \
> +    -name osf-share -prune -o \
> +    -name '*-stub.c' -prune -o \
> +    -name '*-exp.c' -prune -o \
> +    -name ada-lex.c -prune -o \
> +    -name cp-name-parser.c -prune -o \
> +    -type f -name '*.[lyhc]' -print | sort

I am thinking that this script can probably be inlined in the top-level
script (the one you called create-web-ari-in-src.sh).

> +PATH=/bin:/usr/bin:/usr/local/bin:$HOME/bin
> +export PATH

This is really sourceware-specific, let's remove it from here.

> +# Really mindless usage
> +if test $# -ne 4
> +then
> +    echo "Usage: $0 <snapshot/sourcedir> <tmpdir> <destdir> <project>" 1>&2
> +    exit 1
> +fi
> +snapshot=$1 ; shift
> +tmpdir=$1 ; shift
> +wwwdir=$1 ; shift
> +project=$1 ; shift

Eventually, I'd like to simply the user interface:
  - Remove the <tmpdir> parameter and replace it by $TMP or /tmp,
    or whatever;
  - Remove the <destdir> parameter and replace it by the current
    working directory
  - Remove the <project> parameter, since the project is always gdb.

Also, I don't think we'll need 3 scripts in total to do the job.
I think we can merge them all into 1, and hopefully that'll make
things a little simpler. But maybe not.

What I propose is that we avoid making the entry barrier too high,
and refrain from making dramatic design changes for now.  Once we
have something working, we can work on those design ideas to
simplify things as you see fit. As long as the interface to
the entry-point script does not change, anything underneath can
change at will...

> +    # Check if ${diff} is not empty
> +    if [ -s ${diff} ]; then
> +	# Send an email to muller@sourceware.org	
> +	mutt -s "New ARI warning ${email_suffix}" \
> +	    ${warning_email} < ${wwwdir}/mail-message 
> +    else
> +      if [ -s ${wwwdir}/${mail-message} ]; then
> +	# Send an email to muller@sourceware.org	
> +	mutt -s "ARI warning list change ${email_suffix}" \
> +	    muller@sourceware.org < ${wwwdir}/mail-message 
> +      fi
> +    fi

This part, on the other hand, needs to be excised from this script.
Otherwise, users wanting to compare the ARI before and after a change
might accidently send email to the list...

We'll probably want to have the bit of code that compares the ARI
results also in the GDB repository, so perhaps a separate script?
In the meantime, it's find to not include this part.


-- 
Joel