gh-123596: Add missing tracebacklimit attribute to sys module

[rruuaanng]

test code:

import sys

def f():
    f()

sys.setrecursionlimit(2000)
f()

In fact, in

cpython/Python/sysmodule.c

Line 3647 in 322f14e

_PySys_UpdateConfig(PyThreadState *tstate)

the sys.tracebacklimit attribute isn't set.
However, in

cpython/Python/traceback.c

Line 722 in c3ed775

limitv = PySys_GetObject("tracebacklimit");

it tries to get the attribute and performs the same behavior in

cpython/Lib/traceback.py

Line 458 in c3ed775

limit = getattr(sys, 'tracebacklimit', None)

Therefore, the number of tracebacks is inaccurate.

• Issue: Python implementation of traceback limit does not default to 1000 as documented in 3.13 #123596

[ZeroIntensity]

Hi, submitting your own PR is uncourteous when someone said they would already. For future reference, you should have asked @blhsing whether he was still working on it.

A few other things here:

• Needs a NEWS entry--this is a user-facing change.
• Please add a test.

This is a recurring issue, so I really suggest you read the pull request section of the devguide.

Regarding the technical aspect of this PR, don't redefine PyTraceBack_LIMIT. I agree that it should be in a header file, but not pycore_call.h. The proper place to move it to is pycore_traceback.h, and then include that in sysmodule.c if it isn't already.

Thanks!

[rruuaanng]

Oh, I'm sorry, it seems that it has been a long time since it initiated the submission. It seems that it doesn't have time because it is busy. So I think maybe I can help finish it.

[rruuaanng]

Hi, submitting your own PR is uncourteous when someone said they would already. For future reference, you should have asked @blhsing whether he was still working on it.

A few other things here:

* Needs a NEWS entry--this is a user-facing change.

* Please add a test.

This is a recurring issue, so I really suggest you read the pull request section of the devguide.

Regarding the technical aspect of this PR, don't redefine PyTraceBack_LIMIT. I agree that it should be in a header file, but not pycore_call.h. The proper place to move it to is pycore_traceback.h, and then include that in sysmodule.c if it isn't already.

Thanks!

There are two reasons why I don't add NEWS:

1. It's already in the documentation. It's just not implemented in C.
2. Because it already exists in the document, the user knows about it. So I don't think it needs to be reiterated.

exist test:

cpython/Lib/test/test_sys.py

Line 1161 in e924bb6

def test_sys_tracebacklimit(self):

exist docs:

cpython/Doc/library/sys.rst

Lines 1917 to 1922 in e924bb6

	.. data:: tracebacklimit
	When this variable is set to an integer value, it determines the maximum number
	of levels of traceback information printed when an unhandled exception occurs.
	The default is ``1000``. When set to ``0`` or less, all traceback information
	is suppressed and only the exception type and value are printed.

[ZeroIntensity]

No, it's a bug that it differs from the docs, so we need to document that we fixed it with a NEWS entry.

That's the right place to put the test, but you need to add something to make sure that this issue was fixed.

[rruuaanng]

I think I'm making a change. Thank for your reply.

[rruuaanng]

It seems that there is a problem with the CI service's documentation build that does not seem to be caused by my commits. Maybe I should open an issue.

0s
Run xvfb-run make -C Doc/ PYTHON=../python SPHINXERRORHANDLING="-W --keep-going" doctest
make: Entering directory '/home/runner/work/cpython/cpython/Doc'
make[1]: Entering directory '/home/runner/work/cpython/cpython/Doc'
mkdir -p build

Missing the required blurb or sphinx-build tools.
Please run 'make venv' to install local copies.

make[1]: *** [Makefile:55: build] Error 1
make[1]: Leaving directory '/home/runner/work/cpython/cpython/Doc'
Testing of doctests in the sources finished, look at the results in build/doctest/output.txt
make: *** [Makefile:134: doctest] Error 1
make: Leaving directory '/home/runner/work/cpython/cpython/Doc'

[vstinner]

Before adding sys.tracebacklimit, I would prefer to fix #123596 : fix traceback.py limit when sys.tracebacklimit is not defined.

I understand that the fix is just about replacing getattr(sys, 'tracebacklimit', None) with getattr(sys, 'tracebacklimit', 1_000), but tests should be written for that.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[rruuaanng]

I personally think that if we only replace getattr(sys, 'tracebacklimit', None) with getattr(sys, 'tracebacklimit', 1_000), there is actually a problem that has not been fundamentally solved, that is, limitv = PySys_GetObject("tracebacklimit"); cannot obtain the corresponding attributes.
I think.

[vstinner]

I'm not against adding sys.tracebacklimit. But I would prefer to fix the bug when the attribute doesn't exist.

[rruuaanng]

Maybe I should also make changes for python so that when the tracebacklimit is really not reached, it can be set to 1000.

[vstinner]

@rruuaanng: Apparently, you don't listen to my advices, so I will stop reviewing your PR. I asked you to create a new PR to fix traceback.py and add new tests only for this change, but you ignore your request.

[rruuaanng]

Oh, I'm sorry victor. I think I misunderstood you. I'll make the change now.
Edit:
I thought you wanted me to make changes based on this PR. So I'm sorry.

[rruuaanng]

I'll rollback this PR so that it only adds the sys.tracebacklimit attribute.

[rruuaanng]

#125777

[rruuaanng]

Maybe remove the DO-NOT-MERGE if you can :)

[ZeroIntensity]

This doesn't have the DO-NOT-MERGE label. You need to trigger the bot to move it to the next stage.

[rruuaanng]

I didn't expect the Spanish Inquisition

[bedevere-app]

Nobody expects the Spanish Inquisition!

@vstinner: please review the changes made to this pull request.

[ZeroIntensity]

I think this looks ok. Take my review as provisional, though--Victor certainly knows more about the underlying issue than I do.

[rruuaanng]

Thank for your reviewer Peter!

[erlend-aasland]

Please follow Victors remark:

Before adding sys.tracebacklimit, I would prefer to fix #123596 : fix traceback.py limit when sys.tracebacklimit is not defined.

I understand that the fix is just about replacing getattr(sys, 'tracebacklimit', None) with getattr(sys, 'tracebacklimit', 1_000), but tests should be written for that.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.