* git-clone fails when current user is not in /etc/passwd @ 2015-12-02 20:10 Taylor Braun-Jones 2015-12-09 15:23 ` Taylor Braun-Jones 2015-12-09 16:08 ` Duy Nguyen 0 siblings, 2 replies; 20+ messages in thread From: Taylor Braun-Jones @ 2015-12-02 20:10 UTC (permalink / raw) To: git My use case it running git clone inside a docker container with `docker run --user $(id -u):$(id -g) --volume /foo:/foo ...`. I want all /foo/* file creation/access from inside the Docker container to be done as the current uid/gid of the host system. Steps to reproduce: mkdir /tmp/docker-git cat > /tmp/docker-git/Dockerfile <<EOF FROM ubuntu RUN apt-get update && apt-get install -y git-core EOF docker build -t git /tmp/docker-git/ docker run --user $(id -u):$(id -g) git git clone https://github.com/git/git.git /tmp/git # fatal: unable to look up current user in the passwd file: no such user echo $? # 128 My current workaround is: cat >> /tmp/docker-git/Dockerfile <<EOF RUN git config --system user.name Docker && git config --system user.email docker@localhost EOF But I don't see why this should be necessary just to clone a repo. I run complex build jobs inside a docker container using this approach and git-clone is the first command to fail due to the lack of a passwd file entry for the current user. Some complain to stderr, but still succeed. Regards, Taylor ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: git-clone fails when current user is not in /etc/passwd 2015-12-02 20:10 git-clone fails when current user is not in /etc/passwd Taylor Braun-Jones @ 2015-12-09 15:23 ` Taylor Braun-Jones 2015-12-09 16:08 ` Duy Nguyen 1 sibling, 0 replies; 20+ messages in thread From: Taylor Braun-Jones @ 2015-12-09 15:23 UTC (permalink / raw) To: git What's the feeling on this one? If there's agreement in principle that git-clone should not fail when the current UID cannot be found in /etc/passwd then I'm happy to submit a patch to fix it. Thanks, Taylor On Wed, Dec 2, 2015 at 3:10 PM, Taylor Braun-Jones <taylor@braun-jones.org> wrote: > My use case it running git clone inside a docker container with > `docker run --user $(id -u):$(id -g) --volume /foo:/foo ...`. I want > all /foo/* file creation/access from inside the Docker container to be > done as the current uid/gid of the host system. > > Steps to reproduce: > > mkdir /tmp/docker-git > cat > /tmp/docker-git/Dockerfile <<EOF > FROM ubuntu > RUN apt-get update && apt-get install -y git-core > EOF > docker build -t git /tmp/docker-git/ > docker run --user $(id -u):$(id -g) git git clone > https://github.com/git/git.git /tmp/git > # fatal: unable to look up current user in the passwd file: no such user > echo $? # 128 > > My current workaround is: > > cat >> /tmp/docker-git/Dockerfile <<EOF > RUN git config --system user.name Docker && git config --system > user.email docker@localhost > EOF > > But I don't see why this should be necessary just to clone a repo. I > run complex build jobs inside a docker container using this approach > and git-clone is the first command to fail due to the lack of a passwd > file entry for the current user. Some complain to stderr, but still > succeed. > > Regards, > Taylor ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: git-clone fails when current user is not in /etc/passwd 2015-12-02 20:10 git-clone fails when current user is not in /etc/passwd Taylor Braun-Jones 2015-12-09 15:23 ` Taylor Braun-Jones @ 2015-12-09 16:08 ` Duy Nguyen 2015-12-09 18:24 ` Duy Nguyen 1 sibling, 1 reply; 20+ messages in thread From: Duy Nguyen @ 2015-12-09 16:08 UTC (permalink / raw) To: Taylor Braun-Jones; +Cc: Git Mailing List On Wed, Dec 2, 2015 at 9:10 PM, Taylor Braun-Jones <taylor@braun-jones.org> wrote: > My use case it running git clone inside a docker container with > `docker run --user $(id -u):$(id -g) --volume /foo:/foo ...`. I want > all /foo/* file creation/access from inside the Docker container to be > done as the current uid/gid of the host system. > > Steps to reproduce: > > mkdir /tmp/docker-git > cat > /tmp/docker-git/Dockerfile <<EOF > FROM ubuntu > RUN apt-get update && apt-get install -y git-core > EOF > docker build -t git /tmp/docker-git/ > docker run --user $(id -u):$(id -g) git git clone > https://github.com/git/git.git /tmp/git > # fatal: unable to look up current user in the passwd file: no such user It probably helps if you could get the stack trace to this message (printed from function xgetpwuid_self). I haven't checked if my personal laptop has docker to reproduce this.. In general we won't ask passwd if the user specifies name/email in the config file. But I still don't see why git-clone needs that info in the first place. -- Duy ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: git-clone fails when current user is not in /etc/passwd 2015-12-09 16:08 ` Duy Nguyen @ 2015-12-09 18:24 ` Duy Nguyen 2015-12-09 22:35 ` Taylor Braun-Jones 2015-12-10 18:43 ` Junio C Hamano 0 siblings, 2 replies; 20+ messages in thread From: Duy Nguyen @ 2015-12-09 18:24 UTC (permalink / raw) To: Taylor Braun-Jones; +Cc: Git Mailing List On Wed, Dec 9, 2015 at 5:08 PM, Duy Nguyen <pclouds@gmail.com> wrote: > On Wed, Dec 2, 2015 at 9:10 PM, Taylor Braun-Jones > <taylor@braun-jones.org> wrote: >> My use case it running git clone inside a docker container with >> `docker run --user $(id -u):$(id -g) --volume /foo:/foo ...`. I want >> all /foo/* file creation/access from inside the Docker container to be >> done as the current uid/gid of the host system. >> >> Steps to reproduce: >> >> mkdir /tmp/docker-git >> cat > /tmp/docker-git/Dockerfile <<EOF >> FROM ubuntu >> RUN apt-get update && apt-get install -y git-core >> EOF >> docker build -t git /tmp/docker-git/ >> docker run --user $(id -u):$(id -g) git git clone >> https://github.com/git/git.git /tmp/git >> # fatal: unable to look up current user in the passwd file: no such user > > It probably helps if you could get the stack trace to this message > (printed from function xgetpwuid_self). I haven't checked if my > personal laptop has docker to reproduce this.. In general we won't ask > passwd if the user specifies name/email in the config file. But I > still don't see why git-clone needs that info in the first place. Well.. reflog needs it. So either you disable reflog at clone time or define name/email via config file. I don't see anything wrong with this behavior. Stack trace (gdb) bt #0 0x0000000000585aaf in xgetpwuid_self () at wrapper.c:608 #1 0x00000000004ef2c4 in ident_default_name () at ident.c:108 #2 0x00000000004ef851 in fmt_ident (name=0x0, email=0x0, date_str=0x0, flag=0) at ident.c:300 #3 0x00000000004efb25 in git_committer_info (flag=0) at ident.c:366 #4 0x000000000052fc6f in log_ref_write_1 (refname=0x16236c0 "refs/remotes/origin/HEAD", old_sha1=0x7ffc91e44190 "", new_sha1=0x7ffc91e44170 "\355\025\033\215p{o\177V\234qX\212j?!v|}D\177", msg=0x1620e50 "clone: from https://github.com/rswier/c4.git", logfile=0x7ffc91e44100, flags=0, err=0x7ffc91e44150) at refs.c:3403 #5 0x000000000052fd85 in log_ref_write (refname=0x16236c0 "refs/remotes/origin/HEAD", old_sha1=0x7ffc91e44190 "", new_sha1=0x7ffc91e44170 "\355\025\033\215p{o\177V\234qX\212j?!v|}D\177", msg=0x1620e50 "clone: from https://github.com/rswier/c4.git", flags=0, err=0x7ffc91e44150) at refs.c:3424 #6 0x00000000005304af in create_symref (ref_target=0x16236c0 "refs/remotes/origin/HEAD", refs_heads_master=0x1624cc0 "refs/remotes/origin/master", logmsg=0x1620e50 "clone: from https://github.com/rswier/c4.git") at refs.c:3592 #7 0x000000000042e50c in update_remote_refs (refs=0x1622d40, mapped_refs=0x1622cc0, remote_head_points_at=0x1624bc0, branch_top=0x1621ca0 "refs/remotes/origin/", msg=0x1620e50 "clone: from https://github.com/rswier/c4.git", transport=0x1622960, check_connectivity=1) at builtin/clone.c:639 #8 0x000000000042f80c in cmd_clone (argc=2, argv=0x7ffc91e44ab8, prefix=0x0) at builtin/clone.c:1066 #9 0x0000000000405fda in run_builtin (p=0x825948 <commands+456>, argc=5, argv=0x7ffc91e44ab8) at git.c:352 #10 0x00000000004061d0 in handle_builtin (argc=5, argv=0x7ffc91e44ab8) at git.c:542 #11 0x00000000004062ec in run_argv (argcp=0x7ffc91e4498c, argv=0x7ffc91e449a0) at git.c:588 #12 0x00000000004064df in main (argc=5, av=0x7ffc91e44aa8) at git.c:695 -- Duy ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: git-clone fails when current user is not in /etc/passwd 2015-12-09 18:24 ` Duy Nguyen @ 2015-12-09 22:35 ` Taylor Braun-Jones 2015-12-10 18:33 ` Taylor Braun-Jones 2015-12-10 18:34 ` Jeff King 2015-12-10 18:43 ` Junio C Hamano 1 sibling, 2 replies; 20+ messages in thread From: Taylor Braun-Jones @ 2015-12-09 22:35 UTC (permalink / raw) To: Duy Nguyen; +Cc: Git Mailing List On Wed, Dec 9, 2015 at 1:24 PM, Duy Nguyen <pclouds@gmail.com> wrote: > On Wed, Dec 9, 2015 at 5:08 PM, Duy Nguyen <pclouds@gmail.com> wrote: >> On Wed, Dec 2, 2015 at 9:10 PM, Taylor Braun-Jones >> <taylor@braun-jones.org> wrote: >>> My use case it running git clone inside a docker container with >>> `docker run --user $(id -u):$(id -g) --volume /foo:/foo ...`. I want >>> all /foo/* file creation/access from inside the Docker container to be >>> done as the current uid/gid of the host system. >>> >>> Steps to reproduce: >>> >>> mkdir /tmp/docker-git >>> cat > /tmp/docker-git/Dockerfile <<EOF >>> FROM ubuntu >>> RUN apt-get update && apt-get install -y git-core >>> EOF >>> docker build -t git /tmp/docker-git/ >>> docker run --user $(id -u):$(id -g) git git clone >>> https://github.com/git/git.git /tmp/git >>> # fatal: unable to look up current user in the passwd file: no such user >> >> It probably helps if you could get the stack trace to this message >> (printed from function xgetpwuid_self). I haven't checked if my >> personal laptop has docker to reproduce this.. In general we won't ask >> passwd if the user specifies name/email in the config file. But I >> still don't see why git-clone needs that info in the first place. > > Well.. reflog needs it. So either you disable reflog at clone time or > define name/email via config file. I don't see anything wrong with > this behavior. Can't git just use "unknown" and "unknown@localhost" if the name or email can not be determined from xgetpwuid? This seems to be the way that Mercurial behaves. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: git-clone fails when current user is not in /etc/passwd 2015-12-09 22:35 ` Taylor Braun-Jones @ 2015-12-10 18:33 ` Taylor Braun-Jones 2015-12-10 18:34 ` Jeff King 1 sibling, 0 replies; 20+ messages in thread From: Taylor Braun-Jones @ 2015-12-10 18:33 UTC (permalink / raw) To: Duy Nguyen; +Cc: Git Mailing List [-- Attachment #1: Type: text/plain, Size: 1998 bytes --] On Wed, Dec 9, 2015 at 5:35 PM, Taylor Braun-Jones <taylor@braun-jones.org> wrote: > > On Wed, Dec 9, 2015 at 1:24 PM, Duy Nguyen <pclouds@gmail.com> wrote: > > On Wed, Dec 9, 2015 at 5:08 PM, Duy Nguyen <pclouds@gmail.com> wrote: > >> On Wed, Dec 2, 2015 at 9:10 PM, Taylor Braun-Jones > >> <taylor@braun-jones.org> wrote: > >>> My use case it running git clone inside a docker container with > >>> `docker run --user $(id -u):$(id -g) --volume /foo:/foo ...`. I want > >>> all /foo/* file creation/access from inside the Docker container to be > >>> done as the current uid/gid of the host system. > >>> > >>> Steps to reproduce: > >>> > >>> mkdir /tmp/docker-git > >>> cat > /tmp/docker-git/Dockerfile <<EOF > >>> FROM ubuntu > >>> RUN apt-get update && apt-get install -y git-core > >>> EOF > >>> docker build -t git /tmp/docker-git/ > >>> docker run --user $(id -u):$(id -g) git git clone > >>> https://github.com/git/git.git /tmp/git > >>> # fatal: unable to look up current user in the passwd file: no such user > >> > >> It probably helps if you could get the stack trace to this message > >> (printed from function xgetpwuid_self). I haven't checked if my > >> personal laptop has docker to reproduce this.. In general we won't ask > >> passwd if the user specifies name/email in the config file. But I > >> still don't see why git-clone needs that info in the first place. > > > > Well.. reflog needs it. So either you disable reflog at clone time or > > define name/email via config file. I don't see anything wrong with > > this behavior. > > Can't git just use "unknown" and "unknown@localhost" if the name or > email can not be determined from xgetpwuid? This seems to be the way > that Mercurial behaves. See attached patch for an implementation of the fallback-to-unknown idea. With this, I'm able to clone a repository under a UID that does not exist in /etc/passwd and do everything that I'd expect to do with a local git repository. Is there something that I've missed? Taylor [-- Attachment #2: 0001-Warn-not-die-when-unable-to-look-up-current-user-in-.patch --] [-- Type: text/x-patch, Size: 1296 bytes --] From dd5c2ccf5df4104e677f8488fbb4be98ff1831c7 Mon Sep 17 00:00:00 2001 From: Taylor Braun-Jones <taylor@braun-jones.org> Date: Thu, 10 Dec 2015 13:21:25 -0500 Subject: [PATCH] Warn (not die) when unable to look up current user in the passwd file --- wrapper.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/wrapper.c b/wrapper.c index 6fcaa4d..8119068 100644 --- a/wrapper.c +++ b/wrapper.c @@ -604,12 +604,26 @@ int access_or_die(const char *path, int mode, unsigned flag) struct passwd *xgetpwuid_self(void) { struct passwd *pw; + static struct passwd pw_unknown = {0}; + uid_t uid = getuid(); errno = 0; - pw = getpwuid(getuid()); - if (!pw) - die(_("unable to look up current user in the passwd file: %s"), - errno ? strerror(errno) : _("no such user")); + pw = getpwuid(uid); + if (!pw) { + warning(_("unable to look up current user in the passwd file: %s"), + errno ? strerror(errno) : _("no such user")); + pw_unknown.pw_name = "unknown"; + pw_unknown.pw_passwd = NULL; + pw_unknown.pw_uid = uid; + pw_unknown.pw_gid = getgid(); + pw_unknown.pw_dir = NULL; + pw_unknown.pw_shell = NULL; + #ifndef NO_GECOS_IN_PWENT + pw_unknown.pw_gecos = NULL; + #endif + pw = &pw_unknown; + } + return pw; } -- 2.6.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: git-clone fails when current user is not in /etc/passwd 2015-12-09 22:35 ` Taylor Braun-Jones 2015-12-10 18:33 ` Taylor Braun-Jones @ 2015-12-10 18:34 ` Jeff King 2015-12-10 19:57 ` Junio C Hamano 1 sibling, 1 reply; 20+ messages in thread From: Jeff King @ 2015-12-10 18:34 UTC (permalink / raw) To: Taylor Braun-Jones; +Cc: Duy Nguyen, Git Mailing List On Wed, Dec 09, 2015 at 05:35:52PM -0500, Taylor Braun-Jones wrote: > > Well.. reflog needs it. So either you disable reflog at clone time or > > define name/email via config file. I don't see anything wrong with > > this behavior. > > Can't git just use "unknown" and "unknown@localhost" if the name or > email can not be determined from xgetpwuid? This seems to be the way > that Mercurial behaves. Yeah, I think there is a mismatch here in the ident code. When we are making a commit (which is likely to get published and is set in stone), we ask for "IDENT_STRICT", which will barf if we can't come up with something sensible. For reflogs, we are OK putting whatever auto-generated crap in there we can come up with. But before we even hit the strict-check, we call xgetpwuid_self(), which unconditionally dies on failure. I think that function needs to be taught a "gently" form which we use for non-strict ident lookups. Unfortunately it's a little non-trivial because the strictness will need to get passed all the way down to ident_default_name() (and we need to make sure that a non-strict check followed by a strict one does not fail; i.e., that the first does not pollute the contents of git_default_name). -Peff ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: git-clone fails when current user is not in /etc/passwd 2015-12-10 18:34 ` Jeff King @ 2015-12-10 19:57 ` Junio C Hamano 2015-12-10 20:40 ` Jeff King 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2015-12-10 19:57 UTC (permalink / raw) To: Jeff King; +Cc: Taylor Braun-Jones, Duy Nguyen, Git Mailing List Jeff King <peff@peff.net> writes: > But before we even hit the strict-check, we call xgetpwuid_self(), which > unconditionally dies on failure. I think that function needs to be > taught a "gently" form which we use for non-strict ident lookups. > Unfortunately it's a little non-trivial because the strictness will need > to get passed all the way down to ident_default_name() (and we need to > make sure that a non-strict check followed by a strict one does not > fail; i.e., that the first does not pollute the contents of > git_default_name). All true. The adding of "(none)" in add_domainname() I used as an excuse to make the function stop barfing is a symptom coming from the above. That one should die when asked to do a strict thing (there is a corresponding kludge to do strstr "(none)" to cover it up, which is even uglier X-<). ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: git-clone fails when current user is not in /etc/passwd 2015-12-10 19:57 ` Junio C Hamano @ 2015-12-10 20:40 ` Jeff King 2015-12-10 21:32 ` [PATCH 0/3] " Jeff King 0 siblings, 1 reply; 20+ messages in thread From: Jeff King @ 2015-12-10 20:40 UTC (permalink / raw) To: Junio C Hamano; +Cc: Taylor Braun-Jones, Duy Nguyen, Git Mailing List On Thu, Dec 10, 2015 at 11:57:41AM -0800, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > But before we even hit the strict-check, we call xgetpwuid_self(), which > > unconditionally dies on failure. I think that function needs to be > > taught a "gently" form which we use for non-strict ident lookups. > > Unfortunately it's a little non-trivial because the strictness will need > > to get passed all the way down to ident_default_name() (and we need to > > make sure that a non-strict check followed by a strict one does not > > fail; i.e., that the first does not pollute the contents of > > git_default_name). > > All true. The adding of "(none)" in add_domainname() I used as an > excuse to make the function stop barfing is a symptom coming from > the above. That one should die when asked to do a strict thing > (there is a corresponding kludge to do strstr "(none)" to cover it > up, which is even uglier X-<). Yes, I have always hated that. It seems like we should just be able to carry a "strict" flag (both from xgetpwuid() and from the "(none)" hack) along with ident_default_*, and barf at the right time if it is not set. -Peff ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/3] git-clone fails when current user is not in /etc/passwd 2015-12-10 20:40 ` Jeff King @ 2015-12-10 21:32 ` Jeff King 2015-12-10 21:33 ` [PATCH 1/3] ident: make xgetpwuid_self() a static local helper Jeff King ` (4 more replies) 0 siblings, 5 replies; 20+ messages in thread From: Jeff King @ 2015-12-10 21:32 UTC (permalink / raw) To: Junio C Hamano; +Cc: Taylor Braun-Jones, Duy Nguyen, Git Mailing List On Thu, Dec 10, 2015 at 03:40:35PM -0500, Jeff King wrote: > > All true. The adding of "(none)" in add_domainname() I used as an > > excuse to make the function stop barfing is a symptom coming from > > the above. That one should die when asked to do a strict thing > > (there is a corresponding kludge to do strstr "(none)" to cover it > > up, which is even uglier X-<). > > Yes, I have always hated that. It seems like we should just be able to > carry a "strict" flag (both from xgetpwuid() and from the "(none)" hack) > along with ident_default_*, and barf at the right time if it is not set. I don't think we want to pass down a "be strict" flag to the low-level code filling in default_{name,email}. We might be strict in one call, and non-strict in another within the same program. Worse, we actually call ident_default_name() even when we don't actually want the name (we could fix that, too, but it just adds more cases to the code). So here's my solution, which instead carries the "is it bogus" flag along with the default strings. [1/3]: ident: make xgetpwuid_self() a static local helper [2/3]: ident: keep a flag for bogus default_email [3/3]: ident: loosen getpwuid error in non-strict mode -Peff ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] ident: make xgetpwuid_self() a static local helper 2015-12-10 21:32 ` [PATCH 0/3] " Jeff King @ 2015-12-10 21:33 ` Jeff King 2015-12-10 23:39 ` Junio C Hamano 2015-12-10 21:35 ` [PATCH 2/3] ident: keep a flag for bogus default_email Jeff King ` (3 subsequent siblings) 4 siblings, 1 reply; 20+ messages in thread From: Jeff King @ 2015-12-10 21:33 UTC (permalink / raw) To: Junio C Hamano; +Cc: Taylor Braun-Jones, Duy Nguyen, Git Mailing List This function is defined in wrapper.c, but nobody besides ident.c uses it. And nobody is likely to in the future, either, as anything that cares about the user's name should be going through the ident code. Moving it here is a cleanup of the global namespace, but it will also enable further cleanups inside ident.c. Signed-off-by: Jeff King <peff@peff.net> --- git-compat-util.h | 3 --- ident.c | 12 ++++++++++++ wrapper.c | 12 ------------ 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index 8e39867..2da0a75 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -929,9 +929,6 @@ int access_or_die(const char *path, int mode, unsigned flag); /* Warn on an inaccessible file that ought to be accessible */ void warn_on_inaccessible(const char *path); -/* Get the passwd entry for the UID of the current process. */ -struct passwd *xgetpwuid_self(void); - #ifdef GMTIME_UNRELIABLE_ERRORS struct tm *git_gmtime(const time_t *); struct tm *git_gmtime_r(const time_t *, struct tm *); diff --git a/ident.c b/ident.c index 5ff1aad..d7c70e2 100644 --- a/ident.c +++ b/ident.c @@ -23,6 +23,18 @@ static int author_ident_explicitly_given; #define get_gecos(struct_passwd) ((struct_passwd)->pw_gecos) #endif +static struct passwd *xgetpwuid_self(void) +{ + struct passwd *pw; + + errno = 0; + pw = getpwuid(getuid()); + if (!pw) + die(_("unable to look up current user in the passwd file: %s"), + errno ? strerror(errno) : _("no such user")); + return pw; +} + static void copy_gecos(const struct passwd *w, struct strbuf *name) { char *src; diff --git a/wrapper.c b/wrapper.c index 6fcaa4d..c95e290 100644 --- a/wrapper.c +++ b/wrapper.c @@ -601,18 +601,6 @@ int access_or_die(const char *path, int mode, unsigned flag) return ret; } -struct passwd *xgetpwuid_self(void) -{ - struct passwd *pw; - - errno = 0; - pw = getpwuid(getuid()); - if (!pw) - die(_("unable to look up current user in the passwd file: %s"), - errno ? strerror(errno) : _("no such user")); - return pw; -} - char *xgetcwd(void) { struct strbuf sb = STRBUF_INIT; -- 2.6.3.636.g1460207 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] ident: make xgetpwuid_self() a static local helper 2015-12-10 21:33 ` [PATCH 1/3] ident: make xgetpwuid_self() a static local helper Jeff King @ 2015-12-10 23:39 ` Junio C Hamano 0 siblings, 0 replies; 20+ messages in thread From: Junio C Hamano @ 2015-12-10 23:39 UTC (permalink / raw) To: Jeff King; +Cc: Taylor Braun-Jones, Duy Nguyen, Git Mailing List Makes tons of sense (even without patches 2 and 3). Thanks. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/3] ident: keep a flag for bogus default_email 2015-12-10 21:32 ` [PATCH 0/3] " Jeff King 2015-12-10 21:33 ` [PATCH 1/3] ident: make xgetpwuid_self() a static local helper Jeff King @ 2015-12-10 21:35 ` Jeff King 2015-12-10 22:54 ` Jeff King 2015-12-10 21:41 ` [PATCH 3/3] ident: loosen getpwuid error in non-strict mode Jeff King ` (2 subsequent siblings) 4 siblings, 1 reply; 20+ messages in thread From: Jeff King @ 2015-12-10 21:35 UTC (permalink / raw) To: Junio C Hamano; +Cc: Taylor Braun-Jones, Duy Nguyen, Git Mailing List If we have to deduce the user's email address and can't come up with something plausible for the hostname, we simply write "(none)" or ".(none)" in the hostname. Later, our strict-check is forced to use strstr to look for this magic string. This is probably not a problem in practice, but it's rather ugly. Let's keep an extra flag that tells us the email is bogus, and check that instead. We could get away with simply setting the global in add_domainname(); it only gets called to write into git_default_email. However, let's make the code a little more obvious to future readers by actually passing a pointer to our "bogus" flag down the call-chain. Signed-off-by: Jeff King <peff@peff.net> --- I waffled on the "passing it around" thing. The patch is a lot smaller if we just set the global directly, and I doubt these static helpers will ever get reused for anything else. But the pattern does get reused in the next patch, which sets the "email" and "name" flags separately depending on context. I was also tempted to have a single "default_ident_is_flaky", but that does get some corner cases wrong (e.g., you have a bogus auto-guessed name, an OK auto-guessed email, and set GIT_COMMITTER_NAME but not GIT_COMMITTER_EMAIL). I doubt those come up in practice, but it's not that hard to do the right thing. ident.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/ident.c b/ident.c index d7c70e2..085cfbe 100644 --- a/ident.c +++ b/ident.c @@ -10,6 +10,7 @@ static struct strbuf git_default_name = STRBUF_INIT; static struct strbuf git_default_email = STRBUF_INIT; static struct strbuf git_default_date = STRBUF_INIT; +static int default_email_is_bogus; #define IDENT_NAME_GIVEN 01 #define IDENT_MAIL_GIVEN 02 @@ -82,7 +83,7 @@ static int add_mailname_host(struct strbuf *buf) return 0; } -static void add_domainname(struct strbuf *out) +static void add_domainname(struct strbuf *out, int *is_bogus) { char buf[1024]; struct hostent *he; @@ -90,17 +91,21 @@ static void add_domainname(struct strbuf *out) if (gethostname(buf, sizeof(buf))) { warning("cannot get host name: %s", strerror(errno)); strbuf_addstr(out, "(none)"); + *is_bogus = 1; return; } if (strchr(buf, '.')) strbuf_addstr(out, buf); else if ((he = gethostbyname(buf)) && strchr(he->h_name, '.')) strbuf_addstr(out, he->h_name); - else + else { strbuf_addf(out, "%s.(none)", buf); + *is_bogus = 1; + } } -static void copy_email(const struct passwd *pw, struct strbuf *email) +static void copy_email(const struct passwd *pw, struct strbuf *email, + int *is_bogus) { /* * Make up a fake email address @@ -111,7 +116,7 @@ static void copy_email(const struct passwd *pw, struct strbuf *email) if (!add_mailname_host(email)) return; /* read from "/etc/mailname" (Debian) */ - add_domainname(email); + add_domainname(email, is_bogus); } const char *ident_default_name(void) @@ -133,7 +138,8 @@ const char *ident_default_email(void) committer_ident_explicitly_given |= IDENT_MAIL_GIVEN; author_ident_explicitly_given |= IDENT_MAIL_GIVEN; } else - copy_email(xgetpwuid_self(), &git_default_email); + copy_email(xgetpwuid_self(), &git_default_email, + &default_email_is_bogus); strbuf_trim(&git_default_email); } return git_default_email.buf; @@ -325,8 +331,7 @@ const char *fmt_ident(const char *name, const char *email, name = pw->pw_name; } - if (strict && email == git_default_email.buf && - strstr(email, "(none)")) { + if (strict && email == git_default_email.buf && default_email_is_bogus) { fputs(env_hint, stderr); die("unable to auto-detect email address (got '%s')", email); } -- 2.6.3.636.g1460207 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] ident: keep a flag for bogus default_email 2015-12-10 21:35 ` [PATCH 2/3] ident: keep a flag for bogus default_email Jeff King @ 2015-12-10 22:54 ` Jeff King 0 siblings, 0 replies; 20+ messages in thread From: Jeff King @ 2015-12-10 22:54 UTC (permalink / raw) To: Junio C Hamano; +Cc: Taylor Braun-Jones, Duy Nguyen, Git Mailing List On Thu, Dec 10, 2015 at 04:35:35PM -0500, Jeff King wrote: > @@ -82,7 +83,7 @@ static int add_mailname_host(struct strbuf *buf) > return 0; > } > > -static void add_domainname(struct strbuf *out) > +static void add_domainname(struct strbuf *out, int *is_bogus) > { > char buf[1024]; > struct hostent *he; > @@ -90,17 +91,21 @@ static void add_domainname(struct strbuf *out) > if (gethostname(buf, sizeof(buf))) { > warning("cannot get host name: %s", strerror(errno)); > strbuf_addstr(out, "(none)"); > + *is_bogus = 1; > return; > } > if (strchr(buf, '.')) > strbuf_addstr(out, buf); > else if ((he = gethostbyname(buf)) && strchr(he->h_name, '.')) > strbuf_addstr(out, he->h_name); > - else > + else { > strbuf_addf(out, "%s.(none)", buf); > + *is_bogus = 1; > + } This hunk has some minor conflicts with ep/ident-with-getaddrinfo, which graduated to master in the tips you just pushed out. Here's a rebased version on top of the current master (the other patches can remain the same). -- >8 -- Subject: [PATCH] ident: keep a flag for bogus default_email If we have to deduce the user's email address and can't come up with something plausible for the hostname, we simply write "(none)" or ".(none)" in the hostname. Later, our strict-check is forced to use strstr to look for this magic string. This is probably not a problem in practice, but it's rather ugly. Let's keep an extra flag that tells us the email is bogus, and check that instead. We could get away with simply setting the global in add_domainname(); it only gets called to write into git_default_email. However, let's make the code a little more obvious to future readers by actually passing a pointer to our "bogus" flag down the call-chain. Signed-off-by: Jeff King <peff@peff.net> --- ident.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/ident.c b/ident.c index 140dd63..fde3244 100644 --- a/ident.c +++ b/ident.c @@ -10,6 +10,7 @@ static struct strbuf git_default_name = STRBUF_INIT; static struct strbuf git_default_email = STRBUF_INIT; static struct strbuf git_default_date = STRBUF_INIT; +static int default_email_is_bogus; #define IDENT_NAME_GIVEN 01 #define IDENT_MAIL_GIVEN 02 @@ -108,22 +109,26 @@ static int canonical_name(const char *host, struct strbuf *out) return status; } -static void add_domainname(struct strbuf *out) +static void add_domainname(struct strbuf *out, int *is_bogus) { char buf[1024]; if (gethostname(buf, sizeof(buf))) { warning("cannot get host name: %s", strerror(errno)); strbuf_addstr(out, "(none)"); + *is_bogus = 1; return; } if (strchr(buf, '.')) strbuf_addstr(out, buf); - else if (canonical_name(buf, out) < 0) + else if (canonical_name(buf, out) < 0) { strbuf_addf(out, "%s.(none)", buf); + *is_bogus = 1; + } } -static void copy_email(const struct passwd *pw, struct strbuf *email) +static void copy_email(const struct passwd *pw, struct strbuf *email, + int *is_bogus) { /* * Make up a fake email address @@ -134,7 +139,7 @@ static void copy_email(const struct passwd *pw, struct strbuf *email) if (!add_mailname_host(email)) return; /* read from "/etc/mailname" (Debian) */ - add_domainname(email); + add_domainname(email, is_bogus); } const char *ident_default_name(void) @@ -156,7 +161,8 @@ const char *ident_default_email(void) committer_ident_explicitly_given |= IDENT_MAIL_GIVEN; author_ident_explicitly_given |= IDENT_MAIL_GIVEN; } else - copy_email(xgetpwuid_self(), &git_default_email); + copy_email(xgetpwuid_self(), &git_default_email, + &default_email_is_bogus); strbuf_trim(&git_default_email); } return git_default_email.buf; @@ -348,8 +354,7 @@ const char *fmt_ident(const char *name, const char *email, name = pw->pw_name; } - if (strict && email == git_default_email.buf && - strstr(email, "(none)")) { + if (strict && email == git_default_email.buf && default_email_is_bogus) { fputs(env_hint, stderr); die("unable to auto-detect email address (got '%s')", email); } -- 2.6.3.636.g1460207 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] ident: loosen getpwuid error in non-strict mode 2015-12-10 21:32 ` [PATCH 0/3] " Jeff King 2015-12-10 21:33 ` [PATCH 1/3] ident: make xgetpwuid_self() a static local helper Jeff King 2015-12-10 21:35 ` [PATCH 2/3] ident: keep a flag for bogus default_email Jeff King @ 2015-12-10 21:41 ` Jeff King 2015-12-14 15:07 ` Jeff King 2015-12-10 23:44 ` [PATCH 0/3] git-clone fails when current user is not in /etc/passwd Junio C Hamano 2015-12-11 2:20 ` Taylor Braun-Jones 4 siblings, 1 reply; 20+ messages in thread From: Jeff King @ 2015-12-10 21:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: Taylor Braun-Jones, Duy Nguyen, Git Mailing List If the user has not specified an identity and we have to turn to getpwuid() to find the username or gecos field, we die immediately when getpwuid fails (e.g., because the user does not exist). This is OK for making a commit, where we have set IDENT_STRICT and would want to bail on bogus input. But for something like a reflog, where the ident is "best effort", it can be pain. For instance, even running "git clone" with a UID that is not in /etc/passwd will result in git barfing, just because we can't find an ident to put in the reflog. Instead of dying in xgetpwuid_self, we can instead return a fallback value, and set a "bogus" flag. For the username in an email, we already have a "default_email_is_bogus" flag. For the name field, we introduce (and check) a matching "default_name_is_bogus" flag. As a bonus, this means you now get the usual "tell me who you are" advice instead of just a "no such user" error. No tests, as this is dependent on configuration outside of git's control. However, I did confirm that it behaves sensibly when I delete myself from the local /etc/passwd (reflogs get written, and commits complain). Signed-off-by: Jeff King <peff@peff.net> --- I dislike #ifdefs in the middle of functions like the one below, but I couldn't come up with an elegant way to avoid it. We're reusing "struct passwd" for our fallback, so we need to know whether we have that field to set or not. An obvious alternative would be to always return our own struct git_passwd_struct { const char *name; const char *gecos; }; and translate the existing getpwuid() on the fly. That still needs an #ifdef, but we could then get rid of the #ifdef for the get_gecos() macro. Yet another alternative is to simply zero the fallback passwd struct, and pw->pw_gecos will either not exist, or will be NULL. But then the callers have to do something sane with the NULL, and the whole point of this exercise was to avoid callers having to deal with the error cases directly, so... ident.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/ident.c b/ident.c index 085cfbe..2d5b876 100644 --- a/ident.c +++ b/ident.c @@ -11,6 +11,7 @@ static struct strbuf git_default_name = STRBUF_INIT; static struct strbuf git_default_email = STRBUF_INIT; static struct strbuf git_default_date = STRBUF_INIT; static int default_email_is_bogus; +static int default_name_is_bogus; #define IDENT_NAME_GIVEN 01 #define IDENT_MAIL_GIVEN 02 @@ -24,15 +25,22 @@ static int author_ident_explicitly_given; #define get_gecos(struct_passwd) ((struct_passwd)->pw_gecos) #endif -static struct passwd *xgetpwuid_self(void) +static struct passwd *xgetpwuid_self(int *is_bogus) { struct passwd *pw; errno = 0; pw = getpwuid(getuid()); - if (!pw) - die(_("unable to look up current user in the passwd file: %s"), - errno ? strerror(errno) : _("no such user")); + if (!pw) { + struct passwd fallback; + fallback.pw_name = "unknown"; +#ifndef NO_GECOS_IN_PWENT + fallback.pw_gecos = "Unknown"; +#endif + pw = &fallback; + if (is_bogus) + *is_bogus = 1; + } return pw; } @@ -122,7 +130,7 @@ static void copy_email(const struct passwd *pw, struct strbuf *email, const char *ident_default_name(void) { if (!git_default_name.len) { - copy_gecos(xgetpwuid_self(), &git_default_name); + copy_gecos(xgetpwuid_self(&default_name_is_bogus), &git_default_name); strbuf_trim(&git_default_name); } return git_default_name.buf; @@ -138,8 +146,8 @@ const char *ident_default_email(void) committer_ident_explicitly_given |= IDENT_MAIL_GIVEN; author_ident_explicitly_given |= IDENT_MAIL_GIVEN; } else - copy_email(xgetpwuid_self(), &git_default_email, - &default_email_is_bogus); + copy_email(xgetpwuid_self(&default_email_is_bogus), + &git_default_email, &default_email_is_bogus); strbuf_trim(&git_default_email); } return git_default_email.buf; @@ -327,10 +335,16 @@ const char *fmt_ident(const char *name, const char *email, fputs(env_hint, stderr); die("empty ident name (for <%s>) not allowed", email); } - pw = xgetpwuid_self(); + pw = xgetpwuid_self(NULL); name = pw->pw_name; } + if (want_name && strict && + name == git_default_name.buf && default_name_is_bogus) { + fputs(env_hint, stderr); + die("unable to auto-detect name (got '%s')", name); + } + if (strict && email == git_default_email.buf && default_email_is_bogus) { fputs(env_hint, stderr); die("unable to auto-detect email address (got '%s')", email); -- 2.6.3.636.g1460207 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] ident: loosen getpwuid error in non-strict mode 2015-12-10 21:41 ` [PATCH 3/3] ident: loosen getpwuid error in non-strict mode Jeff King @ 2015-12-14 15:07 ` Jeff King 0 siblings, 0 replies; 20+ messages in thread From: Jeff King @ 2015-12-14 15:07 UTC (permalink / raw) To: Junio C Hamano; +Cc: Taylor Braun-Jones, Duy Nguyen, Git Mailing List On Thu, Dec 10, 2015 at 04:41:29PM -0500, Jeff King wrote: > -static struct passwd *xgetpwuid_self(void) > +static struct passwd *xgetpwuid_self(int *is_bogus) > { > struct passwd *pw; > > errno = 0; > pw = getpwuid(getuid()); > - if (!pw) > - die(_("unable to look up current user in the passwd file: %s"), > - errno ? strerror(errno) : _("no such user")); > + if (!pw) { > + struct passwd fallback; > + fallback.pw_name = "unknown"; > +#ifndef NO_GECOS_IN_PWENT > + fallback.pw_gecos = "Unknown"; > +#endif > + pw = &fallback; > + if (is_bogus) > + *is_bogus = 1; > + } > return pw; Ugh. The fallback struct should be static, of course, as we are returning its address from the function. Anybody have a brown paper bag I can borrow? diff --git a/ident.c b/ident.c index 74de079..831072c 100644 --- a/ident.c +++ b/ident.c @@ -32,7 +32,7 @@ static struct passwd *xgetpwuid_self(int *is_bogus) errno = 0; pw = getpwuid(getuid()); if (!pw) { - struct passwd fallback; + static struct passwd fallback; fallback.pw_name = "unknown"; #ifndef NO_GECOS_IN_PWENT fallback.pw_gecos = "Unknown"; -Peff ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] git-clone fails when current user is not in /etc/passwd 2015-12-10 21:32 ` [PATCH 0/3] " Jeff King ` (2 preceding siblings ...) 2015-12-10 21:41 ` [PATCH 3/3] ident: loosen getpwuid error in non-strict mode Jeff King @ 2015-12-10 23:44 ` Junio C Hamano 2015-12-11 2:20 ` Taylor Braun-Jones 4 siblings, 0 replies; 20+ messages in thread From: Junio C Hamano @ 2015-12-10 23:44 UTC (permalink / raw) To: Jeff King; +Cc: Taylor Braun-Jones, Duy Nguyen, Git Mailing List Jeff King <peff@peff.net> writes: > I don't think we want to pass down a "be strict" flag to the low-level > code filling in default_{name,email}. We might be strict in one call, > and non-strict in another within the same program. Worse, we actually > call ident_default_name() even when we don't actually want the name (we > could fix that, too, but it just adds more cases to the code). > > So here's my solution, which instead carries the "is it bogus" flag > along with the default strings. > > [1/3]: ident: make xgetpwuid_self() a static local helper > [2/3]: ident: keep a flag for bogus default_email > [3/3]: ident: loosen getpwuid error in non-strict mode Thanks. Looks sensible. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] git-clone fails when current user is not in /etc/passwd 2015-12-10 21:32 ` [PATCH 0/3] " Jeff King ` (3 preceding siblings ...) 2015-12-10 23:44 ` [PATCH 0/3] git-clone fails when current user is not in /etc/passwd Junio C Hamano @ 2015-12-11 2:20 ` Taylor Braun-Jones 4 siblings, 0 replies; 20+ messages in thread From: Taylor Braun-Jones @ 2015-12-11 2:20 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Duy Nguyen, Git Mailing List On Thu, Dec 10, 2015 at 4:32 PM, Jeff King <peff@peff.net> wrote: > So here's my solution, which instead carries the "is it bogus" flag > along with the default strings. > > [1/3]: ident: make xgetpwuid_self() a static local helper > [2/3]: ident: keep a flag for bogus default_email > [3/3]: ident: loosen getpwuid error in non-strict mode 1 definitely makes sense to me. As a newcomer to the git code base, I was wondering why it wasn't this way already. 2 and 3 solve my original problem - thanks! Taylor ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: git-clone fails when current user is not in /etc/passwd 2015-12-09 18:24 ` Duy Nguyen 2015-12-09 22:35 ` Taylor Braun-Jones @ 2015-12-10 18:43 ` Junio C Hamano 2015-12-10 18:52 ` Taylor Braun-Jones 1 sibling, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2015-12-10 18:43 UTC (permalink / raw) To: Duy Nguyen; +Cc: Taylor Braun-Jones, Git Mailing List Duy Nguyen <pclouds@gmail.com> writes: > Well.. reflog needs it. So either you disable reflog at clone time or > define name/email via config file. I don't see anything wrong with > this behavior. Hmm, I am not quite sure about that. In the codepath that computes ident_default_email(), which is one half of what the "reflog" code you cite wants to do, copy_email() calls copy_email() which in turn calls add_domainname(). If your getpwuid() gave you some username, but your gethostname() gave you a NULL, we do not barf but add "(none)" as and then issue a warning. Perhaps we can do the same by returning a structure with members set to a set of fake values. Because we already have checks in places that really matter to the recorded history (read: not reflog) in the form of *_ident_sufficiently_given() functions, potential damage due to having phoney names returned from here would not be too bad. Totally untested... ident.c | 13 ++++++++++--- wrapper.c | 4 ---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/ident.c b/ident.c index 4e7f99d..2ccae2c 100644 --- a/ident.c +++ b/ident.c @@ -31,7 +31,7 @@ static void copy_gecos(const struct passwd *w, struct strbuf *name) * with commas. Also & stands for capitalized form of the login name. */ - for (src = get_gecos(w); *src && *src != ','; src++) { + for (src = w ? get_gecos(w) : "&"; *src && *src != ','; src++) { int ch = *src; if (ch != '&') strbuf_addch(name, ch); @@ -117,7 +117,7 @@ static void copy_email(const struct passwd *pw, struct strbuf *email) * Make up a fake email address * (name + '@' + hostname [+ '.' + domainname]) */ - strbuf_addstr(email, pw->pw_name); + strbuf_addstr(email, pw ? pw->pw_name : "unknown"); strbuf_addch(email, '@'); if (!add_mailname_host(email)) @@ -332,8 +332,15 @@ const char *fmt_ident(const char *name, const char *email, fputs(env_hint, stderr); die("empty ident name (for <%s>) not allowed", email); } + errno = 0; pw = xgetpwuid_self(); - name = pw->pw_name; + if (!pw) { + warning(_("unable to look up current user: %s"), + errno ? strerror(errno) : _("no such user")); + name = "unknown"; + } else { + name = pw->pw_name; + } } if (strict && email == git_default_email.buf && diff --git a/wrapper.c b/wrapper.c index 6fcaa4d..16ab45f 100644 --- a/wrapper.c +++ b/wrapper.c @@ -605,11 +605,7 @@ struct passwd *xgetpwuid_self(void) { struct passwd *pw; - errno = 0; pw = getpwuid(getuid()); - if (!pw) - die(_("unable to look up current user in the passwd file: %s"), - errno ? strerror(errno) : _("no such user")); return pw; } ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: git-clone fails when current user is not in /etc/passwd 2015-12-10 18:43 ` Junio C Hamano @ 2015-12-10 18:52 ` Taylor Braun-Jones 0 siblings, 0 replies; 20+ messages in thread From: Taylor Braun-Jones @ 2015-12-10 18:52 UTC (permalink / raw) To: Junio C Hamano; +Cc: Duy Nguyen, Git Mailing List On Thu, Dec 10, 2015 at 1:43 PM, Junio C Hamano <gitster@pobox.com> wrote: > Duy Nguyen <pclouds@gmail.com> writes: > >> Well.. reflog needs it. So either you disable reflog at clone time or >> define name/email via config file. I don't see anything wrong with >> this behavior. > > Hmm, I am not quite sure about that. > > In the codepath that computes ident_default_email(), which is one > half of what the "reflog" code you cite wants to do, copy_email() > calls copy_email() which in turn calls add_domainname(). If your > getpwuid() gave you some username, but your gethostname() gave you a > NULL, we do not barf but add "(none)" as and then issue a warning. > > Perhaps we can do the same by returning a structure with members set > to a set of fake values. Because we already have checks in places > that really matter to the recorded history (read: not reflog) in the > form of *_ident_sufficiently_given() functions, potential damage due > to having phoney names returned from here would not be too bad. > > Totally untested... > > ident.c | 13 ++++++++++--- > wrapper.c | 4 ---- > 2 files changed, 10 insertions(+), 7 deletions(-) > > diff --git a/ident.c b/ident.c > index 4e7f99d..2ccae2c 100644 > --- a/ident.c > +++ b/ident.c > @@ -31,7 +31,7 @@ static void copy_gecos(const struct passwd *w, struct strbuf *name) > * with commas. Also & stands for capitalized form of the login name. > */ > > - for (src = get_gecos(w); *src && *src != ','; src++) { > + for (src = w ? get_gecos(w) : "&"; *src && *src != ','; src++) { > int ch = *src; > if (ch != '&') > strbuf_addch(name, ch); > @@ -117,7 +117,7 @@ static void copy_email(const struct passwd *pw, struct strbuf *email) > * Make up a fake email address > * (name + '@' + hostname [+ '.' + domainname]) > */ > - strbuf_addstr(email, pw->pw_name); > + strbuf_addstr(email, pw ? pw->pw_name : "unknown"); > strbuf_addch(email, '@'); > > if (!add_mailname_host(email)) > @@ -332,8 +332,15 @@ const char *fmt_ident(const char *name, const char *email, > fputs(env_hint, stderr); > die("empty ident name (for <%s>) not allowed", email); > } > + errno = 0; > pw = xgetpwuid_self(); > - name = pw->pw_name; > + if (!pw) { > + warning(_("unable to look up current user: %s"), > + errno ? strerror(errno) : _("no such user")); > + name = "unknown"; > + } else { > + name = pw->pw_name; > + } > } > > if (strict && email == git_default_email.buf && > diff --git a/wrapper.c b/wrapper.c > index 6fcaa4d..16ab45f 100644 > --- a/wrapper.c > +++ b/wrapper.c > @@ -605,11 +605,7 @@ struct passwd *xgetpwuid_self(void) > { > struct passwd *pw; > > - errno = 0; > pw = getpwuid(getuid()); > - if (!pw) > - die(_("unable to look up current user in the passwd file: %s"), > - errno ? strerror(errno) : _("no such user")); > return pw; > } > Yes, this looks better. +1 ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2015-12-14 15:07 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-02 20:10 git-clone fails when current user is not in /etc/passwd Taylor Braun-Jones 2015-12-09 15:23 ` Taylor Braun-Jones 2015-12-09 16:08 ` Duy Nguyen 2015-12-09 18:24 ` Duy Nguyen 2015-12-09 22:35 ` Taylor Braun-Jones 2015-12-10 18:33 ` Taylor Braun-Jones 2015-12-10 18:34 ` Jeff King 2015-12-10 19:57 ` Junio C Hamano 2015-12-10 20:40 ` Jeff King 2015-12-10 21:32 ` [PATCH 0/3] " Jeff King 2015-12-10 21:33 ` [PATCH 1/3] ident: make xgetpwuid_self() a static local helper Jeff King 2015-12-10 23:39 ` Junio C Hamano 2015-12-10 21:35 ` [PATCH 2/3] ident: keep a flag for bogus default_email Jeff King 2015-12-10 22:54 ` Jeff King 2015-12-10 21:41 ` [PATCH 3/3] ident: loosen getpwuid error in non-strict mode Jeff King 2015-12-14 15:07 ` Jeff King 2015-12-10 23:44 ` [PATCH 0/3] git-clone fails when current user is not in /etc/passwd Junio C Hamano 2015-12-11 2:20 ` Taylor Braun-Jones 2015-12-10 18:43 ` Junio C Hamano 2015-12-10 18:52 ` Taylor Braun-Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).