bpo-29680: Older gdb does not have gdb.error.

[abalkin]

No description provided.

[mention-bot]

@abalkin, thanks for your PR! By analyzing the history of the files in this pull request, we identified @benjaminp, @loewis and @Haypo to be potential reviewers.

[abalkin]

@the-knights-who-say-ni - I don't see anythyng wrong with my b.p.o details:

[abalkin]

@methane - it looks like you introduced this issue in d7d2bc8. Could you, please review my fix?

[methane]

Would you move this to top of this file?

[abalkin]

This is currently at the PyDictObjectPtr class scope. I can move this to the top of the class unless you think it may be useful elsewhere.

[methane]

gdb.error seems inherits RuntimeError.
We can use just RuntimeError, maybe.

[abalkin]

Good idea. BTW, can you point me where gdb.error is defined? I cloned binutils-gdb, but could not find the right place. Nevertheless I can confirm the inheritance:

(gdb) python print gdb.error.mro()
[<class 'gdb.error'>, <type 'exceptions.RuntimeError'>, <type 'exceptions.StandardError'>, <type 'exceptions.Exception'>, <type 'exceptions.BaseException'>, <type 'object'>]

[methane]

here
https://github.com/bminor/binutils-gdb/blob/bf25e9a0f1315829defcb6ef36d8fef9d370e822/gdb/python/python.c#L1612-L1615

[vstinner]

@methane: " We can use just RuntimeError, maybe."

I prefer to use gdb.error when available to not catch a real RuntimeError bug by mistake.

[methane]

OK.
There are many except RuntimeError in this file and most of them may be gdb.error.
So let's make _gdb_error global instead of class attribute for it can be used from other places in the future.

[methane]

(nits) _gdb_error = getattr(gdb, 'error', RuntimeError) is shorter than try~except.

[Mariatta]

@abalkin, somehow your change was committed under GitHub username abelopolsky instead of abalkin 🤔

[abalkin]

@Mariatta - I can probably fix that.

[vstinner]

I dislike this change. I would prefer to catch gdb.error if available, or fallback on RuntimeError. Something like:

try:
   GDB_ERROR = gdb.error
except AttributeError:
   # gdb.error was introduced in gdb X.3
   GDB_ERROR = RuntimeError

...
except GDB_ERROR

gdb.error was introduced in gdb 7.3 by the commit 07ca107c2d958b45633ef0cdcce7219a95f0cf01:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;f=gdb/python/python.c;h=07ca107c2d958b45633ef0cdcce7219a95f0cf01

I tried but failed to use Git to find the first release including the commit, so I downloaded tarballs and use dichotomy to find gdb 7.3 version ;-)

Please update also the commit message to mention that the change adds support for gdb 7.0-7.2.

[vstinner]

@abalkin abalkin added the cherry-pick for 3.6 label 5 hours ago

No, you should use "needs backport to 3.6" label. The other label is for the cherry-pick PR ;-)

[vstinner]

@abalkin, somehow your change was committed under GitHub username abelopolsky instead of abalkin 🤔

http://bugs.python.org/user1427 is "Alexander Belopolsky" and has GitHub login "abalkin".

@abalkin: are you Alexander Belopolsky? https://github.com/abalkin says "Lev Abalkin".

I would prefer to see this identify issue fixed before merging the change.

[vstinner]

There are many except RuntimeError in this file and most of them may be gdb.error.

Note: Please don't replace RuntimeError with gdb.error: I looked at some .c file in gdb/python/, and it seems like RuntimeError is sometimes raised instead of gdb.error!

[abalkin]

@Haypo - I have a small preference for keeping this bug fix simple and just use RuntimeError as is done elsewhere in this file. If someone wants to switch to catching more specific exceptions, this can be done in a separate issue after a careful review of other uses of RuntimeError.

On the other hand, if you and @methane will come to a consensus on this – I'll follow that.

[methane]

I'm neutral, because there are many RuntimeError in this script.

I don't really care of a single except

[vstinner]

I don't really care of a single except.