Fix python compatibility with old versions of GDB

Message ID CAGHpTBJWOuixhd5U-rfGbvqgDwCaMFykTzPev9DHriUbKptvGg@mail.gmail.com
State New, archived
Headers

Commit Message

Orgad Shaneh June 11, 2017, 5:48 p.m. UTC
  > Hi Orgad,
>
> Thanks for submitting this, and sorry for the wait.  Could you provide a
> little bit of context to help others understand what's wrong?  Can you give
> some details on how to reproduce the problem?  Make sure to add it to the
> commit message.
>
> Also, please include a ChangeLog entry in your commit message, more details
> here:
>
> https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_Formatted_GNU_ChangeLog
>
> If anything is not clear, please ask.
>
> Thanks,
>
> Simon

If data-directory is shared between various version of GDB, it should
work for all of them.

There are several hasattr conditions that enable this kind of
compatibility.

RegexpCollectionPrettyPrinter was missing a check that enables it to
work with GDB prior to 7.9, when Type.name was introduced.

gdb/ChangeLog:

   * printing.py: Fix compatibility with old versions of GDB
---
gdb/ChangeLog                  | 4 ++++
gdb/python/lib/gdb/printing.py | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)

--
2.11.0
  

Comments

Simon Marchi June 11, 2017, 9:09 p.m. UTC | #1
On 2017-06-11 19:48, Orgad Shaneh wrote:
> If data-directory is shared between various version of GDB, it should
> work for all of them.
> 
> There are several hasattr conditions that enable this kind of
> compatibility.
> 
> RegexpCollectionPrettyPrinter was missing a check that enables it to
> work with GDB prior to 7.9, when Type.name was introduced.

Hi Orgad,

I still don't understand what problem this is trying to fix.  It looks 
like you want to make older versions of GDB work with newer versions of 
the Python scripts in the data directory.  I am not sure this is what we 
want.  If you want multiple version of GDB in parallel on a system, they 
should all be compiled with a different --prefix, and they will all get 
their own data-directory.  We should only expect a certain version of 
the data directory to be compatible with the version of GDB it was 
shipped with.  Or am I missing some use case where this is not true?

Simon
  
Orgad Shaneh June 12, 2017, 4:31 a.m. UTC | #2
On Mon, Jun 12, 2017 at 12:09 AM, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> On 2017-06-11 19:48, Orgad Shaneh wrote:
>>
>> If data-directory is shared between various version of GDB, it should
>> work for all of them.
>>
>> There are several hasattr conditions that enable this kind of
>> compatibility.
>>
>> RegexpCollectionPrettyPrinter was missing a check that enables it to
>> work with GDB prior to 7.9, when Type.name was introduced.
>
>
> Hi Orgad,
>
> I still don't understand what problem this is trying to fix.  It looks like
> you want to make older versions of GDB work with newer versions of the
> Python scripts in the data directory.  I am not sure this is what we want.
> If you want multiple version of GDB in parallel on a system, they should all
> be compiled with a different --prefix, and they will all get their own
> data-directory.  We should only expect a certain version of the data
> directory to be compatible with the version of GDB it was shipped with.  Or
> am I missing some use case where this is not true?

Hi,

Practically the data-directory is mostly backwards-compatible, except this
small part which I found (there might be others which I didn't find). With this
patch, I'm able to run GDB 7.8 with the latest data-directory.

Is there a reason not to accept it?

- Orgad
  
Simon Marchi June 13, 2017, 11:23 a.m. UTC | #3
On 2017-06-12 06:31, Orgad Shaneh wrote:
> On Mon, Jun 12, 2017 at 12:09 AM, Simon Marchi 
> <simon.marchi@polymtl.ca> wrote:
>> Hi Orgad,
>> 
>> I still don't understand what problem this is trying to fix.  It looks 
>> like
>> you want to make older versions of GDB work with newer versions of the
>> Python scripts in the data directory.  I am not sure this is what we 
>> want.
>> If you want multiple version of GDB in parallel on a system, they 
>> should all
>> be compiled with a different --prefix, and they will all get their own
>> data-directory.  We should only expect a certain version of the data
>> directory to be compatible with the version of GDB it was shipped 
>> with.  Or
>> am I missing some use case where this is not true?
> 
> Hi,
> 
> Practically the data-directory is mostly backwards-compatible, except 
> this
> small part which I found (there might be others which I didn't find). 
> With this
> patch, I'm able to run GDB 7.8 with the latest data-directory.
> 
> Is there a reason not to accept it?

I'd like to know what other maintainers think about this.  We may have 
already faced this situation before and taken a decision which I'm not 
aware of.  Otherwise, we need to take it now.

Simon
  
Tom Tromey June 13, 2017, 12:11 p.m. UTC | #4
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

>> Practically the data-directory is mostly backwards-compatible, except
>> this small part which I found (there might be others which I didn't
>> find). With this patch, I'm able to run GDB 7.8 with the latest
>> data-directory.

>> Is there a reason not to accept it?

Simon> I'd like to know what other maintainers think about this.  We may have
Simon> already faced this situation before and taken a decision which I'm not
Simon> aware of.  Otherwise, we need to take it now.

I think it doesn't make sense to have a policy allowing the sharing of
installed Python scripts across versions.  If this were a policy then it
would mean that the scripts could never use new features of gdb's Python
layer -- but that's clearly an absurd result.

For the particular patch in question, it seems to me it would at least
need a comment to be accepted.  Otherwise the point of the additional
test is obscure.

Tom
  
Phil Muldoon June 13, 2017, 12:29 p.m. UTC | #5
On 13/06/17 13:11, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
>>> Practically the data-directory is mostly backwards-compatible, except
>>> this small part which I found (there might be others which I didn't
>>> find). With this patch, I'm able to run GDB 7.8 with the latest
>>> data-directory.
> 
>>> Is there a reason not to accept it?
> 
> Simon> I'd like to know what other maintainers think about this.  We may have
> Simon> already faced this situation before and taken a decision which I'm not
> Simon> aware of.  Otherwise, we need to take it now.
> 
> I think it doesn't make sense to have a policy allowing the sharing of
> installed Python scripts across versions.  If this were a policy then it
> would mean that the scripts could never use new features of gdb's Python
> layer -- but that's clearly an absurd result.

I agree. It would make maintenance a nightmare, moreso with supporting Python 2 and 3.

Cheers

Phil
  
Koning, Paul June 13, 2017, 12:53 p.m. UTC | #6
> On Jun 13, 2017, at 7:23 AM, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> 
> On 2017-06-12 06:31, Orgad Shaneh wrote:
>> On Mon, Jun 12, 2017 at 12:09 AM, Simon Marchi <simon.marchi@polymtl.ca> wrote:
>>> Hi Orgad,
>>> I still don't understand what problem this is trying to fix.  It looks like
>>> you want to make older versions of GDB work with newer versions of the
>>> Python scripts in the data directory.  I am not sure this is what we want.
>>> If you want multiple version of GDB in parallel on a system, they should all
>>> be compiled with a different --prefix, and they will all get their own
>>> data-directory.  We should only expect a certain version of the data
>>> directory to be compatible with the version of GDB it was shipped with.  Or
>>> am I missing some use case where this is not true?
>> Hi,
>> Practically the data-directory is mostly backwards-compatible, except this
>> small part which I found (there might be others which I didn't find). With this
>> patch, I'm able to run GDB 7.8 with the latest data-directory.
>> Is there a reason not to accept it?
> 
> I'd like to know what other maintainers think about this.  We may have already faced this situation before and taken a decision which I'm not aware of.  Otherwise, we need to take it now.

Are we talking about GDB-supplied scripts, or user scripts?

For scripts that come with GDB, the script comes with the rest of GDB so it can be specific to a version.  That makes for more work in creating or modifying GDB, but it's our problem.

User scripts should deliver backward compatibility: a script that conforms to the documented API should continue to work in later releases.  The converse doesn't hold, of course: a script written to the latest API doesn't necessarily work with older versions, just as a program written to C11 doesn't necessarily compile or run with C89.

As for Python 2 vs. 3, GDB supports both and the Python-facing API is the same (modulo the places where Python 3 itself is different, such as around strings vs. bytes or ints vs. long ints).  A simple script may work with both; a more complex script might not be unless the author goes through some effort to make it so.  But that's not particularly related to GDB, it's more a question of how you write Python-version-independent scripts.

	paul
  
Eli Zaretskii June 13, 2017, 2:44 p.m. UTC | #7
> From: Tom Tromey <tom@tromey.com>
> Cc: Orgad Shaneh <orgads@gmail.com>,  gdb-patches@sourceware.org
> Date: Tue, 13 Jun 2017 06:11:57 -0600
> 
> >>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> >> Practically the data-directory is mostly backwards-compatible, except
> >> this small part which I found (there might be others which I didn't
> >> find). With this patch, I'm able to run GDB 7.8 with the latest
> >> data-directory.
> 
> >> Is there a reason not to accept it?
> 
> Simon> I'd like to know what other maintainers think about this.  We may have
> Simon> already faced this situation before and taken a decision which I'm not
> Simon> aware of.  Otherwise, we need to take it now.
> 
> I think it doesn't make sense to have a policy allowing the sharing of
> installed Python scripts across versions.  If this were a policy then it
> would mean that the scripts could never use new features of gdb's Python
> layer -- but that's clearly an absurd result.

FWIW, I always configure GDB to use a version-specific data-directory,
because I leave old GDB versions (renamed) on my system, so I could
still use them after installing a new version.  E.g., if the new
version turns out to have a bug.  I would actually be happier if we
changed our installation scripts to do that by default, because the
scripts are almost never backward-compatible IME.
  
Simon Marchi June 13, 2017, 5:29 p.m. UTC | #8
On 2017-06-13 16:44, Eli Zaretskii wrote:
> FWIW, I always configure GDB to use a version-specific data-directory,
> because I leave old GDB versions (renamed) on my system, so I could
> still use them after installing a new version.  E.g., if the new
> version turns out to have a bug.  I would actually be happier if we
> changed our installation scripts to do that by default, because the
> scripts are almost never backward-compatible IME.

Can you give more details on what you think could/should be done by 
default?

For such a use case, I would compile each gdb with a separate prefix 
(e.g. --prefix=/opt/gdb/<version>), which would give each install its 
own data directory.  So from my point of view, the "feature" is pretty 
well-covered.
  
Matt Rice June 13, 2017, 5:47 p.m. UTC | #9
On Tue, Jun 13, 2017 at 10:29 AM, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> On 2017-06-13 16:44, Eli Zaretskii wrote:
>>
>> FWIW, I always configure GDB to use a version-specific data-directory,
>> because I leave old GDB versions (renamed) on my system, so I could
>> still use them after installing a new version.  E.g., if the new
>> version turns out to have a bug.  I would actually be happier if we
>> changed our installation scripts to do that by default, because the
>> scripts are almost never backward-compatible IME.
>
>
> Can you give more details on what you think could/should be done by default?
>
> For such a use case, I would compile each gdb with a separate prefix (e.g.
> --prefix=/opt/gdb/<version>), which would give each install its own data
> directory.  So from my point of view, the "feature" is pretty well-covered.

Is there any argument against appending version to the data-directory
by default?

e.g. gcc configures /usr/lib/gcc/triplet/version?  I see a number of
/usr/share/*/[0-9]\.* directories here.
  
Tom Tromey June 13, 2017, 6:41 p.m. UTC | #10
>>>>> "Matt" == Matt Rice <ratmice@gmail.com> writes:

Matt> Is there any argument against appending version to the data-directory
Matt> by default?

Matt> e.g. gcc configures /usr/lib/gcc/triplet/version?  I see a number of
Matt> /usr/share/*/[0-9]\.* directories here.

It's not harmful to do this, but it's also incomplete, because there's
also the manual and the executable to consider.

Tom
  
Eli Zaretskii June 13, 2017, 7:28 p.m. UTC | #11
> Date: Tue, 13 Jun 2017 19:29:21 +0200
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Cc: Tom Tromey <tom@tromey.com>, orgads@gmail.com,
>  gdb-patches@sourceware.org
> 
> On 2017-06-13 16:44, Eli Zaretskii wrote:
> > FWIW, I always configure GDB to use a version-specific data-directory,
> > because I leave old GDB versions (renamed) on my system, so I could
> > still use them after installing a new version.  E.g., if the new
> > version turns out to have a bug.  I would actually be happier if we
> > changed our installation scripts to do that by default, because the
> > scripts are almost never backward-compatible IME.
> 
> Can you give more details on what you think could/should be done by 
> default?

I'd like the data-directory be version-dependent, i.e. be
/usr/share/gdb/VERSION/ and not just /usr/share/gdb/.  But I wonder
why that isn't the default behavior, as the files we install into
data-directory are version-dependent and incompatible with other GDB
versions.

> For such a use case, I would compile each gdb with a separate prefix 
> (e.g. --prefix=/opt/gdb/<version>), which would give each install its 
> own data directory.

That's close to what I do, as described above.  I just don't set
"--prefix", because that would affect directories like bindir, mandir,
infodir, which I don't want to keep separately for each version.
  
Eli Zaretskii June 13, 2017, 7:33 p.m. UTC | #12
> From: Tom Tromey <tom@tromey.com>
> Cc: Simon Marchi <simon.marchi@polymtl.ca>,  Eli Zaretskii <eliz@gnu.org>,  Tom Tromey <tom@tromey.com>,  orgads@gmail.com,  "gdb-patches\@sourceware.org" <gdb-patches@sourceware.org>
> Date: Tue, 13 Jun 2017 12:41:14 -0600
> 
> Matt> Is there any argument against appending version to the data-directory
> Matt> by default?
> 
> Matt> e.g. gcc configures /usr/lib/gcc/triplet/version?  I see a number of
> Matt> /usr/share/*/[0-9]\.* directories here.
> 
> It's not harmful to do this, but it's also incomplete, because there's
> also the manual and the executable to consider.

The executable can be easily renamed, and there's no need to keep old
manuals, since the new one is mostly backward compatible (we rarely
make changes that change documented behavior).
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c48fb92e9e..0a0005090b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@ 
+2017-06-11  Orgad Shaneh  <orgads@gmail.com>
+
+       * printing.py: Fix compatibility with old versions of GDB
+
2017-06-10  Simon Marchi  <simon.marchi@polymtl.ca>

       * gdbarch.sh (displaced_step_free_closure): Remove.
diff --git a/gdb/python/lib/gdb/printing.py b/gdb/python/lib/gdb/printing.py
index 181701c719..106da4ac8b 100644
--- a/gdb/python/lib/gdb/printing.py
+++ b/gdb/python/lib/gdb/printing.py
@@ -205,7 +205,7 @@  class RegexpCollectionPrettyPrinter(PrettyPrinter):

        # Get the type name.
        typename = gdb.types.get_basic_type(val.type).tag
-        if not typename:
+        if not typename and hasattr(val.type, "name"):
            typename = val.type.name
        if not typename:
            return None