Tested, original setup works fine. On Fri, 2013-04-12 at 14:03 -0700, Jonathan Nieder wrote: > The changes v1.7.12.1~2^2~4 (config: warn on inaccessible files, > 2012-08-21) and v1.8.1.1~22^2~2 (config: treat user and xdg config > permission problems as errors, 2012-10-13) were intended to prevent > important configuration (think "[transfer] fsckobjects") from being > ignored when the configuration is unintentionally unreadable (for > example with EIO on a flaky filesystem, or with ENOMEM due to a DoS > attack). Usually ~/.gitconfig and ~/.config/git are readable by the > current user, and if they aren't then it would be easy to fix those > permissions, so the damage from adding this check should have been > minimal. > > Unfortunately the access() check often trips when git is being run as > a server. A daemon (such as inetd or git-daemon) starts as "root", > creates a listening socket, and then drops privileges, meaning that > when git commands are invoked they cannot access $HOME and die with > > fatal: unable to access '/root/.config/git/config': Permission denied > > Any patch to fix this would have one of three problems: > > 1. We annoy sysadmins who need to take an extra step to handle HOME > when dropping privileges (the current behavior, or any other > proposal that they have to opt into). > > 2. We annoy sysadmins who want to set HOME when dropping privileges, > either by making what they want to do impossible, or making them > set an extra variable or option to accomplish what used to work > (e.g., a patch to git-daemon to set HOME when --user is passed). > > 3. We loosen the check, so some cases which might be noteworthy are > not caught. > > This patch is of type (3). > > Treat user and xdg configuration that are inaccessible due to > permissions (EACCES) as though no user configuration was provided at > all. > > An alternative method would be to check if $HOME is readable, but that > would not help in cases where the user who dropped privileges had a > globally readable HOME with only .config or .gitconfig being private. > > This does not change the behavior when /etc/gitconfig or .git/config > is unreadable (since those are more serious configuration errors), > nor when ~/.gitconfig or ~/.config/git is unreadable due to problems > other than permissions. > > Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx> > Improved-by: Jeff King <peff@xxxxxxxx> > --- > Jonathan Nieder wrote: > > > --- a/wrapper.c > > +++ b/wrapper.c > > @@ -408,11 +408,16 @@ void warn_on_inaccessible(const char *path) > > warning(_("unable to access '%s': %s"), path, strerror(errno)); > > } > > > > +static int access_error_is_ok(int err, unsigned flag) > > +{ > > + return errno == ENOENT || errno == ENOTDIR || > > Sigh, I can't spell "err". Here's a v2 incorporating that change and > with commit message incorporating the latest discussion. > > builtin/config.c | 4 ++-- > config.c | 10 +++++----- > dir.c | 4 ++-- > git-compat-util.h | 5 +++-- > wrapper.c | 14 ++++++++++---- > 5 files changed, 22 insertions(+), 15 deletions(-) > > diff --git a/builtin/config.c b/builtin/config.c > index 33c9bf9..19ffcaf 100644 > --- a/builtin/config.c > +++ b/builtin/config.c > @@ -379,8 +379,8 @@ int cmd_config(int argc, const char **argv, const char *prefix) > */ > die("$HOME not set"); > > - if (access_or_warn(user_config, R_OK) && > - xdg_config && !access_or_warn(xdg_config, R_OK)) > + if (access_or_warn(user_config, R_OK, 0) && > + xdg_config && !access_or_warn(xdg_config, R_OK, 0)) > given_config_file = xdg_config; > else > given_config_file = user_config; > diff --git a/config.c b/config.c > index aefd80b..830ee14 100644 > --- a/config.c > +++ b/config.c > @@ -58,7 +58,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc > path = buf.buf; > } > > - if (!access_or_die(path, R_OK)) { > + if (!access_or_die(path, R_OK, 0)) { > if (++inc->depth > MAX_INCLUDE_DEPTH) > die(include_depth_advice, MAX_INCLUDE_DEPTH, path, > cf && cf->name ? cf->name : "the command line"); > @@ -954,23 +954,23 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) > > home_config_paths(&user_config, &xdg_config, "config"); > > - if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK)) { > + if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0)) { > ret += git_config_from_file(fn, git_etc_gitconfig(), > data); > found += 1; > } > > - if (xdg_config && !access_or_die(xdg_config, R_OK)) { > + if (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK)) { > ret += git_config_from_file(fn, xdg_config, data); > found += 1; > } > > - if (user_config && !access_or_die(user_config, R_OK)) { > + if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) { > ret += git_config_from_file(fn, user_config, data); > found += 1; > } > > - if (repo_config && !access_or_die(repo_config, R_OK)) { > + if (repo_config && !access_or_die(repo_config, R_OK, 0)) { > ret += git_config_from_file(fn, repo_config, data); > found += 1; > } > diff --git a/dir.c b/dir.c > index 91cfd99..9cb2f3d 100644 > --- a/dir.c > +++ b/dir.c > @@ -1637,9 +1637,9 @@ void setup_standard_excludes(struct dir_struct *dir) > home_config_paths(NULL, &xdg_path, "ignore"); > excludes_file = xdg_path; > } > - if (!access_or_warn(path, R_OK)) > + if (!access_or_warn(path, R_OK, 0)) > add_excludes_from_file(dir, path); > - if (excludes_file && !access_or_warn(excludes_file, R_OK)) > + if (excludes_file && !access_or_warn(excludes_file, R_OK, 0)) > add_excludes_from_file(dir, excludes_file); > } > > diff --git a/git-compat-util.h b/git-compat-util.h > index cde442f..51a29b8 100644 > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -691,8 +691,9 @@ int remove_or_warn(unsigned int mode, const char *path); > * Call access(2), but warn for any error except "missing file" > * (ENOENT or ENOTDIR). > */ > -int access_or_warn(const char *path, int mode); > -int access_or_die(const char *path, int mode); > +#define ACCESS_EACCES_OK (1U << 0) > +int access_or_warn(const char *path, int mode, unsigned flag); > +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); > diff --git a/wrapper.c b/wrapper.c > index bac59d2..dd7ecbb 100644 > --- a/wrapper.c > +++ b/wrapper.c > @@ -408,18 +408,24 @@ void warn_on_inaccessible(const char *path) > warning(_("unable to access '%s': %s"), path, strerror(errno)); > } > > -int access_or_warn(const char *path, int mode) > +static int access_error_is_ok(int err, unsigned flag) > +{ > + return err == ENOENT || err == ENOTDIR || > + ((flag & ACCESS_EACCES_OK) && err == EACCES); > +} > + > +int access_or_warn(const char *path, int mode, unsigned flag) > { > int ret = access(path, mode); > - if (ret && errno != ENOENT && errno != ENOTDIR) > + if (ret && !access_error_is_ok(errno, flag)) > warn_on_inaccessible(path); > return ret; > } > > -int access_or_die(const char *path, int mode) > +int access_or_die(const char *path, int mode, unsigned flag) > { > int ret = access(path, mode); > - if (ret && errno != ENOENT && errno != ENOTDIR) > + if (ret && !access_error_is_ok(errno, flag)) > die_errno(_("unable to access '%s'"), path); > return ret; > } -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html