-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't
gdbgui
needed foridf.py build
?There was a problem hiding this comment.
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 foridf.py build
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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+There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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....
There was a problem hiding this comment.
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.