From e5c52c9898de5f829317729afc0416825531b0d5 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 13 Oct 2012 17:03:07 -0700 Subject: [PATCH 1/4] config, gitignore: failure to access with ENOTDIR is ok The access_or_warn() function is used to check for optional configuration files like .gitconfig and .gitignore and warn when they are not accessible due to a configuration issue (e.g., bad permissions). It is not supposed to complain when a file is simply missing. Noticed on a system where ~/.config/git was a file --- when the new XDG_CONFIG_HOME support looks for ~/.config/git/config it should ignore ~/.config/git instead of printing irritating warnings: $ git status -s warning: unable to access '/home/jrn/.config/git/config': Not a directory warning: unable to access '/home/jrn/.config/git/config': Not a directory warning: unable to access '/home/jrn/.config/git/config': Not a directory warning: unable to access '/home/jrn/.config/git/config': Not a directory Compare v1.7.12.1~2^2 (attr:failure to open a .gitattributes file is OK with ENOTDIR, 2012-09-13). Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- git-compat-util.h | 5 ++++- wrapper.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index 000042d793..dba87da496 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -604,7 +604,10 @@ int rmdir_or_warn(const char *path); */ int remove_or_warn(unsigned int mode, const char *path); -/* Call access(2), but warn for any error besides ENOENT. */ +/* + * Call access(2), but warn for any error except "missing file" + * (ENOENT or ENOTDIR). + */ int access_or_warn(const char *path, int mode); /* Warn on an inaccessible file that ought to be accessible */ diff --git a/wrapper.c b/wrapper.c index 68739aaa3b..c1b919f335 100644 --- a/wrapper.c +++ b/wrapper.c @@ -411,7 +411,7 @@ void warn_on_inaccessible(const char *path) int access_or_warn(const char *path, int mode) { int ret = access(path, mode); - if (ret && errno != ENOENT) + if (ret && errno != ENOENT && errno != ENOTDIR) warn_on_inaccessible(path); return ret; } From 96b9e0e313604f77456906ce58db8f366e47f2ab Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 13 Oct 2012 17:04:02 -0700 Subject: [PATCH 2/4] config: treat user and xdg config permission problems as errors Git reads multiple configuration files: settings come first from the system config file (typically /etc/gitconfig), then the xdg config file (typically ~/.config/git/config), then the user's dotfile (~/.gitconfig), then the repository configuration (.git/config). Git has always used access(2) to decide whether to use each file; as an unfortunate side effect, that means that if one of these files is unreadable (e.g., EPERM or EIO), git skips it. So if I use ~/.gitconfig to override some settings but make a mistake and give it the wrong permissions then I am subject to the settings the sysadmin chose for /etc/gitconfig. Better to error out and ask the user to correct the problem. This only affects the user and xdg config files, since the user presumably has enough access to fix their permissions. If the system config file is unreadable, the best we can do is to warn about it so the user knows to notify someone and get on with work in the meantime. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- config.c | 4 ++-- git-compat-util.h | 1 + wrapper.c | 8 ++++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/config.c b/config.c index 08e47e2e48..e8875b8a57 100644 --- a/config.c +++ b/config.c @@ -945,12 +945,12 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) found += 1; } - if (xdg_config && !access_or_warn(xdg_config, R_OK)) { + if (xdg_config && !access_or_die(xdg_config, R_OK)) { ret += git_config_from_file(fn, xdg_config, data); found += 1; } - if (user_config && !access_or_warn(user_config, R_OK)) { + if (user_config && !access_or_die(user_config, R_OK)) { ret += git_config_from_file(fn, user_config, data); found += 1; } diff --git a/git-compat-util.h b/git-compat-util.h index dba87da496..cfbfaff431 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -609,6 +609,7 @@ int remove_or_warn(unsigned int mode, const char *path); * (ENOENT or ENOTDIR). */ int access_or_warn(const char *path, int mode); +int access_or_die(const char *path, int mode); /* 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 c1b919f335..7cbe96a0cc 100644 --- a/wrapper.c +++ b/wrapper.c @@ -416,6 +416,14 @@ int access_or_warn(const char *path, int mode) return ret; } +int access_or_die(const char *path, int mode) +{ + int ret = access(path, mode); + if (ret && errno != ENOENT && errno != ENOTDIR) + die_errno(_("unable to access '%s'"), path); + return ret; +} + struct passwd *xgetpwuid_self(void) { struct passwd *pw; From e8ef401cd0ff5d51b148c1b1297dab6ef32227c1 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sun, 14 Oct 2012 01:53:59 -0700 Subject: [PATCH 3/4] doc: advertise GIT_CONFIG_NOSYSTEM On a multiuser system where mortals do not have write access to /etc, the GIT_CONFIG_NOSYSTEM variable is the best tool we have to keep getting work done when a syntax error or other problem renders /etc/gitconfig buggy, until the sysadmin sorts the problem out. Noticed while experimenting with teaching git to error out when /etc/gitconfig is unreadable. Signed-off-by: Jonathan Nieder Acked-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/git-config.txt | 4 ++++ Documentation/git.txt | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 2d6ef32a08..4ba17c187b 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -240,6 +240,10 @@ GIT_CONFIG:: Using the "--global" option forces this to ~/.gitconfig. Using the "--system" option forces this to $(prefix)/etc/gitconfig. +GIT_CONFIG_NOSYSTEM:: + Whether to skip reading settings from the system-wide + $(prefix)/etc/gitconfig file. See linkgit:git[1] for details. + See also <>. diff --git a/Documentation/git.txt b/Documentation/git.txt index 27da0eb209..96712889f8 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -771,6 +771,14 @@ for further details. and read the password from its STDOUT. See also the 'core.askpass' option in linkgit:git-config[1]. +'GIT_CONFIG_NOSYSTEM':: + Whether to skip reading settings from the system-wide + `$(prefix)/etc/gitconfig` file. This environment variable can + be used along with `$HOME` and `$XDG_CONFIG_HOME` to create a + predictable environment for a picky script, or you can set it + temporarily to avoid using a buggy `/etc/gitconfig` file while + waiting for someone with sufficient permissions to fix it. + 'GIT_FLUSH':: If this environment variable is set to "1", then commands such as 'git blame' (in incremental mode), 'git rev-list', 'git log', From 8f2bbe452e2c2917ec3c9a5d1593f26908cab83b Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sun, 14 Oct 2012 01:46:00 -0700 Subject: [PATCH 4/4] config: exit on error accessing any config file There is convenience in warning and moving on when somebody has a bogus permissions on /etc/gitconfig and cannot do anything about it. But the cost in predictability and security is too high --- when unreadable config files are skipped, it means an I/O error or permissions problem causes important configuration to be bypassed. For example, servers may depend on /etc/gitconfig to enforce security policy (setting transfer.fsckObjects or receive.deny*). Best to always error out when encountering trouble accessing a config file. This may add inconvenience in some cases: 1. You are inspecting somebody else's repo, and you do not have access to their .git/config file. Git typically dies in this case already since we cannot read core.repositoryFormatVersion, so the change should not be too noticeable. 2. You have used "sudo -u" or a similar tool to switch uid, and your environment still points Git at your original user's global config, which is not readable. In this case people really would be inconvenienced (they would rather see the harmless warning and continue the operation) but they can work around it by setting HOME appropriately after switching uids. 3. You do not have access to /etc/gitconfig due to a broken setup. In this case, erroring out is a good way to put pressure on the sysadmin to fix the setup. While they wait for a reply, users can set GIT_CONFIG_NOSYSTEM to true to keep Git working without complaint. After this patch, errors accessing the repository-local and systemwide config files and files requested in include directives cause Git to exit, just like errors accessing ~/.gitconfig. Explained-by: Jeff King Signed-off-by: Jonathan Nieder Acked-by: Jeff King Signed-off-by: Junio C Hamano --- config.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config.c b/config.c index e8875b8a57..a4d153f631 100644 --- a/config.c +++ b/config.c @@ -60,7 +60,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc path = buf.buf; } - if (!access_or_warn(path, R_OK)) { + if (!access_or_die(path, R_OK)) { if (++inc->depth > MAX_INCLUDE_DEPTH) die(include_depth_advice, MAX_INCLUDE_DEPTH, path, cf && cf->name ? cf->name : "the command line"); @@ -939,7 +939,7 @@ 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_warn(git_etc_gitconfig(), R_OK)) { + if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK)) { ret += git_config_from_file(fn, git_etc_gitconfig(), data); found += 1; @@ -955,7 +955,7 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) found += 1; } - if (repo_config && !access_or_warn(repo_config, R_OK)) { + if (repo_config && !access_or_die(repo_config, R_OK)) { ret += git_config_from_file(fn, repo_config, data); found += 1; }