Skip to content

Conversation

@arhadthedev
Copy link
Member

@arhadthedevarhadthedev commented Jan 15, 2023

This PR addresses #94687 (comment):

Todo: replace groups_size/groups parameter names with extra_group_size/extra_groups across the whole file to avoid confusion of passers-by. It's necessary to mitigate a misleading name of setgroups that supposes replacement of not supplimentary group affinities, but all ones. The mislead results in seeing the omission of setgroups call for groups_size == 0 as a bug (like we should reset the inherited affinities but do nothing instead).

I'll do it in a separate PR because both this PR and gh-94519 are already big enough to clobber them with extra details.

Edit: this PR also unifies terminology by further renaming groups_list parameter and num_groups variable into extra_groups_packed and extra_group_size.

cc @gpshead

@arhadthedevarhadthedev marked this pull request as draft January 15, 2023 09:17
@arhadthedev
Copy link
MemberAuthor

Converted to draft because num_groups and groups_list seem to require renaming too.

@arhadthedevarhadthedev marked this pull request as ready for review January 15, 2023 09:53
@arhadthedevarhadthedev marked this pull request as draft January 15, 2023 10:33
@arhadthedev
Copy link
MemberAuthor

Converted to draft again to rerun tests because spurious PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'D:\\a\\cpython\\cpython\\build\\test_python_6372�\\test_python_worker_6400�'struck again on Windows x64.

@arhadthedevarhadthedev marked this pull request as ready for review January 15, 2023 11:34
@arhadthedev
Copy link
MemberAuthor

Removed spuriously added technical _packed suffix from a Python signature of fork_exec. It's relevant inside C code only (to distinguish PyObject and gid_t * variants of extra_groups).

However, a new extra_groups argument name retains because fork_exec arguments are positional-only.

@gpsheadgpshead merged commit 73245d0 into python:mainJan 26, 2023
@arhadthedevarhadthedev deleted the _posixsubprocess-rename-groups branch January 26, 2023 07:05
mdboom pushed a commit to mdboom/cpython that referenced this pull request Jan 31, 2023
…ython#101054) * Rename `group*` to `extra_group*` to avoid confusion * Rename `num_groups` into `extra_group_size` * Rename `groups_list` to `extra_groups_packed`
Sign up for freeto 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.

3 participants

@arhadthedev@gpshead@bedevere-bot