Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable gdbgui for python >=3.11 as it´s incompatible #25739

Closed
wants to merge 1 commit into from

Conversation

Spacefish
Copy link

GDBGUI is incompatible with Python >= 3.11 see cs01/gdbgui#447

and the similar fix done in esp-idf here:
espressif/esp-idf@f9e5272

@CLAassistant
Copy link

CLAassistant commented Mar 18, 2023

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link

PR #25739: Size comparison from 3edd101 to 2240db2

Decreases (1 build for cc32xx)
platform target config section 3edd101 2240db2 change % change
cc32xx lock CC3235SF_LAUNCHXL .debug_info 20265057 20265055 -2 -0.0
Full report (4 builds for cc32xx, mbed, qpg)
platform target config section 3edd101 2240db2 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 645945 645945 0 0.0
(read/write) 203848 203848 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197248 0 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 930399 930399 0 0.0
.debug_aranges 87520 87520 0 0.0
.debug_frame 300716 300716 0 0.0
.debug_info 20265057 20265055 -2 -0.0
.debug_line 2663587 2663587 0 0.0
.debug_loc 2808805 2808805 0 0.0
.debug_ranges 283608 283608 0 0.0
.debug_str 3036926 3036926 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 106009 106009 0 0.0
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 381423 381423 0 0.0
.symtab 257664 257664 0 0.0
.text 537816 537816 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2468688 2468688 0 0.0
.bss 215964 215964 0 0.0
.data 5880 5880 0 0.0
.text 1431332 1431332 0 0.0
qpg lighting-app qpg6105+debug (read/write) 1153852 1153852 0 0.0
.bss 100076 100076 0 0.0
.data 852 852 0 0.0
.text 600948 600948 0 0.0
lock-app qpg6105+debug (read/write) 1120716 1120716 0 0.0
.bss 96468 96468 0 0.0
.data 864 864 0 0.0
.text 567816 567816 0 0.0

jinja2<3.1
itsdangerous<2.1
python-socketio<5 ; python_version < "3.11"
gdbgui==0.13.2.0 ; platform_machine != 'aarch64' and sys_platform == 'linux' and python_version < "3.11"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't gdbgui needed for idf.py build?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bzbarsky-apple gdbgui is needed for idf.py build

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to effectively say "if you use python 3.11, you will not be able to build esp examples". However it also starts allowing any other non-esp builds ... so it seems better than before.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bzbarsky-apple gdbgui is needed for idf.py build

Not sure about that, espressif-idf removed it as well:

see: espressif/esp-idf@2685a0a
and: espressif/esp-idf@a816dfb

as well as the issue here: espressif/esp-idf#10116

i think it is only used, when you run idf.py gdbgui which won´t work with python 3.11 as gdbgui does not support Python 3.11+

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue with older esp is that idf.py validates the environment, specifically when we do a cmake generate. So even though we would compile just fine, we fail cmake at start because gdbui is missing :(

We did complain to ESP and were told that indeed this was removed in 5.0 I believe ... still waiting to get matter updated to the latest SDK and hoping then this will go away.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andy31415 IDF 5.0 is currently supported with connectedhomeip and examples should compile (#24720 #24983). Although, we have not officially moved to it as there may be some customers who still wish to use IDF v4.4 and not change at the last moment.

Will accepting this patch result into build failure for ESP builds for v4.4.x with Python 11?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With python 11 the build fails no matter what: without this patch it fails trying to install gdbui, with this patch it will fail at idf.py build time due to lack of gdbui.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch however allows bootstrap and compile for other environments ... so while esp32 will fail on py11 no matter what, without this patch everything else fails, with this patch others may pass (e.g. linux builds or other platforms that are not esp)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ideally we would have something that very clearly fails with an actionable error (e.g. "python 3.11 is not supported") in the cases when we really can't compile with it, while compiling in other cases. Our current errors are completely inscrutable.

If we can't do that, I agree that this is probably better than nothing....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhrishi - what should we do here? as this does not make things worse, it seems it should be acceptaable.

@stale
Copy link

stale bot commented Jun 14, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@andy31415
Copy link
Contributor

In the mean time we disable gdbgui globally in our builds and patched esp-idf dependencies.

@andy31415 andy31415 closed this Aug 29, 2023
@andy31415
Copy link
Contributor

also #28326 moved to esp idf 5.1 and removed gdbgui completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants