Re: RFC: add ability to "source" Python code

Pedro Alves
2009-02-10T00:00:38Z

On Sunday 08 February 2009 01:16:10, Tom Tromey wrote:
> This patch adds Python support to the gdb "source" command.
> I added this after several user questions about how to do this.
> Also, due to completion, it is much more convenient than the
> equivalent "python" command.
>
> With this patch, a file ending in ".py" is automatically treated as
> Python. Also, other files can be interpreted as Python code by using
> the "-p" switch. I made other switches into errors, on the theory
> that maybe we'd want to take more later ... I'm not sure if this is a
> good idea or not.
>
> This patch also changes '-x' to accept .py files, as a side effect.
>
> This could use a documentation review. Also I'd appreciate any
> comments on it; it does change "source" incompatibly, though in a way
> that is very unlikely to affect users.
>

I haven't played with the python stuff yet (shame on me!), but,
this got me curious on one thing.

Could you explain for the archives why this is better than having
the script itself tell GDB that it's running python? Either
through something similar to a shebang, or starting the script with
"python" (does that work?) ? I think you've discussed this before,
what were the limitations you found?

I'm thinking that as a user I don't usually care much scripting
language was used to implement a script -- it's an
implementation detail.

Say that in the future we add support for another scripting language
to GDB, and we want to keep the current python support working in
parallel. As a matter of example, let's call it "Python 3.0". Would
we want to add more "-p" switches to the source command?

> Built and regression tested on x86-64 (compile farm).
> New test case included.
>
> Tom
>
> 2009-02-07 Tom Tromey <tromey@redhat.com>
>
> * python/python.c (source_python_script): New function.
> * python/python.h (source_python_script): Declare.
> * cli/cli-cmds.c (find_argument): New function.
> (source_command): Use find_argument. Handle -p argument. Use
> make_cleanup_restore_integer.
> (source_verbose_cleanup): Remove.
> (source_python): New global.
> Include python.h.
> (init_cli_cmds): Update.
>
> 2009-02-07 Tom Tromey <tromey@redhat.com>
>
> * gdb.texinfo (File Options): Document -x on .py files.
> (Command Files): Document source -p.
>
> 2009-02-07 Tom Tromey <tromey@redhat.com>
>
> * gdb.base/help.exp: Update "source" help text.
> * gdb.python/source2.py: New file.
> * gdb.python/source1: New file.
> * gdb.python/python.exp: Test "source" command.
>
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index 4d9c4f3..3713927 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -45,6 +45,8 @@
> #include "cli/cli-setshow.h"
> #include "cli/cli-cmds.h"
>
> +#include "python/python.h"
> +
> #ifdef TUI
> #include "tui/tui.h" /* For tui_active et.al. */
> #endif
> @@ -178,6 +180,7 @@ struct cmd_list_element *showchecklist;
>
> /* Command tracing state. */
>
> +static int source_python = 0;
> int source_verbose = 0;
> int trace_commands = 0;
> \f
> @@ -439,6 +442,7 @@ source_script (char *file, int from_tty)
> struct cleanup *old_cleanups;
> char *full_pathname = NULL;
> int fd;
> + int is_python;
>
> if (file == NULL || *file == 0)
> {
> @@ -471,30 +475,44 @@ source_script (char *file, int from_tty)
> }
> }
>
> + is_python = source_python;
> + if (strlen (file) > 3 && !strcmp (&file[strlen (file) - 3], ".py"))
> + is_python = 1;
> +
> stream = fdopen (fd, FOPEN_RT);
> - script_from_file (stream, file);
> +
> + if (is_python)
> + source_python_script (stream, file);
> + else
> + script_from_file (stream, file);
>
> do_cleanups (old_cleanups);
> }
>
> -/* Return the source_verbose global variable to its previous state
> - on exit from the source command, by whatever means. */
> -static void
> -source_verbose_cleanup (void *old_value)
> +/* A helper for source_command. Look for an argument in *ARGS.
> + Update *ARGS by stripping leading whitespace. If an argument is
> + found, return it (a character). Otherwise, return 0. */
> +static int
> +find_argument (char **args)
> {
> - source_verbose = *(int *)old_value;
> - xfree (old_value);
> + int result = 0;
> + while (isspace ((*args)[0]))
> + ++*args;
> + if ((*args)[0] == '-' && isalpha ((*args)[1]))
> + {
> + result = (*args)[1];
> + *args += 3;
> + }
> + return result;
> }
>
> static void
> source_command (char *args, int from_tty)
> {
> struct cleanup *old_cleanups;
> - char *file = args;
> - int *old_source_verbose = xmalloc (sizeof(int));
>
> - *old_source_verbose = source_verbose;
> - old_cleanups = make_cleanup (source_verbose_cleanup, old_source_verbose);
> + old_cleanups = make_cleanup_restore_integer (&source_verbose);
> + make_cleanup_restore_integer (&source_python);
>
> /* -v causes the source command to run in verbose mode.
> We still have to be able to handle filenames with spaces in a
> @@ -502,23 +520,28 @@ source_command (char *args, int from_tty)
>
> if (args)
> {
> - /* Make sure leading white space does not break the comparisons. */
> - while (isspace(args[0]))
> - args++;
> -
> - /* Is -v the first thing in the string? */
> - if (args[0] == '-' && args[1] == 'v' && isspace (args[2]))
> + while (1)
> {
> - source_verbose = 1;
> -
> - /* Trim -v and whitespace from the filename. */
> - file = &args[3];
> - while (isspace (file[0]))
> - file++;
> + int arg = find_argument (&args);
> + if (!arg)
> + break;
> + switch (arg)
> + {
> + case 'v':
> + source_verbose = 1;
> + break;
> + case 'p':
> + source_python = 1;
> + break;
> + default:
> + error (_("unrecognized option -%c"), arg);
> + }
> }
> }
>
> - source_script (file, from_tty);
> + source_script (args, from_tty);
> +
> + do_cleanups (old_cleanups);
> }
>
>
> @@ -1281,6 +1304,8 @@ Commands defined in this way may have up to ten arguments."));
> Read commands from a file named FILE.\n\
> Optional -v switch (before the filename) causes each command in\n\
> FILE to be echoed as it is executed.\n\
> +Optional -p switch (before the filename) causes FILE to be evaluated\n\
> +as Python code.\n\
> Note that the file \"%s\" is read automatically in this way\n\
> when GDB is started."), gdbinit);
> c = add_cmd ("source", class_support, source_command,
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 166b84d..e6472ae 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -955,8 +955,10 @@ Connect to process ID @var{number}, as with the @code{attach} command.
> @itemx -x @var{file}
> @cindex @code{--command}
> @cindex @code{-x}
> -Execute @value{GDBN} commands from file @var{file}. @xref{Command
> -Files,, Command files}.
> +Execute commands from file @var{file}. If @var{file} ends in
> +@samp{.py}, then the file is evaluated as Python code. If Python
> +support is not enabled in this @value{GDBN}, then an error occurs.
> +@xref{Command Files,, Command files}.
>
> @item -eval-command @var{command}
> @itemx -ex @var{command}
> @@ -17753,7 +17755,7 @@ command:
> @table @code
> @kindex source
> @cindex execute commands from a file
> -@item source [@code{-v}] @var{filename}
> +@item source [@code{-v}] [@code{-p}] @var{filename}
> Execute the command file @var{filename}.
> @end table
>
> @@ -17770,6 +17772,11 @@ If @code{-v}, for verbose mode, is given then @value{GDBN} displays
> each command as it is executed. The option must be given before
> @var{filename}, and is interpreted as part of the filename anywhere else.
>
> +If @var{filename} ends in @samp{.py}, or if @code{-p} (for ``Python'')
> +is given, then @value{GDBN} evaluates the contents of the file as
> +Python code. If Python support is not compiled in to @value{GDBN},
> +then an error occurs.
> +
> Commands that would ask for confirmation if used interactively proceed
> without asking when used in a command file. Many @value{GDBN} commands that
> normally print messages to say what they are doing omit the messages
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index e778ac4..b275437 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -273,6 +273,22 @@ execute_gdb_command (PyObject *self, PyObject *args)
> Py_RETURN_NONE;
> }
>
> +/* Read a file as Python code. STREAM is the input file; FILE is the
> + name of the file. */
> +
> +void
> +source_python_script (FILE *stream, char *file)
> +{
> + PyGILState_STATE state;
> +
> + state = PyGILState_Ensure ();
> +
> + PyRun_SimpleFile (stream, file);
> +
> + fclose (stream);
> + PyGILState_Release (state);
> +}
> +
> \f
>
> /* Printing. */
> @@ -335,6 +351,13 @@ eval_python_from_control_command (struct command_line *cmd)
> error (_("Python scripting is not supported in this copy of GDB."));
> }
>
> +void
> +source_python_script (FILE *stream)
> +{
> + fclose (stream);
> + error (_("Python scripting is not supported in this copy of GDB."));
> +}
> +
> #endif /* HAVE_PYTHON */
>
> \f
> diff --git a/gdb/python/python.h b/gdb/python/python.h
> index e63c447..5a6a067 100644
> --- a/gdb/python/python.h
> +++ b/gdb/python/python.h
> @@ -26,4 +26,6 @@ extern struct value *values_in_python;
>
> void eval_python_from_control_command (struct command_line *);
>
> +void source_python_script (FILE *stream, char *file);
> +
> #endif /* GDB_PYTHON_H */
> diff --git a/gdb/testsuite/gdb.base/help.exp b/gdb/testsuite/gdb.base/help.exp
> index 4618a2c..ff2838a 100644
> --- a/gdb/testsuite/gdb.base/help.exp
> +++ b/gdb/testsuite/gdb.base/help.exp
> @@ -603,7 +603,7 @@ gdb_test "help stepi" "Step one instruction exactly\.\[\r\n\]+Argument N means d
> gdb_test "help signal" "Continue program giving it signal.*" "help signal"
> # test help source
> # vxgdb reads .vxgdbinit
> -gdb_test "help source" "Read commands from a file named FILE\.\[\r\n\]+Optional -v switch \\(before the filename\\) causes each command in\[\r\n\]+FILE to be echoed as it is executed\.\[\r\n\]+Note that the file \"\[^\"\]*\" is read automatically in this way\[\r\n\]+when GDB is started\." "help source"
> +gdb_test "help source" "Read commands from a file named FILE\.\[\r\n\]+Optional -v switch \\(before the filename\\) causes each command in\[\r\n\]+FILE to be echoed as it is executed\.\[\r\n\]+Optional -p switch \\(before the filename\\) causes FILE to be evaluated\[\r\n\]+as Python code\\.\[\r\n\]+Note that the file \"\[^\"\]*\" is read automatically in this way\[\r\n\]+when GDB is started\." "help source"
> # test help stack
> test_class_help "stack" {
> "Examining the stack\..*\[\r\n\]+"
> diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
> index a0c9b39..51c1a24 100644
> --- a/gdb/testsuite/gdb.python/python.exp
> +++ b/gdb/testsuite/gdb.python/python.exp
> @@ -71,3 +71,6 @@ gdb_py_test_multiple "indented multi-line python command" \
> " print 'hello, world!'" "" \
> "foo ()" "" \
> "end" "hello, world!"
> +
> +gdb_test "source -p $srcdir/$subdir/source1" "yes"
> +gdb_test "source $srcdir/$subdir/source2.py" "yes"
> diff --git a/gdb/testsuite/gdb.python/source1 b/gdb/testsuite/gdb.python/source1
> new file mode 100644
> index 0000000..b7ca419
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/source1
> @@ -0,0 +1,19 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2008, 2009 Free Software Foundation, Inc.
> +
> +# 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/>.
> +
> +# This is sourced as python; pick an expression that is not valid for gdb.
> +print 'y%ss' % 'e'
> diff --git a/gdb/testsuite/gdb.python/source2.py b/gdb/testsuite/gdb.python/source2.py
> new file mode 100644
> index 0000000..3d12222
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/source2.py
> @@ -0,0 +1,18 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2008, 2009 Free Software Foundation, Inc.
> +
> +# 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/>.
> +
> +print 'y%ss' % 'e'
>

--
Pedro Alves