Opened 6 years ago

Closed 4 years ago

#4920 enhancement closed fixed (fixed)

Use the stdlib `os.initgroups` if it is available

Reported by: Jean-Paul Calderone Owned by: Jean-Paul Calderone
Priority: low Milestone: Python 3.3 Minimal
Component: core Keywords:
Cc: Branch: branches/stdlib-initgroups-4920
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

os.initgroups was added to the Python 2.7 standard library. We should prefer it over our twisted.python._initgroups extension.

Attachments (1)

initgroups.patch (579 bytes) - added by Vladimir Perić 5 years ago.

Download all attachments as: .zip

Change History (10)

Changed 5 years ago by Vladimir Perić

Attachment: initgroups.patch added

comment:1 Changed 5 years ago by Vladimir Perić

Keywords: review added

As far as I can see, this is a simple matter of wrapping the current code with another "try/except" block. I've attached a patch doing just that. Not really sure how to test it, either.

comment:2 Changed 5 years ago by therve

Keywords: review removed
Owner: set to Vladimir Perić

I think the tests are covering it already, and the builders on 2.7 will pick it up. But you should change twisted/topfiles/setup.py to not build _initgroups.c on 2.7 and newer. Please add a NEWS fragment as well (and merge). Thanks!

comment:3 Changed 5 years ago by Jean-Paul Calderone

But you should change twisted/topfiles/setup.py to not build _initgroups.c on 2.7 and newer

With unit tests, of course.

comment:4 Changed 4 years ago by Itamar Turner-Trauring

Milestone: Python 3.3 Minimal

comment:5 Changed 4 years ago by Jean-Paul Calderone

Owner: changed from Vladimir Perić to Jean-Paul Calderone
Status: newassigned

comment:6 Changed 4 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/stdlib-initgroups-4920

(In [35884]) Branching to 'stdlib-initgroups-4920'

comment:7 Changed 4 years ago by Jean-Paul Calderone

Keywords: review added
Owner: changed from Jean-Paul Calderone to Itamar Turner-Trauring
Status: assignednew

comment:8 Changed 4 years ago by Itamar Turner-Trauring

Keywords: review removed
Owner: changed from Itamar Turner-Trauring to Jean-Paul Calderone

There's a failure on one of the builds: http://buildbot.twistedmatrix.com/builders/debian-easy-py2.6-epoll/builds/401/steps/epoll/logs/problems

Other than that, looks good; merge when fixed.

comment:9 Changed 4 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [35900]) Merge stdlib-initgroups-4920

Author: exarkun Reviewer: itamar Fixes: #4920

Use os.initgroups when it is available (and skip building our extension module for the same functionality in that case).

Note: See TracTickets for help on using tickets.