Skip to content

Make GitPython IndexFile.commit() work from a cron job #52

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

Closed

Conversation

alexfernandez
Copy link

Make GitPython IndexFile.commit() work from a cron job: use pwd.getpwduid() instead of os.getlogin(). Ref: https://groups.google.com/forum/#!msg/git-python/NU7APS5i5Xw/uziOFlneZ8gJ

@Byron
Copy link
Member

Byron commented Jun 7, 2012

Just for completeness, I post Alex's detailed email here:

Hi all,

We are having a new issue with GitPython, version 0.3.2.RC1. Still doing continuous deployment from a cool script, hopefully in a cron job. When doing a commit from an index in the cron job we were getting a silent error. However running the script manually was fine. My colleague tracked it to an error in os.getlogin() when run from cron. This StackOverflow post explains it very well. The official docs say:

For most purposes, it is more useful to use the environment variable LOGNAME to find out who the user is, or pwd.getpwuid(os.getuid())[0] to get the login name of the currently effective user id.

We have tried that, and it works, but we don't want to patch the sources locally; it would be great if you could apply the patch on the GitPython source.

The fix is quite simple: in util.py line 120, replace:

username = os.getlogin()

with

username = pwd.getpwuid(os.getuid())[0]

Of course the line above also references os.getlogin() indirectly:

if username == ukn and hasattr(os, 'getlogin'):

So you might want to change that too. The complete patch is attached.

For reference, there are a couple of workarounds. One is setting the $USER or $USERNAME environment variables (not tested). Another (and the one we have settled for) is doing the commits using

repo.git.commit('-a', '-m Autocommit')

which works just fine.

Thanks in advance,

Alex.

@maxyz
Copy link
Contributor

maxyz commented Sep 23, 2014

Why not simply replace this with getpass.getuser() 1 ?

@Byron Byron added this to the v0.3.2 milestone Nov 14, 2014
@Byron
Copy link
Member

Byron commented Nov 14, 2014

I totally agree, the current implementation is bloated and does in fact already include the behaviour of this pull request.
Therefore I believe it can be closed, and I will see that I just use getpass.getuser() instead.

@Byron Byron closed this Nov 14, 2014
Byron added a commit that referenced this pull request Nov 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants