gdb/python: provide fallback when curses library is not available

Message ID 7c515b37e2bf7ca319e76ebcbd3b16137d8919c3.1725900938.git.aburgess@redhat.com
State New
Headers
Series gdb/python: provide fallback when curses library is not available |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm fail Test failed

Commit Message

Andrew Burgess Sept. 9, 2024, 4:55 p.m. UTC
  The commit:

  commit 29c70787112e01cd52b53bf14bdcacb0a11e0725
  Date:   Sun Sep 8 07:46:09 2024 +0200

      [gdb/testsuite] Handle missing curses in gdb.python/py-missing-debug.exp

Highlighted that in some cases we might be running on a system with an
older version of Python (earlier than 3.7), and on a system for which
the curses library has not been installed.

In these circumstances the gdb.missing_debug module will not load as
it uses curses to provide isalnum() and isascii() functions.

To avoid this problem I propose that we copy the isalnum() and
isascii() from the Python curses library.  These functions are
basically trivial and removing the curses dependency means GDB will
work in more cases without increasing its dependencies.

I did consider keeping the uses of curses and only having the function
definitions be a fallback for when the curses library failed to load,
but this felt like overkill.  The function definitions are both tiny
and I think "obvious" given their specifications, so I figure we might
as well just use our own definitions if they are not available as
builtin methods on the str class.

For testing I changed this line:

  if sys.version_info >= (3, 7):

to

  if sys.version_info >= (3, 7) and False:

then reran gdb.python/py-missing-debug.exp, there were no failures.
---
 gdb/python/lib/gdb/missing_debug.py | 30 ++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)


base-commit: b6900dc73952f9c8b41414bdb26ea4dae33c6e2a
  

Comments

Tom de Vries Sept. 10, 2024, 11:02 a.m. UTC | #1
On 9/9/24 18:55, Andrew Burgess wrote:
> The commit:
> 
>    commit 29c70787112e01cd52b53bf14bdcacb0a11e0725
>    Date:   Sun Sep 8 07:46:09 2024 +0200
> 
>        [gdb/testsuite] Handle missing curses in gdb.python/py-missing-debug.exp
> 
> Highlighted that in some cases we might be running on a system with an
> older version of Python (earlier than 3.7), and on a system for which
> the curses library has not been installed.
> 
> In these circumstances the gdb.missing_debug module will not load as
> it uses curses to provide isalnum() and isascii() functions.
> 
> To avoid this problem I propose that we copy the isalnum() and
> isascii() from the Python curses library.  These functions are
> basically trivial and removing the curses dependency means GDB will
> work in more cases without increasing its dependencies.
> 

Hi Andrew,

thanks for doing this.

I was a bit concerned about the copying part, so I've looked a bit into 
this, and AFAIU given that the python license is both permissive and 
GPL-compatible, this should be fine.

I'll try to keep that in mind in the future.

> I did consider keeping the uses of curses and only having the function
> definitions be a fallback for when the curses library failed to load,
> but this felt like overkill.  The function definitions are both tiny
> and I think "obvious" given their specifications, so I figure we might
> as well just use our own definitions if they are not available as
> builtin methods on the str class.
> 
> For testing I changed this line:
> 
>    if sys.version_info >= (3, 7):
> 
> to
> 
>    if sys.version_info >= (3, 7) and False:
> 
> then reran gdb.python/py-missing-debug.exp, there were no failures.
> ---
>   gdb/python/lib/gdb/missing_debug.py | 30 ++++++++++++++++++++++++++---
>   1 file changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/python/lib/gdb/missing_debug.py b/gdb/python/lib/gdb/missing_debug.py
> index 6d57462c185..d33205eebd9 100644
> --- a/gdb/python/lib/gdb/missing_debug.py
> +++ b/gdb/python/lib/gdb/missing_debug.py
> @@ -31,9 +31,33 @@ if sys.version_info >= (3, 7):
>           return ch.isalnum()
>   
>   else:
> -    # Fall back to curses.ascii.isascii() and curses.ascii.isalnum() for
> -    # earlier versions.
> -    from curses.ascii import isalnum, isascii
> +    # Older version of Python doesn't have str.isascii() and
> +    # str.isalnum() so provide our own.
> +    #
> +    # We could import isalnum() and isascii() from the curses library,
> +    # but that adds an extra dependency.  Given these functions are
> +    # both small and trivial lets implement them here.
> +    #
> +    # These definitions are based on those in the curses library, but
> +    # simplified as we know C will always be a 'str'.

AFAIU, the specific assumption is that it's a 'str' of length 1, this 
won't work for a 'str' of length 2.  Please update the comment to 
clarify that.

Also, include reverting the test-case patch that you mention, since it 
should no longer be necessary.

Otherwise, LGTM.

Approved-By: Tom de Vries <tdevries@suse.de>

Thanks,
- Tom

> +
> +    def isdigit(c):
> +        return 48 <= ord(c) <= 57
> +
> +    def islower(c):
> +        return 97 <= ord(c) <= 122
> +
> +    def isupper(c):
> +        return 65 <= ord(c) <= 90
> +
> +    def isalpha(c):
> +        return isupper(c) or islower(c)
> +
> +    def isalnum(c):
> +        return isalpha(c) or isdigit(c)
> +
> +    def isascii(c):
> +        return 0 <= ord(c) <= 127
>   
>   
>   def _validate_name(name):
> 
> base-commit: b6900dc73952f9c8b41414bdb26ea4dae33c6e2a
  
Andrew Burgess Sept. 10, 2024, 1:18 p.m. UTC | #2
Tom de Vries <tdevries@suse.de> writes:

> On 9/9/24 18:55, Andrew Burgess wrote:
>> The commit:
>> 
>>    commit 29c70787112e01cd52b53bf14bdcacb0a11e0725
>>    Date:   Sun Sep 8 07:46:09 2024 +0200
>> 
>>        [gdb/testsuite] Handle missing curses in gdb.python/py-missing-debug.exp
>> 
>> Highlighted that in some cases we might be running on a system with an
>> older version of Python (earlier than 3.7), and on a system for which
>> the curses library has not been installed.
>> 
>> In these circumstances the gdb.missing_debug module will not load as
>> it uses curses to provide isalnum() and isascii() functions.
>> 
>> To avoid this problem I propose that we copy the isalnum() and
>> isascii() from the Python curses library.  These functions are
>> basically trivial and removing the curses dependency means GDB will
>> work in more cases without increasing its dependencies.
>> 
>
> Hi Andrew,
>
> thanks for doing this.
>
> I was a bit concerned about the copying part, so I've looked a bit into 
> this, and AFAIU given that the python license is both permissive and 
> GPL-compatible, this should be fine.
>
> I'll try to keep that in mind in the future.
>
>> I did consider keeping the uses of curses and only having the function
>> definitions be a fallback for when the curses library failed to load,
>> but this felt like overkill.  The function definitions are both tiny
>> and I think "obvious" given their specifications, so I figure we might
>> as well just use our own definitions if they are not available as
>> builtin methods on the str class.
>> 
>> For testing I changed this line:
>> 
>>    if sys.version_info >= (3, 7):
>> 
>> to
>> 
>>    if sys.version_info >= (3, 7) and False:
>> 
>> then reran gdb.python/py-missing-debug.exp, there were no failures.
>> ---
>>   gdb/python/lib/gdb/missing_debug.py | 30 ++++++++++++++++++++++++++---
>>   1 file changed, 27 insertions(+), 3 deletions(-)
>> 
>> diff --git a/gdb/python/lib/gdb/missing_debug.py b/gdb/python/lib/gdb/missing_debug.py
>> index 6d57462c185..d33205eebd9 100644
>> --- a/gdb/python/lib/gdb/missing_debug.py
>> +++ b/gdb/python/lib/gdb/missing_debug.py
>> @@ -31,9 +31,33 @@ if sys.version_info >= (3, 7):
>>           return ch.isalnum()
>>   
>>   else:
>> -    # Fall back to curses.ascii.isascii() and curses.ascii.isalnum() for
>> -    # earlier versions.
>> -    from curses.ascii import isalnum, isascii
>> +    # Older version of Python doesn't have str.isascii() and
>> +    # str.isalnum() so provide our own.
>> +    #
>> +    # We could import isalnum() and isascii() from the curses library,
>> +    # but that adds an extra dependency.  Given these functions are
>> +    # both small and trivial lets implement them here.
>> +    #
>> +    # These definitions are based on those in the curses library, but
>> +    # simplified as we know C will always be a 'str'.
>
> AFAIU, the specific assumption is that it's a 'str' of length 1, this 
> won't work for a 'str' of length 2.  Please update the comment to 
> clarify that.
>
> Also, include reverting the test-case patch that you mention, since it 
> should no longer be necessary.
>
> Otherwise, LGTM.

Thanks, I updated the comment and pushed along with reverting the
testsuite patch as suggested.

Thanks,
Andrew

>
> Approved-By: Tom de Vries <tdevries@suse.de>
>
> Thanks,
> - Tom
>
>> +
>> +    def isdigit(c):
>> +        return 48 <= ord(c) <= 57
>> +
>> +    def islower(c):
>> +        return 97 <= ord(c) <= 122
>> +
>> +    def isupper(c):
>> +        return 65 <= ord(c) <= 90
>> +
>> +    def isalpha(c):
>> +        return isupper(c) or islower(c)
>> +
>> +    def isalnum(c):
>> +        return isalpha(c) or isdigit(c)
>> +
>> +    def isascii(c):
>> +        return 0 <= ord(c) <= 127
>>   
>>   
>>   def _validate_name(name):
>> 
>> base-commit: b6900dc73952f9c8b41414bdb26ea4dae33c6e2a
  

Patch

diff --git a/gdb/python/lib/gdb/missing_debug.py b/gdb/python/lib/gdb/missing_debug.py
index 6d57462c185..d33205eebd9 100644
--- a/gdb/python/lib/gdb/missing_debug.py
+++ b/gdb/python/lib/gdb/missing_debug.py
@@ -31,9 +31,33 @@  if sys.version_info >= (3, 7):
         return ch.isalnum()
 
 else:
-    # Fall back to curses.ascii.isascii() and curses.ascii.isalnum() for
-    # earlier versions.
-    from curses.ascii import isalnum, isascii
+    # Older version of Python doesn't have str.isascii() and
+    # str.isalnum() so provide our own.
+    #
+    # We could import isalnum() and isascii() from the curses library,
+    # but that adds an extra dependency.  Given these functions are
+    # both small and trivial lets implement them here.
+    #
+    # These definitions are based on those in the curses library, but
+    # simplified as we know C will always be a 'str'.
+
+    def isdigit(c):
+        return 48 <= ord(c) <= 57
+
+    def islower(c):
+        return 97 <= ord(c) <= 122
+
+    def isupper(c):
+        return 65 <= ord(c) <= 90
+
+    def isalpha(c):
+        return isupper(c) or islower(c)
+
+    def isalnum(c):
+        return isalpha(c) or isdigit(c)
+
+    def isascii(c):
+        return 0 <= ord(c) <= 127
 
 
 def _validate_name(name):