Skip to content
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

gh-94518: Rename group* to extra_group* to avoid confusion #101054

Merged
merged 7 commits into from
Jan 26, 2023

Conversation

arhadthedev
Copy link
Member

@arhadthedev arhadthedev 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

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

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

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

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.

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

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.

@gpshead gpshead merged commit 73245d0 into python:main Jan 26, 2023
@arhadthedev arhadthedev 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 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.

3 participants