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

Jump to bottom

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

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

wants to merge 6 commits into from
+31 −28

Conversation

arhadthedev
Copy link
Contributor

@arhadthedev arhadthedev commented Jan 15, 2023 •
edited

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 requested a review from gpshead as a code owner Jan 15, 2023
@bedevere-bot bedevere-bot mentioned this pull request Jan 15, 2023
Open
@arhadthedev arhadthedev marked this pull request as draft Jan 15, 2023
@arhadthedev
Copy link
Contributor 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 Jan 15, 2023
@arhadthedev arhadthedev marked this pull request as draft Jan 15, 2023
@arhadthedev
Copy link
Contributor 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 Jan 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gpshead gpshead

Assignees
No one assigned
Labels
awaiting review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@arhadthedev @bedevere-bot