bpo-21263: Skip test_gdb when python has been compiled with LLVM clang by lysnikolaou · Pull Request #10318 · python/cpython

lysnikolaou · 2018-11-04T14:09:10Z

I have also added a new field in the test support module, which enables one to find out which compiler was used to build Python.

https://bugs.python.org/issue21263

ned-deily · 2018-11-04T14:26:48Z

Do we foresee any other uses for such a utility function? If not, I'd prefer just a direct check in test_gdb.

lysnikolaou · 2018-11-04T14:30:19Z

Do we foresee any other uses for such a utility function? If not, I'd prefer just a direct check in test_gdb.

I thought it might be helpful in the future.
One other thing is that I would have to import platform in test_gdb, but it is already imported in the support module, so I didn't know if it was okay to import it once more.

ned-deily · 2018-11-04T14:34:37Z

Importing again is fine. That's the way Python works! While it is often a good idea to not repeat yourself, in this case the test is so simple that it is almost more work to create and document a utility function. I would leave it out.

lysnikolaou · 2018-11-04T14:43:17Z

Ok then. I updated my PR. Thanks a lot for your help.

lysnikolaou · 2018-11-04T15:50:36Z

Should I maybe include a news entry for this? I have never done so in the past, but I am not sure about this PR.

ned-deily

With regard to a NEWS entry, since there are already two bpo issues that have been opened about this, I suppose it makes sense to include one. If you feel comfortable doing so, you can use the blurb tool to do so. Otherwise, the core committer making the final merge could do it. Thanks!

ned-deily · 2018-11-04T18:59:29Z

Sorry, I didn't notice this before but the skip should be limited to macOS, i.e. add a sys.platform == "darwin" clause to the if test. AFAICT gdb does work OK with clang on Linux for example.

ned-deily · 2018-11-04T19:00:33Z

No need for the comma (,) there.

bedevere-bot · 2018-11-04T19:05:19Z

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.

lysnikolaou · 2018-11-04T19:20:29Z

I have made the requested changes; please review again

I have added a news entry, but I am not exactly sure about the wording.

bedevere-bot · 2018-11-04T19:20:30Z

Thanks for making the requested changes!

@ned-deily: please review the changes made to this pull request.

ned-deily

Looks good, thanks! If you like, you can add your name to Misc/ACKS and to the NEWS entry: Patch by ....

lysnikolaou · 2018-11-04T21:03:46Z

Can I directly change the NEWS entry or should I somehow use blurb to do so?

ned-deily · 2018-11-04T21:05:38Z

Either way is fine.

lysnikolaou · 2018-11-04T21:12:48Z

I just committed one last time to update Misc/ACKS and the news entry. Thanks a lot for the review and all your help.

miss-islington · 2018-11-04T21:21:30Z

Thanks @lysnikolaou for the PR, and @ned-deily for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

bedevere-bot · 2018-11-04T21:21:48Z

GH-10325 is a backport of this pull request to the 3.7 branch.

pythonGH-10318) (cherry picked from commit 59668aa) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>

miss-islington · 2018-11-04T21:23:53Z

Thanks @lysnikolaou for the PR, and @ned-deily for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖

bedevere-bot · 2018-11-04T21:24:03Z

GH-10326 is a backport of this pull request to the 3.6 branch.

pythonGH-10318) (cherry picked from commit 59668aa) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>

GH-10318) (GH-10325) (cherry picked from commit 59668aa) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>

GH-10318) (GH-10326) (cherry picked from commit 59668aa) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>

the-knights-who-say-ni added the CLA signed label Nov 4, 2018

bedevere-bot added the awaiting review label Nov 4, 2018

lysnikolaou force-pushed the bpo21263 branch from 871413b to 8e8b73dCompare November 4, 2018 14:11

lysnikolaou force-pushed the bpo21263 branch from 8e8b73d to f8afc22Compare November 4, 2018 14:42

ned-deily requested changes Nov 4, 2018
View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels Nov 4, 2018

bedevere-bot added awaiting change review and removed awaiting changes labels Nov 4, 2018

lysnikolaou force-pushed the bpo21263 branch from 73dd868 to 3ee57f6Compare November 4, 2018 19:22

ned-deily approved these changes Nov 4, 2018
View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Nov 4, 2018

Skip test_gdb on macOS when python has been compiled with LLVM clang
6301a65

lysnikolaou force-pushed the bpo21263 branch from 56b17e4 to 6301a65Compare November 4, 2018 21:07

ned-deily added the needs backport to 3.7 label Nov 4, 2018

ned-deily merged commit 59668aa into python:masterNov 4, 2018

bedevere-bot removed the awaiting merge label Nov 4, 2018

bedevere-bot removed the needs backport to 3.7 label Nov 4, 2018

ned-deily added the needs backport to 3.6 label Nov 4, 2018

lysnikolaou deleted the bpo21263 branch November 4, 2018 21:23

bedevere-bot removed the needs backport to 3.6 label Nov 4, 2018

ned-deily pushed a commit that referenced this pull request Nov 4, 2018
bpo-21263: Skip test_gdb when python has been compiled with LLVM clang (
96fb350
GH-10318) (GH-10325) (cherry picked from commit 59668aa) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>

ned-deily pushed a commit that referenced this pull request Nov 4, 2018
bpo-21263: Skip test_gdb when python has been compiled with LLVM clang (
f574ce7
GH-10318) (GH-10326) (cherry picked from commit 59668aa) Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>

sorcio mentioned this pull request Sep 6, 2023
gh-91960: skip test_gdb when built with clang #108993
Closed

Uh oh!

Uh oh!

Conversation

lysnikolaou commented Nov 4, 2018• edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ned-deily commented Nov 4, 2018

Uh oh!

lysnikolaou commented Nov 4, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ned-deily commented Nov 4, 2018

Uh oh!

lysnikolaou commented Nov 4, 2018

Uh oh!

lysnikolaou commented Nov 4, 2018

Uh oh!

ned-deily left a comment

Choose a reason for hiding this comment

Uh oh!

ned-deilyNov 4, 2018

Choose a reason for hiding this comment

Uh oh!

ned-deilyNov 4, 2018

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Nov 4, 2018

Uh oh!

lysnikolaou commented Nov 4, 2018

Uh oh!

bedevere-bot commented Nov 4, 2018

Uh oh!

ned-deily left a comment

Choose a reason for hiding this comment

Uh oh!

lysnikolaou commented Nov 4, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ned-deily commented Nov 4, 2018

Uh oh!

lysnikolaou commented Nov 4, 2018

Uh oh!

miss-islington commented Nov 4, 2018

Uh oh!

bedevere-bot commented Nov 4, 2018

Uh oh!

miss-islington commented Nov 4, 2018

Uh oh!

bedevere-bot commented Nov 4, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lysnikolaou commented Nov 4, 2018•
edited by bedevere-bot
Loading

lysnikolaou commented Nov 4, 2018•
edited
Loading

lysnikolaou commented Nov 4, 2018•
edited
Loading