Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Get system user id in a lazy manner #1072

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

Merged
Byron merged 1 commit into gitpython-developers:master from athos-ribeiro:lazy_get_user
Oct 22, 2020

Conversation

Copy link

@athos-ribeiro athos-ribeiro commented Oct 20, 2020

Calling getpass.getuser may lead to breakage in environments where there
is no entries in the /etc/passwd file for the current user.

Setting the environment variables for the git user configurations should
prevents GitPython from using values from /etc/passwd. However, doing so
will not prevent reading /etc/passwd and looking for an entry with the
current user UID.

This patch changes the behavior described above so GitPython will
perform a lazy evaluation of /etc/passwd, only doing so when the
environment variables for the git user configuration are not available.

Signed-off-by: Athos Ribeiro athos@redhat.com

Calling getpass.getuser may lead to breakage in environments where there
is no entries in the /etc/passwd file for the current user.
Setting the environment variables for the git user configurations should
prevents GitPython from using values from /etc/passwd. However, doing so
will not prevent reading /etc/passwd and looking for an entry with the
current user UID.
This patch changes the behavior described above so GitPython will
perform a lazy evaluation of /etc/passwd, only doing so when the
environment variables for the git user configuration are not available.
Signed-off-by: Athos Ribeiro <athos@redhat.com>
athos-ribeiro pushed a commit to athos-ribeiro/cachito that referenced this pull request Oct 21, 2020
GitPython will check the git user related environment variables when
setting reflogs on submodule updates. This patch sets such variables so
Cachito does not need to rely on the values in the passwd file.
Moreover, with
gitpython-developers/GitPython#1072, values in
/etc/passwd will not be checked at all.
Signed-off-by: Athos Ribeiro <athos@redhat.com>
athos-ribeiro pushed a commit to containerbuildsystem/cachito that referenced this pull request Oct 21, 2020
GitPython will check the git user related environment variables when
setting reflogs on submodule updates. This patch sets such variables so
Cachito does not need to rely on the values in the passwd file.
Moreover, with
gitpython-developers/GitPython#1072, values in
/etc/passwd will not be checked at all.
Signed-off-by: Athos Ribeiro <athos@redhat.com>
@Byron Byron added this to the v3.1.10 - Bugfixes milestone Oct 22, 2020
Copy link
Member

Byron commented Oct 22, 2020

Great work, thanks a lot! It's also my first time to see the nonlocal keyword, good learning experience.

If this issue is holding you back, I am happy to create a new patch release if you request it.

@Byron Byron merged commit c96476b into gitpython-developers:master Oct 22, 2020
Copy link
Author

Great work, thanks a lot! It's also my first time to see the nonlocal keyword, good learning experience.

If this issue is holding you back, I am happy to create a new patch release if you request it.

I would really appreciate that!

As soon as that release is out, I will work to push that to Fedora (that is where I consume the package from).

Thank you :)

Copy link
Member

Byron commented Oct 23, 2020

Alright, v3.1.10 was just released to pypi.
Thanks again for your contribution.

athos-ribeiro reacted with rocket emoji

Copy link
Author

Thanks for that!

Note that a new tag was not pushed to this repository yet.

Copy link
Member

Byron commented Oct 23, 2020 via email

Thanks for the reminder. Done.
...
On 23. Oct 2020, at 16:33, Athos Ribeiro ***@***.***> wrote: Thanks for that! Note that a new tag was not pushed to this repository yet. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#1072 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAPRBWBXNQAQRJ5ZOZP3U3SME5VZANCNFSM4SYWCHHQ>.

Copy link
Author

That was fast! I was about to update my comment to say https://gitpython.readthedocs.io/en/stable/changes.html still needs updating. Let me know if you need any help with that. I am building the new Fedora packages right now.

Thanks again for the quick replies on this one!!!

return user_id

def default_name():
default_email().split('@')[0]
Copy link

@harupy harupy Oct 23, 2020
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@athos-ribeiro @Byron
(not sure if I'm really correct) Is return missing here?

Copy link

@harupy harupy Oct 23, 2020
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was investigating an error in our CI and found this patch which is included in gitpython==3.10.0 and seems related to the error.

https://github.com/mlflow/mlflow/pull/3584/checks?check_run_id=1297036420#step:4:1677

athos-ribeiro reacted with thumbs up emoji
Copy link
Author

@athos-ribeiro athos-ribeiro Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess I messed up on this one :)

Will provide a follow-up fix right away.

I am sorry for this :(

harupy reacted with thumbs up emoji
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no problem :) Thanks for the quick reponse!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks everyone. With the next PR to fix this one you will get a new patch release right away.

Copy link
Author

@athos-ribeiro athos-ribeiro Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing it in #1073.

Once again, sorry for the inconvenient

softdevm pushed a commit to softdevm/cachito-app that referenced this pull request Sep 2, 2022
GitPython will check the git user related environment variables when
setting reflogs on submodule updates. This patch sets such variables so
Cachito does not need to rely on the values in the passwd file.
Moreover, with
gitpython-developers/GitPython#1072, values in
/etc/passwd will not be checked at all.
Signed-off-by: Athos Ribeiro <athos@redhat.com>
softdev87 added a commit to softdev87/cachito-app that referenced this pull request Sep 8, 2022
GitPython will check the git user related environment variables when
setting reflogs on submodule updates. This patch sets such variables so
Cachito does not need to rely on the values in the passwd file.
Moreover, with
gitpython-developers/GitPython#1072, values in
/etc/passwd will not be checked at all.
Signed-off-by: Athos Ribeiro <athos@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Byron Byron Byron left review comments

+2 more reviewers

@harupy harupy harupy left review comments

@nirzari nirzari nirzari approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /