Opened 4 years ago

Closed 2 years ago

#4920 enhancement closed fixed (fixed)

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

Reported by: exarkun Owned by: exarkun
Priority: low Milestone: Python 3.3 Minimal
Component: core Keywords:
Cc: Branch: branches/stdlib-initgroups-4920
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

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 vperic 3 years ago.

Download all attachments as: .zip

Change History (10)

Changed 3 years ago by vperic

comment:1 Changed 3 years ago by vperic

  • 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 3 years ago by therve

  • Keywords review removed
  • Owner set to vperic

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 3 years ago by exarkun

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 2 years ago by itamar

  • Milestone set to Python 3.3 Minimal

comment:5 Changed 2 years ago by exarkun

  • Owner changed from vperic to exarkun
  • Status changed from new to assigned

comment:6 Changed 2 years ago by exarkun

  • Author set to exarkun
  • Branch set to branches/stdlib-initgroups-4920

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

comment:7 Changed 2 years ago by exarkun

  • Keywords review added
  • Owner changed from exarkun to itamar
  • Status changed from assigned to new

comment:8 Changed 2 years ago by itamar

  • Keywords review removed
  • Owner changed from itamar to exarkun

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 2 years ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(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.