Skip to content

bpo-37160: Thread native ID netbsd support #13835

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

Merged
merged 3 commits into from Jun 12, 2019
Merged

bpo-37160: Thread native ID netbsd support #13835

merged 3 commits into from Jun 12, 2019

Conversation

ghost
Copy link

@ghost ghost commented Jun 5, 2019

@ghost
Copy link
Author

ghost commented Jun 5, 2019

Purposely did not update the doc yet as I do not know if it is too late for 3.8.

@ghost ghost changed the title Thread native ID netbsd support bpo-37160: Thread native ID netbsd support Jun 5, 2019
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Please update also the doc to mention NetBSD availability.

Wow, yet another implementation of "native thread id": _lwp_self!

@ghost
Copy link
Author

ghost commented Jun 5, 2019

Please update also the doc to mention NetBSD availability.

I meant should I add with the rest (thus 3.8) or create separated lines for 3.9 ?

@vstinner
Copy link
Member

vstinner commented Jun 5, 2019

I meant should I add with the rest (thus 3.8) or create separated lines for 3.9 ?

I'm fine to backport this change to 3.8. So update the 3.8 note ;-)

@@ -0,0 +1,2 @@
Thread native id support
- Adding NetBSD native call support.
Copy link
Member

Choose a reason for hiding this comment

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

I suggest:

:func:`threading.get_native_id` now also supports NetBSD.

or

Added NetBSD support to :func:`threading.get_native_id`.

@@ -0,0 +1,2 @@
Thread native id support
- :func:`threading.get_native_id` now also supports NetBSD.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that the NEWS entry written as two lines will be rendered properly. My suggestion was to only keep the second sentence: remove " Thread native id support" and remove "- " from the second line.

Copy link
Author

Choose a reason for hiding this comment

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

All right sorry will remove it.

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

@ghost
Copy link
Author

ghost commented Jun 5, 2019

Another topic. Is cpython considering Cirrus CI (can help testing FreeBSD build for example) ? I see more and more github projects adopting it.

@vstinner
Copy link
Member

vstinner commented Jun 5, 2019

@the-knights-who-say-ni the-knights-who-say-ni added the CLA not signed label just now

I don't get it. I found https://bugs.python.org/user27678 user in the bugs.python.org database which has the CLA signed and the GitHub username "dcarlier-afilias".

Note: There is also https://bugs.python.org/user27725 user which has no GitHub nickname filled but the CLA signed.

@Mariatta @csabella: Any idea what is happening with the CLA?

@vstinner
Copy link
Member

vstinner commented Jun 5, 2019

@the-knights-who-say-ni the-knights-who-say-ni added CLA not signed and removed CLA signed CLA not signed labels an hour ago

That's even more weird. The CLA was signed. The same author also wrote PR #13654 and I'm quite sure that the CLA was signed when I merged the PR. But then the bot removed the "CLA signed" label to add "CLA not signed"!?

@vstinner
Copy link
Member

vstinner commented Jun 5, 2019

I created python/core-workflow#333 for the CLA issue.

@vstinner
Copy link
Member

vstinner commented Jun 5, 2019

Another topic. Is cpython considering Cirrus CI (can help testing FreeBSD build for example) ? I see more and more github projects adopting it.

I would prefer to not have a BSD CI in pre-commit until we have enough core devs able to fix issues specific to BSD.

So far, I don't see a strong commitement to fix BSD issues as soon as possible. It's more a "best effort" commitement: we notice post-commit failures through our FreeBSD buildbots, and Pablo @pablogsal or me are doing our best when we have some free cycles to handle these issues. I'm mostly using Linux and Pablo is using macOS and Linux.

I would be fine to add OpenBSD and/or NetBSD buildbot if there is someone to fix bugs found by the buildbot. I would prefer to have a buildbot which is always red (failing) with no one taking care of it. That would only waste the time of everybody.

I'm trying to be as transparent as possible ;-) By the way, such topic should be discussed on python-dev mailing list or in the core-workflow group, like at https://discuss.python.org/c/core-workflow ;-)

@ghost
Copy link
Author

ghost commented Jun 5, 2019

@the-knights-who-say-ni the-knights-who-say-ni added CLA not signed and removed CLA signed CLA not signed labels an hour ago

That's even more weird. The CLA was signed. The same author also wrote PR #13654 and I'm quite sure that the CLA was signed when I merged the PR. But then the bot removed the "CLA signed" label to add "CLA not signed"!?

It seems to happen when you commit from a different machine than when you started the PR ... happened to me twice and this time I did not want to bother switch on my NetBSD VM just for a textual change.

@Mariatta
Copy link
Member

Mariatta commented Jun 5, 2019

One of the commits for this PR contains a commit by devnexen who has not signed CLA. Either remove the commit by devenxen, or have devnexen sign the CLA.

@vstinner
Copy link
Member

vstinner commented Jun 5, 2019

One of the commits for this PR contains a commit by devnexen who has not signed CLA. Either remove the commit by devenxen, or have devnexen sign the CLA.

Oh! I didn't notice, thanks ;-)

@terryjreedy
Copy link
Member

@Mariatta Could the Knight be upgraded to specify the GitHub username not found? Instead of just ' your GitHub username'

@Mariatta
Copy link
Member

Mariatta commented Jun 7, 2019

It's possible, I think I tried working on a solution some time ago (for python/the-knights-who-say-ni#128). I'll have to find if I still have the code somewhere...

@ghost
Copy link
Author

ghost commented Jun 12, 2019

Sorry for the mess, it is fixed now :-)

@vstinner vstinner merged commit 5287022 into python:master Jun 12, 2019
@miss-islington
Copy link
Contributor

Thanks @dcarlier-afilias for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-14021 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Jun 12, 2019
(cherry picked from commit 5287022)

Co-authored-by: David Carlier <[email protected]>
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants