Re: [RFC] Framework for easy distribution of SystemTap scripts (V2)

["Frank Ch. Eigler" <fche@redhat.com>]

Hi -

On Fri, Apr 09, 2010 at 09:48:53PM +0530, Anithra P Janakiraman wrote:
> [...]
> Thanks for the review! I've attached a V2 of the patch.

Thanks.

> >>+++ b/scripts/build_pkg/README.stap-buildpkg
> >Could this be a normal man page (stap-buildpkg.1) ?
> 
> We have placed the script in the scripts directory, We dont want to 
> install the script, so didn't see the use for a man page.

Why *not* install it?  How are people supposed to get a hold of your
script if it doesn't get installed?


> >Why does there need to be a separate 'version'?

> To be able to distinguish between packages built for different versions 
> of the same script. We initially had a 'release' too :)

To the extent that all these names are free-form, a user could encode
whatever versioning info he needs right into the main module/package
name, but whatever.


> >>+A post processing script can be provided using the -p option.
> >
> >You should specify what exactly this means.  A '/bin/sh -c' command
> >line to pipe stdout through?
> 
> The post processing script can do anything the administrator creating 
> the package wishes to do, maybe even fwd the output to a support team, 
> or provide suggestions based on the output. Have added an explanation.

The key part is to say that for -p SCRIPT, SCRIPT is actually a file name,
whose contents will be copied into the new package, and which will be run
via a pipe to consume the stdout of stdrun.

> >>+        *)                     STAP_OPTIONS=$@
> >>+				echo $STAP_OPTIONS
> >
> >You'll need to watch the quoting in these $foo variables.
> >They could contain spaces etc.
> 
> Didn't understand the comment. stap-buildpkg script.stp -n "package 
> name" -v 1  would work.

Yes, but you need to check your quotation throughout.  In some cases,
there is "$foo", and in other cases, only $foo.  The difference can
matter in some contexts.  So I'm just saying to check and/or test with
all possible spacey arguments.


> >FWIW, you could include this template within the main buildpkg script, as 
> >in
> >
> >echo>  $PKG<<  'END'
> >function run()
> >....
> >END
> >
> 
> True. This is for simplicity of design. It is intuitively a template and 
> a separate file, so we decided to keep it like that esp since we had no 
> intentions of installing the build-pkg script.

I guess simplicity is in the eye of the beholder.  You are having to
edit the thing with sed and concatenate things before & after: this
too seems clumsy.

From just eyeballing the new version of the scripts, it's not clear
how this template file is located.  (If it were installed, you could
autoconf and @prefix@-inform it.)


> >>+ps -ef | grep TEMPLATE_PKG_NAME | grep stapio | awk '{print $2}' | xargs 
> >>kill -SIGINT
> >
> >You wouldn't need this is you stored $! after the "staprun ...&" a few 
> >lines ago.
> 
> Yes. We had done it this way keeping the older versions of SystemTap in 
> mind, the $! returns pid of 'staprun' in older versions. I've changed 
> the template

(There still appears some ps -ef | grepping in the new version; probably
all of that is unnecessary, if you save child pids properly.)


> >Why does this template need to exist?  The framework maker script
> >could transcribe all of the options right into the output script.
> 
> We want to provide a set of 'default' options.  [...]

But one can provide defaults in lots of ways.  If you believe time-limited
staprun execution is a common and basic option, you can make that an option
provided first class within the stap-buildpkg script, and override it with
a first-class command line options.

To have a separate configuration file for this only, you'd need to
argue why some options have to be treated differently from others.

- FChE