[PATCH] Environment variable convenience function

[PATCH] Environment variable convenience function

[Keith Seitz]

This patch adds a new Python convenience function which simply
returns the value of the given environment variable (or a default value)
or throws a KeyError.

(gdb) p $_env("HOME")
$1 = "/home/keiths"
(gdb) p $_env("IS_THIS_USEFUL")
Python Exception <class 'KeyError'>: 'IS_THIS_USEFUL'
Error occurred in Python: 'IS_THIS_USEFUL'
(gdb) p $_env("IS_THIS_USEFUL", "yes")
$2 = "yes"
---
 gdb/NEWS                            |  3 +++
 gdb/data-directory/Makefile.in      |  1 +
 gdb/doc/gdb.texinfo                 |  7 ++++++
 gdb/python/lib/gdb/function/env.py  | 38 +++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/default.exp  |  1 +
 gdb/testsuite/gdb.python/py-env.c   | 22 +++++++++++++++++
 gdb/testsuite/gdb.python/py-env.exp | 38 +++++++++++++++++++++++++++++
 7 files changed, 110 insertions(+)
 create mode 100644 gdb/python/lib/gdb/function/env.py
 create mode 100644 gdb/testsuite/gdb.python/py-env.c
 create mode 100644 gdb/testsuite/gdb.python/py-env.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 54b5da21245..4897b1e618b 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -156,6 +156,9 @@ info main
      (program-counter) values, and can be used as the frame-id when
      calling gdb.PendingFrame.create_unwind_info.
 
+  ** New Python-based convenience function $_env(name[, default]), which
+     returns the string value of the environment variable of the given name.
+
 *** Changes in GDB 13
 
 * MI version 1 is deprecated, and will be removed in GDB 14.
diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index 39979037245..637d2fa0d55 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -109,6 +109,7 @@ PYTHON_FILE_LIST = \
 	gdb/function/as_string.py \
 	gdb/function/caller_is.py \
 	gdb/function/strfns.py \
+	gdb/function/env.py \
 	gdb/printer/__init__.py \
 	gdb/printer/bound_registers.py
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 940315b2713..1887d0c1ae4 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -13160,6 +13160,13 @@ The type of the imaginary or real part depends on the type of the
 complex number, e.g., using @code{$_cimag} on a @code{float complex}
 will return an imaginary part of type @code{float}.
 
+@findex $_env@r{, convenience function}
+@item $_env(@var{name}@r{[}, @var{default}@r{]})
+Return the value of the environment variable named @var{name}.
+
+If the variable is not defined in the environment, the optional default value
+is returned, if specified, or a Python exception is thrown.
+
 @end table
 
 @value{GDBN} provides the ability to list and get help on
diff --git a/gdb/python/lib/gdb/function/env.py b/gdb/python/lib/gdb/function/env.py
new file mode 100644
index 00000000000..a6414ee63b4
--- /dev/null
+++ b/gdb/python/lib/gdb/function/env.py
@@ -0,0 +1,38 @@
+# Copyright (C) 2023 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/>.
+
+"""$_env"""
+
+import gdb
+import os
+
+class _Env(gdb.Function):
+    """$_env - return the value of an environment variable.
+
+    Usage: $_env("NAME"[, default])
+
+    Returns:
+      Value of the environment variable named NAME or DEFAULT (if supplied);
+      throws KeyError otherwise."""
+
+    def __init__(self):
+        super(_Env, self).__init__("_env")
+
+    def invoke(self, name, default=None):
+        if default is not None and name.string() not in os.environ:
+            return default
+        return os.environ[name.string()]
+
+_Env()
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index 7e73db0576a..551ae7aca9b 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -628,6 +628,7 @@ if [allow_python_tests] {
 	    {$_caller_matches = <internal function _caller_matches>} \
 	    {$_any_caller_is = <internal function _any_caller_is>} \
 	    {$_any_caller_matches = <internal function _any_caller_matches>} \
+	    {$_env = <internal function _env>} \
 	}
 }
 gdb_test_list_exact "show convenience" "show convenience" \
diff --git a/gdb/testsuite/gdb.python/py-env.c b/gdb/testsuite/gdb.python/py-env.c
new file mode 100644
index 00000000000..d17c53b2007
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-env.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 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/>.  */
+
+void
+main (void)
+{
+  return;
+}
diff --git a/gdb/testsuite/gdb.python/py-env.exp b/gdb/testsuite/gdb.python/py-env.exp
new file mode 100644
index 00000000000..09a274dcfe1
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-env.exp
@@ -0,0 +1,38 @@
+# Copyright (C) 2023 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 file is part of the GDB testsuite.  It tests the convenience
+# functions in env.py.
+
+load_lib gdb-python.exp
+
+require allow_python_tests
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
+    return -1
+}
+
+if {![runto_main]} {
+    return 0
+}
+
+gdb_test {p $_env("HOME")} "= \"$::env(HOME)\""
+
+gdb_test {p $_env("I_DO_NOT_EXIST")} ".*KeyError.*"
+
+gdb_test {p $_env("I_DO_NOT_EXIST", "default value for I_DO_NOT_EXIST")} \
+    {= "default value for I_DO_NOT_EXIST"}
-- 
2.39.2

Re: [PATCH] Environment variable convenience function

[Simon Marchi]

On 4/23/23 13:07, Keith Seitz via Gdb-patches wrote:
> This patch adds a new Python convenience function which simply
> returns the value of the given environment variable (or a default value)
> or throws a KeyError.
> 
> (gdb) p $_env("HOME")
> $1 = "/home/keiths"
> (gdb) p $_env("IS_THIS_USEFUL")
> Python Exception <class 'KeyError'>: 'IS_THIS_USEFUL'
> Error occurred in Python: 'IS_THIS_USEFUL'
> (gdb) p $_env("IS_THIS_USEFUL", "yes")
> $2 = "yes"

Hi Keith,

I have the feeling that I've seen this before, and had some thoughts
about the name, but I'm not sure I shared it, so here it is.  I think it
would be good to disambibuate whether this function reads from GDB's own
environment or the current inferior's environment.  Looking at the
implementation it seems, to be GDB's environment.

Should we introduce a function that reads the environment of the current
inferior, how would it be named?

I'm wondering if, to avoid confusion, this one should be named something
like _gdb_env, and an hypothetical future function that reads the
inferior's environment would be named something like _inferior_env.

Simon

Re: [PATCH] Environment variable convenience function

[Eli Zaretskii]

> Date: Sun, 23 Apr 2023 10:07:55 -0700
> From: Keith Seitz via Gdb-patches <gdb-patches@sourceware.org>
> 
> This patch adds a new Python convenience function which simply
> returns the value of the given environment variable (or a default value)
> or throws a KeyError.
> 
> (gdb) p $_env("HOME")
> $1 = "/home/keiths"
> (gdb) p $_env("IS_THIS_USEFUL")
> Python Exception <class 'KeyError'>: 'IS_THIS_USEFUL'
> Error occurred in Python: 'IS_THIS_USEFUL'
> (gdb) p $_env("IS_THIS_USEFUL", "yes")
> $2 = "yes"
> ---
>  gdb/NEWS                            |  3 +++
>  gdb/data-directory/Makefile.in      |  1 +
>  gdb/doc/gdb.texinfo                 |  7 ++++++
>  gdb/python/lib/gdb/function/env.py  | 38 +++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.base/default.exp  |  1 +
>  gdb/testsuite/gdb.python/py-env.c   | 22 +++++++++++++++++
>  gdb/testsuite/gdb.python/py-env.exp | 38 +++++++++++++++++++++++++++++
>  7 files changed, 110 insertions(+)
>  create mode 100644 gdb/python/lib/gdb/function/env.py
>  create mode 100644 gdb/testsuite/gdb.python/py-env.c
>  create mode 100644 gdb/testsuite/gdb.python/py-env.exp

Thanks.

> diff --git a/gdb/NEWS b/gdb/NEWS
> index 54b5da21245..4897b1e618b 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -156,6 +156,9 @@ info main
>       (program-counter) values, and can be used as the frame-id when
>       calling gdb.PendingFrame.create_unwind_info.
>  
> +  ** New Python-based convenience function $_env(name[, default]), which
> +     returns the string value of the environment variable of the given name.

This part is OK.

> +@findex $_env@r{, convenience function}
> +@item $_env(@var{name}@r{[}, @var{default}@r{]})
> +Return the value of the environment variable named @var{name}.
> +
> +If the variable is not defined in the environment, the optional default value
> +is returned, if specified, or a Python exception is thrown.

The last sentence abuses the passive voice, so I suggest to rephrase:

  If the variable @var{name} is not defined in the environment, the
  function returns the vale of the optional argument @var{default},
  and if that was not specified, it throws a Python exception.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Re: [PATCH] Environment variable convenience function

[Keith Seitz]

On 4/23/23 13:20, Simon Marchi wrote:
> On 4/23/23 13:07, Keith Seitz via Gdb-patches wrote:
> 
> I have the feeling that I've seen this before, and had some thoughts
> about the name, but I'm not sure I shared it, so here it is.  I think it
> would be good to disambibuate whether this function reads from GDB's own
> environment or the current inferior's environment.  Looking at the
> implementation it seems, to be GDB's environment.

You probably did see this before -- I submitted an RFC  which included
this feature/patch <cough>a while ago</cough>. I don't recall seeing your
comments before, so I am glad that you're (re)sending them along!

> Should we introduce a function that reads the environment of the current
> inferior, how would it be named?

That is a good idea. I will get that on my TODO list. I presume it would
be acceptable to post a follow-up patch?

> I'm wondering if, to avoid confusion, this one should be named something
> like _gdb_env, and an hypothetical future function that reads the
> inferior's environment would be named something like _inferior_env.

Yes, I completely agree. I've updated my patch to follow this. I will send
a v2.

Thank you for the feedback!
Keith

Re: [PATCH] Environment variable convenience function

[Keith Seitz]

On 4/24/23 04:53, Eli Zaretskii wrote:
>> Date: Sun, 23 Apr 2023 10:07:55 -0700
>> +  ** New Python-based convenience function $_env(name[, default]), which
>> +     returns the string value of the environment variable of the given name.
> 
> This part is OK.
> 
>> +@findex $_env@r{, convenience function}
>> +@item $_env(@var{name}@r{[}, @var{default}@r{]})
>> +Return the value of the environment variable named @var{name}.
>> +
>> +If the variable is not defined in the environment, the optional default value
>> +is returned, if specified, or a Python exception is thrown.
> 
> The last sentence abuses the passive voice, so I suggest to rephrase:
> 
>    If the variable @var{name} is not defined in the environment, the
>    function returns the vale of the optional argument @var{default},
>    and if that was not specified, it throws a Python exception.
> 
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> 

I've updated my hack-job English -- thank you for the review/suggestions!

Keith

PS. I will be posting a v2 with subtle docs changes.

Re: [PATCH] Environment variable convenience function

[Simon Marchi]

On 4/25/23 11:59, Keith Seitz wrote:
> On 4/23/23 13:20, Simon Marchi wrote:
>> On 4/23/23 13:07, Keith Seitz via Gdb-patches wrote:
>>
>> I have the feeling that I've seen this before, and had some thoughts
>> about the name, but I'm not sure I shared it, so here it is.  I think it
>> would be good to disambibuate whether this function reads from GDB's own
>> environment or the current inferior's environment.  Looking at the
>> implementation it seems, to be GDB's environment.
> 
> You probably did see this before -- I submitted an RFC  which included
> this feature/patch <cough>a while ago</cough>. I don't recall seeing your
> comments before, so I am glad that you're (re)sending them along!
> 
>> Should we introduce a function that reads the environment of the current
>> inferior, how would it be named?
> 
> That is a good idea. I will get that on my TODO list. I presume it would
> be acceptable to post a follow-up patch?

Indeed, I'm not suggestion you do it (unless you want to).  I just want
to make sure we keep room for further improvements.

Simon