Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Pierre Muller <pierre.muller@ics-cnrs.unistra.fr>
Cc: gdb-patches@sourceware.org
Subject: Re: [RFC] First scripts for ARI support inisde gdb tree
Date: Thu, 17 May 2012 13:16:00 -0000	[thread overview]
Message-ID: <20120517131616.GD10253@adacore.com> (raw)
In-Reply-To: <003c01cd33b2$b6225130$2266f390$@muller@ics-cnrs.unistra.fr>

>   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


  reply	other threads:[~2012-05-17 13:16 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-16 22:25 Pierre Muller
2012-05-17 13:16 ` Joel Brobecker [this message]
2012-05-17 19:46   ` Tom Tromey
2012-05-17 20:39     ` Joel Brobecker
2012-05-18 13:48       ` Tom Tromey
2012-05-18 17:26         ` Joel Brobecker
2012-05-18  8:25   ` [RFC-v2] " Pierre Muller
2012-05-18 13:03     ` Joel Brobecker
2012-05-17 17:29 ` [RFC] " Sergio Durigan Junior

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=20120517131616.GD10253@adacore.com \
    --to=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=pierre.muller@ics-cnrs.unistra.fr \
    /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