{"thread":{"id":"31809","subject":"[RFC/PATCH 0/2] Re: [PATCH] config: warn on inaccessible files","startedAt":"2012-10-14T00:02:10Z","lastAt":"2012-10-14T16:43:19Z","messageCount":13,"participants":["Jonathan Nieder","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"201094","messageId":"20121014000210.GA19094@elie.Belkin","threadId":"31809","inReplyTo":null,"subject":"[RFC/PATCH 0/2] Re: [PATCH] config: warn on inaccessible files","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-14T00:02:10Z","receivedAt":"2012-10-14T00:02:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Jeff,\n\nIn August, Jeff King wrote:\n\n> Before reading a config file, we check \"!access(path, R_OK)\"\n> to make sure that the file exists and is readable. If it's\n> not, then we silently ignore it.\n\ngit became noisy:\n\n $ git fetch --all\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n Fetching origin\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n Fetching charon\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n [...]\n\nOn this machine, ~/.config/git has been a regular file for a while,\nwith ~/.gitconfig a symlink to it.  Probably ENOTDIR should be ignored\njust like ENOENT is.  Except for the noise, the behavior is fine, but\nsomething still feels wrong.  \n\nWhen ~/.gitconfig is unreadable (EPERM), the messages are a symptom of\nan older issue: the config file is being ignored.  Shouldn't git error\nout instead so the permissions can be fixed?  E.g., if the sysadmin\nhas set \"[branch] autoSetupRebase\" to true in /etc/gitconfig and I\nhave set it to false in my own ~/.gitconfig, I'd rather see git error\nout because ~/.gitconfig has become unreadable in a chmod gone wrong\nthan have a branch set up with the wrong settings and have to learn to\nfix it up myself.\n\nIn other words, how about something like this?\n\nJonathan Nieder (2):\n  config, gitignore: failure to access with ENOTDIR is ok\n  config: treat user and xdg config permission problems as errors\n\n config.c          |  4 ++--\n git-compat-util.h |  6 +++++-\n wrapper.c         | 10 +++++++++-\n 3 files changed, 16 insertions(+), 4 deletions(-)\n"},{"id":"201095","messageId":"20121014000307.GA11399@elie.Belkin","threadId":"31809","inReplyTo":"20121014000210.GA19094@elie.Belkin","subject":"[PATCH 1/2] config, gitignore: failure to access with ENOTDIR is ok","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-14T00:03:07Z","receivedAt":"2012-10-14T00:03:07Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The access_or_warn() function is used to check for optional\nconfiguration files like .gitconfig and .gitignore and warn when they\nare not accessible due to a configuration issue (e.g., bad\npermissions).  It is not supposed to complain when a file is simply\nmissing.\n\nNoticed on a system where ~/.config/git was a file --- when the new\nXDG_CONFIG_HOME support looks for ~/.config/git/config it should\nignore ~/.config/git instead of printing irritating warnings:\n\n $ git status -s\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n warning: unable to access '/home/jrn/.config/git/config': Not a directory\n\nCompare v1.7.12.1~2^2 (attr:failure to open a .gitattributes file\nis OK with ENOTDIR, 2012-09-13).\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n git-compat-util.h | 5 ++++-\n wrapper.c         | 2 +-\n 2 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 2fbf1fd8..f567767f 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -635,7 +635,10 @@ int rmdir_or_warn(const char *path);\n  */\n int remove_or_warn(unsigned int mode, const char *path);\n \n-/* Call access(2), but warn for any error besides ENOENT. */\n+/*\n+ * Call access(2), but warn for any error except \"missing file\"\n+ * (ENOENT or ENOTDIR).\n+ */\n int access_or_warn(const char *path, int mode);\n \n /* Warn on an inaccessible file that ought to be accessible */\ndiff --git a/wrapper.c b/wrapper.c\nindex 68739aaa..c1b919f3 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -411,7 +411,7 @@ void warn_on_inaccessible(const char *path)\n int access_or_warn(const char *path, int mode)\n {\n \tint ret = access(path, mode);\n-\tif (ret && errno != ENOENT)\n+\tif (ret && errno != ENOENT && errno != ENOTDIR)\n \t\twarn_on_inaccessible(path);\n \treturn ret;\n }\n-- \n1.8.0.rc2\n"},{"id":"201096","messageId":"20121014000402.GB11399@elie.Belkin","threadId":"31809","inReplyTo":"20121014000210.GA19094@elie.Belkin","subject":"[PATCH 2/2] config: treat user and xdg config permission problems as errors","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-14T00:04:02Z","receivedAt":"2012-10-14T00:04:02Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Git reads multiple configuration files: settings come first from the\nsystem config file (typically /etc/gitconfig), then the xdg config\nfile (typically ~/.config/git/config), then the user's dotfile\n(~/.gitconfig), then the repository configuration (.git/config).\n\nGit has always used access(2) to decide whether to use each file; as\nan unfortunate side effect, that means that if one of these files is\nunreadable (e.g., EPERM or EIO), git skips it.  So if I use\n~/.gitconfig to override some settings but make a mistake and give it\nthe wrong permissions then I am subject to the settings the sysadmin\nchose for /etc/gitconfig.\n\nBetter to error out and ask the user to correct the problem.\n\nThis only affects the user and xdg config files, since the user\npresumably has enough access to fix their permissions.  If the system\nconfig file is unreadable, the best we can do is to warn about it so\nthe user knows to notify someone and get on with work in the meantime.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n config.c          | 4 ++--\n git-compat-util.h | 1 +\n wrapper.c         | 8 ++++++++\n 3 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 08e47e2e..e8875b8a 100644\n--- a/config.c\n+++ b/config.c\n@@ -945,12 +945,12 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)\n \t\tfound += 1;\n \t}\n \n-\tif (xdg_config && !access_or_warn(xdg_config, R_OK)) {\n+\tif (xdg_config && !access_or_die(xdg_config, R_OK)) {\n \t\tret += git_config_from_file(fn, xdg_config, data);\n \t\tfound += 1;\n \t}\n \n-\tif (user_config && !access_or_warn(user_config, R_OK)) {\n+\tif (user_config && !access_or_die(user_config, R_OK)) {\n \t\tret += git_config_from_file(fn, user_config, data);\n \t\tfound += 1;\n \t}\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex f567767f..dfe2e318 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -640,6 +640,7 @@ int remove_or_warn(unsigned int mode, const char *path);\n  * (ENOENT or ENOTDIR).\n  */\n int access_or_warn(const char *path, int mode);\n+int access_or_die(const char *path, int mode);\n \n /* Warn on an inaccessible file that ought to be accessible */\n void warn_on_inaccessible(const char *path);\ndiff --git a/wrapper.c b/wrapper.c\nindex c1b919f3..7cbe96a0 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -416,6 +416,14 @@ int access_or_warn(const char *path, int mode)\n \treturn ret;\n }\n \n+int access_or_die(const char *path, int mode)\n+{\n+\tint ret = access(path, mode);\n+\tif (ret && errno != ENOENT && errno != ENOTDIR)\n+\t\tdie_errno(_(\"unable to access '%s'\"), path);\n+\treturn ret;\n+}\n+\n struct passwd *xgetpwuid_self(void)\n {\n \tstruct passwd *pw;\n-- \n1.8.0.rc2\n"},{"id":"201119","messageId":"7vhapx1wlh.fsf@alter.siamese.dyndns.org","threadId":"31809","inReplyTo":"20121014000210.GA19094@elie.Belkin","subject":"Re: [RFC/PATCH 0/2] Re: [PATCH] config: warn on inaccessible files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-14T04:55:22Z","receivedAt":"2012-10-14T04:55:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Hi Jeff,\n>\n> In August, Jeff King wrote:\n>\n>> Before reading a config file, we check \"!access(path, R_OK)\"\n>> to make sure that the file exists and is readable. If it's\n>> not, then we silently ignore it.\n>\n> git became noisy:\n>\n>  $ git fetch --all\n>  warning: unable to access '/home/jrn/.config/git/config': Not a directory\n>  ...\n>  warning: unable to access '/home/jrn/.config/git/config': Not a directory\n>  Fetching charon\n>  warning: unable to access '/home/jrn/.config/git/config': Not a directory\n>  [...]\n>\n> On this machine, ~/.config/git has been a regular file for a while,\n> with ~/.gitconfig a symlink to it.  Probably ENOTDIR should be ignored\n> just like ENOENT is.  Except for the noise, the behavior is fine, but\n> something still feels wrong.  \n>\n> When ~/.gitconfig is unreadable (EPERM), the messages are a symptom of\n> an older issue: the config file is being ignored.  Shouldn't git error\n> out instead so the permissions can be fixed?  E.g., if the sysadmin\n> has set \"[branch] autoSetupRebase\" to true in /etc/gitconfig and I\n> have set it to false in my own ~/.gitconfig, I'd rather see git error\n> out because ~/.gitconfig has become unreadable in a chmod gone wrong\n> than have a branch set up with the wrong settings and have to learn to\n> fix it up myself.\n>\n> In other words, how about something like this?\n\nI think that is a reasonable issue to address, but I wonder if we\nshould be sharing more code between these.  If the config side can\nbe switched to unconditionally attempt to fopen and then deal with\nan error when it happens, we can get rid of access_or_{warn,die}\nand replace them with fopen_or_{warn,die} and use them from the two\nplaces (attr.c:read_attr_from_file() and the configuration stuff).\n\nI haven't looked to see if that a too intrusive refactoring to be\nworth it, though.\n"},{"id":"201148","messageId":"20121014061622.GA13477@sigill.intra.peff.net","threadId":"31809","inReplyTo":"20121014000210.GA19094@elie.Belkin","subject":"Re: [RFC/PATCH 0/2] Re: [PATCH] config: warn on inaccessible files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-14T06:16:22Z","receivedAt":"2012-10-14T06:16:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 13, 2012 at 05:02:10PM -0700, Jonathan Nieder wrote:\n\n> > Before reading a config file, we check \"!access(path, R_OK)\"\n> > to make sure that the file exists and is readable. If it's\n> > not, then we silently ignore it.\n> \n> git became noisy:\n> \n>  $ git fetch --all\n>  warning: unable to access '/home/jrn/.config/git/config': Not a directory\n> [...]\n\nI somehow thought that we had dealt with this ENOTDIR already, but I see\nthat 8e950da only dealt with .gitattributes, which may look for\narbitrary path names that are not reflected in the current working tree.\n\nWe didn't ignore ENOTDIR for config files at the same time, because it\nis not obvious that such a bogus config path is not something the user\nwould want to know about.\n\n> On this machine, ~/.config/git has been a regular file for a while,\n> with ~/.gitconfig a symlink to it.  Probably ENOTDIR should be ignored\n> just like ENOENT is.  Except for the noise, the behavior is fine, but\n> something still feels wrong.\n\nHmm. Your use of ~/.config/git is interesting. Recent versions of git\nwill look in ~/.config (or $XDG_CONFIG_HOME), but they want to find\n\"git/config\" there, and your single file is in conflict with that. So\nthis has nothing to do with ~/.gitconfig, or the fact that it is\nsymlinked. This is the XDG lookup code kicking in, because you happened\nto put your file in the same place, and then afterwards git learned to\nlook there (albeit with a slightly different format).\n\nSo on the one hand, this ENOTDIR is uninteresting, because it is not\nreally about an error with the file we are trying to open at all, but\nsimply another way of saying \"the file does not exist\". And therefore it\nshould be ignored.\n\nOn the other hand, it is actually alerting you to an unusual situation\nthat you might want to fix (you are putting stuff in the XDG config\ndirectory, but it is not in the format git wants).\n\nI don't have a strong preference about what should happen, but I would\nlean towards your first patch. ENOTDIR really is just another way of\nsaying ENOENT (it just gives more information about the leading paths).\nIt did find a configuration oddity you might want to fix, but that\noddity was not actually hurting anything.\n\n> When ~/.gitconfig is unreadable (EPERM), the messages are a symptom of\n> an older issue: the config file is being ignored.  Shouldn't git error\n> out instead so the permissions can be fixed?  E.g., if the sysadmin\n> has set \"[branch] autoSetupRebase\" to true in /etc/gitconfig and I\n> have set it to false in my own ~/.gitconfig, I'd rather see git error\n> out because ~/.gitconfig has become unreadable in a chmod gone wrong\n> than have a branch set up with the wrong settings and have to learn to\n> fix it up myself.\n\nThis is a separate issue from above. I tend to agree that dying would be\nbetter in most cases, because an operation may not do what you want if\nopening the config fails (for an even worse example, considering\nsomething like receive-pack trying to figure out if receive.denyDeletes\nis set).\n\nI considered doing this when I wrote the original patch, but was mainly\nworried about regressions in weird situations. The two I can think of\nare:\n\n  1. You are inspecting somebody else's repo, but you do not have access\n     to their .git/config file. But then, I think that is probably a\n     sane time to die anyway, since we cannot read core.repositoryFormatVersion.\n\n  2. You have used sudo or some other tool to switch uid, but your\n     environment still points git at your original user's global config,\n     which may not be readable.\n\nThose are unusual situations, though. It probably makes more sense for\nus to be conservative in the common case and die. Case 1 is pretty\ninsane and should probably involve dying anyway. Case 2 people may be\ninconvenienced (they would rather see the harmless warning and continue\nthe operation), but they can work around it by setting up their\nenvironment properly after switching uids.\n\n> In other words, how about something like this?\n> \n> Jonathan Nieder (2):\n>   config, gitignore: failure to access with ENOTDIR is ok\n>   config: treat user and xdg config permission problems as errors\n\nYeah, those look sane, modulo a question about the second one (I'll\nreply directly).\n\n-Peff\n"},{"id":"201149","messageId":"20121014062218.GB13477@sigill.intra.peff.net","threadId":"31809","inReplyTo":"20121014000402.GB11399@elie.Belkin","subject":"Re: [PATCH 2/2] config: treat user and xdg config permission problems as errors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-14T06:22:18Z","receivedAt":"2012-10-14T06:22:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 13, 2012 at 05:04:02PM -0700, Jonathan Nieder wrote:\n\n> Better to error out and ask the user to correct the problem.\n> \n> This only affects the user and xdg config files, since the user\n> presumably has enough access to fix their permissions.  If the system\n> config file is unreadable, the best we can do is to warn about it so\n> the user knows to notify someone and get on with work in the meantime.\n\nI'm on the fence about treating the systme config specially. On the one\nhand, I see the convenience if somebody has a bogus /etc/gitconfig and\ngets EPERM but can't fix it. On the other hand, if we get EIO, isn't\nthat a good indication that we would want to die?\n\nFor example, servers may depend on /etc/gitconfig to enforce security\npolicy (e.g., setting transfer.fsckObjects or receive.deny*). Perhaps\nour default should be safe, and people can use GIT_CONFIG_NOSYSTEM to\nwork around a broken machine.\n\n-Peff\n"},{"id":"201150","messageId":"20121014062650.GC13477@sigill.intra.peff.net","threadId":"31809","inReplyTo":"7vhapx1wlh.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH 0/2] Re: [PATCH] config: warn on inaccessible files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-14T06:26:50Z","receivedAt":"2012-10-14T06:26:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 13, 2012 at 09:55:22PM -0700, Junio C Hamano wrote:\n\n> > When ~/.gitconfig is unreadable (EPERM), the messages are a symptom of\n> > an older issue: the config file is being ignored.  Shouldn't git error\n> > out instead so the permissions can be fixed?  E.g., if the sysadmin\n> > has set \"[branch] autoSetupRebase\" to true in /etc/gitconfig and I\n> > have set it to false in my own ~/.gitconfig, I'd rather see git error\n> > out because ~/.gitconfig has become unreadable in a chmod gone wrong\n> > than have a branch set up with the wrong settings and have to learn to\n> > fix it up myself.\n> >\n> > In other words, how about something like this?\n> \n> I think that is a reasonable issue to address, but I wonder if we\n> should be sharing more code between these.  If the config side can\n> be switched to unconditionally attempt to fopen and then deal with\n> an error when it happens, we can get rid of access_or_{warn,die}\n> and replace them with fopen_or_{warn,die} and use them from the two\n> places (attr.c:read_attr_from_file() and the configuration stuff).\n> \n> I haven't looked to see if that a too intrusive refactoring to be\n> worth it, though.\n\nHandling the error at the fopen level would eliminate a minor race\ncondition (e.g., access() reports OK, then the file has its permissions\nchanged, then we fopen and get EPERM, but ignore it). So it would not\njust be a refactoring, but would actually improve the code quality.\n\n-Peff\n"},{"id":"201154","messageId":"20121014084244.GA7190@elie.Belkin","threadId":"31809","inReplyTo":"20121014062218.GB13477@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] config: treat user and xdg config permission problems as errors","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-14T08:42:44Z","receivedAt":"2012-10-14T08:42:44Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> For example, servers may depend on /etc/gitconfig to enforce security\n> policy (e.g., setting transfer.fsckObjects or receive.deny*). Perhaps\n> our default should be safe, and people can use GIT_CONFIG_NOSYSTEM to\n> work around a broken machine.\n\nVery good point.  How about these patches on top?\n\nJonathan Nieder (2):\n  config doc: advertise GIT_CONFIG_NOSYSTEM\n  config: exit on error accessing any config file\n\n Documentation/git-config.txt | 8 ++++++++\n config.c                     | 6 +++---\n 2 files changed, 11 insertions(+), 3 deletions(-)\n"},{"id":"201155","messageId":"20121014084437.GB7190@elie.Belkin","threadId":"31809","inReplyTo":"20121014084244.GA7190@elie.Belkin","subject":"[PATCH 3/2] config doc: advertise GIT_CONFIG_NOSYSTEM","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-14T08:44:37Z","receivedAt":"2012-10-14T08:44:37Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"When a syntax error or other problem renders /etc/gitconfig buggy on a\nmultiuser system where mortals do not have write access to /etc, the\nGIT_CONFIG_NOSYSTEM variable is the best tool we have to keep getting\nwork done until the sysadmin sorts the problem out.\n\nNoticed while experimenting with teaching git to error out when\n/etc/gitconfig is unreadable.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n Documentation/git-config.txt | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex eaea0791..907a1fd5 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -240,6 +240,14 @@ GIT_CONFIG::\n \tUsing the \"--global\" option forces this to ~/.gitconfig. Using the\n \t\"--system\" option forces this to $(prefix)/etc/gitconfig.\n \n+GIT_CONFIG_NOSYSTEM::\n+\tWhether to skip reading settings from the system-wide\n+\t$(prefix)/etc/gitconfig file.  This environment variable can\n+\tbe used along with HOME and XDG_CONFIG_HOME to create a\n+\tpredictable environment for a picky script, or you can set it\n+\ttemporarily to avoid using a buggy /etc/gitconfig file while\n+\twaiting for someone with sufficient permissions to fix it.\n+\n See also <<FILES>>.\n \n \n-- \n1.8.0.rc2\n"},{"id":"201156","messageId":"20121014084600.GC7190@elie.Belkin","threadId":"31809","inReplyTo":"20121014084244.GA7190@elie.Belkin","subject":"[PATCH 4/2] config: exit on error accessing any config file","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-14T08:46:00Z","receivedAt":"2012-10-14T08:46:00Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"There is convenience in warning and moving on when somebody has a\nbogus permissions on /etc/gitconfig and cannot do anything about it.\nBut the cost in predictability and security is too high --- when\nunreadable config files are skipped, it means an I/O error or\npermissions problem causes important configuration to be bypassed.\n\nFor example, servers may depend on /etc/gitconfig to enforce security\npolicy (setting transfer.fsckObjects or receive.deny*).  Best to\nalways error out when encountering trouble accessing a config file.\n\nThis may add inconvenience in some cases:\n\n  1. You are inspecting somebody else's repo, and you do not have\n     access to their .git/config file.  Git typically dies in this\n     case already since we cannot read core.repositoryFormatVersion,\n     so the change should not be too noticeable.\n\n  2. You have used \"sudo -u\" or a similar tool to switch uid, and your\n     environment still points Git at your original user's global\n     config, which is not readable.  In this case people really would\n     be inconvenienced (they would rather see the harmless warning and\n     continue the operation) but they can work around it by setting\n     HOME appropriately after switching uids.\n\n  3. You do not have access to /etc/gitconfig due to a broken setup.\n     In this case, erroring out is a good way to put pressure on the\n     sysadmin to fix the setup.  While they wait for a reply, users\n     can set GIT_CONFIG_NOSYSTEM to true to keep Git working without\n     complaint.\n\nAfter this patch, errors accessing the repository-local and systemwide\nconfig files and files requested in include directives cause Git to\nexit, just like errors accessing ~/.gitconfig.\n\nExplained-by: Jeff King <peff@peff.net>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n config.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex e8875b8a..a4d153f6 100644\n--- a/config.c\n+++ b/config.c\n@@ -60,7 +60,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n \t\tpath = buf.buf;\n \t}\n \n-\tif (!access_or_warn(path, R_OK)) {\n+\tif (!access_or_die(path, R_OK)) {\n \t\tif (++inc->depth > MAX_INCLUDE_DEPTH)\n \t\t\tdie(include_depth_advice, MAX_INCLUDE_DEPTH, path,\n \t\t\t    cf && cf->name ? cf->name : \"the command line\");\n@@ -939,7 +939,7 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)\n \n \thome_config_paths(&user_config, &xdg_config, \"config\");\n \n-\tif (git_config_system() && !access_or_warn(git_etc_gitconfig(), R_OK)) {\n+\tif (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK)) {\n \t\tret += git_config_from_file(fn, git_etc_gitconfig(),\n \t\t\t\t\t    data);\n \t\tfound += 1;\n@@ -955,7 +955,7 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)\n \t\tfound += 1;\n \t}\n \n-\tif (repo_config && !access_or_warn(repo_config, R_OK)) {\n+\tif (repo_config && !access_or_die(repo_config, R_OK)) {\n \t\tret += git_config_from_file(fn, repo_config, data);\n \t\tfound += 1;\n \t}\n-- \n1.8.0.rc2\n"},{"id":"201157","messageId":"20121014085359.GD7190@elie.Belkin","threadId":"31809","inReplyTo":"20121014084437.GB7190@elie.Belkin","subject":"[PATCH v2 3/2] doc: advertise GIT_CONFIG_NOSYSTEM","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-14T08:53:59Z","receivedAt":"2012-10-14T08:53:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"On a multiuser system where mortals do not have write access to /etc,\nthe GIT_CONFIG_NOSYSTEM variable is the best tool we have to keep\ngetting work done when a syntax error or other problem renders\n/etc/gitconfig buggy, until the sysadmin sorts the problem out.\n\nNoticed while experimenting with teaching git to error out when\n/etc/gitconfig is unreadable.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nJonathan Nieder wrote:\n\n> --- a/Documentation/git-config.txt\n> +++ b/Documentation/git-config.txt\n> @@ -240,6 +240,14 @@ GIT_CONFIG::\n>  \tUsing the \"--global\" option forces this to ~/.gitconfig. Using the\n>  \t\"--system\" option forces this to $(prefix)/etc/gitconfig.\n>  \n> +GIT_CONFIG_NOSYSTEM::\n\nHm, unlike GIT_CONFIG this applies to all git commands (not just \"git\nconfig\"), so it is misleading to document them in the same place.\nHere's a better patch.\n\n Documentation/git-config.txt | 4 ++++\n Documentation/git.txt        | 8 ++++++++\n 2 files changed, 12 insertions(+)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex eaea0791..9ae2508f 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -240,6 +240,10 @@ GIT_CONFIG::\n \tUsing the \"--global\" option forces this to ~/.gitconfig. Using the\n \t\"--system\" option forces this to $(prefix)/etc/gitconfig.\n \n+GIT_CONFIG_NOSYSTEM::\n+\tWhether to skip reading settings from the system-wide\n+\t$(prefix)/etc/gitconfig file. See linkgit:git[1] for details.\n+\n See also <<FILES>>.\n \n \ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex d1d227a3..ae1f833a 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -757,6 +757,14 @@ for further details.\n \tand read the password from its STDOUT. See also the 'core.askpass'\n \toption in linkgit:git-config[1].\n \n+'GIT_CONFIG_NOSYSTEM'::\n+\tWhether to skip reading settings from the system-wide\n+\t`$(prefix)/etc/gitconfig` file.  This environment variable can\n+\tbe used along with `$HOME` and `$XDG_CONFIG_HOME` to create a\n+\tpredictable environment for a picky script, or you can set it\n+\ttemporarily to avoid using a buggy `/etc/gitconfig` file while\n+\twaiting for someone with sufficient permissions to fix it.\n+\n 'GIT_FLUSH'::\n \tIf this environment variable is set to \"1\", then commands such\n \tas 'git blame' (in incremental mode), 'git rev-list', 'git log',\n-- \n1.8.0.rc2\n"},{"id":"201158","messageId":"20121014090041.GE7190@elie.Belkin","threadId":"31809","inReplyTo":"7vhapx1wlh.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH 0/2] Re: [PATCH] config: warn on inaccessible files","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-14T09:00:42Z","receivedAt":"2012-10-14T09:00:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n>                                             If the config side can\n> be switched to unconditionally attempt to fopen and then deal with\n> an error when it happens, we can get rid of access_or_{warn,die}\n> and replace them with fopen_or_{warn,die} and use them from the two\n> places (attr.c:read_attr_from_file() and the configuration stuff).\n>\n> I haven't looked to see if that a too intrusive refactoring to be\n> worth it, though.\n\nThat sounds reasonable, but I'm punting on it.  The first step would\nbe tweaking the git_config_from_file() calling convention to convey\n\"missing file\" errors specially, perhaps by making sure errno is\nmeaningful when the return value is -1, and that already sounds like\nwork. ;-)\n\nThanks,\nJonathan\n"},{"id":"201185","messageId":"20121014164319.GA16225@sigill.intra.peff.net","threadId":"31809","inReplyTo":"20121014084244.GA7190@elie.Belkin","subject":"Re: [PATCH 2/2] config: treat user and xdg config permission problems as errors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-14T16:43:19Z","receivedAt":"2012-10-14T16:43:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 14, 2012 at 01:42:44AM -0700, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> > For example, servers may depend on /etc/gitconfig to enforce security\n> > policy (e.g., setting transfer.fsckObjects or receive.deny*). Perhaps\n> > our default should be safe, and people can use GIT_CONFIG_NOSYSTEM to\n> > work around a broken machine.\n> \n> Very good point.  How about these patches on top?\n> \n> Jonathan Nieder (2):\n>   config doc: advertise GIT_CONFIG_NOSYSTEM\n>   config: exit on error accessing any config file\n> \n>  Documentation/git-config.txt | 8 ++++++++\n>  config.c                     | 6 +++---\n>  2 files changed, 11 insertions(+), 3 deletions(-)\n\nThis is my absolute favorite type of reply: the kind that you can apply\nwith \"git am\".\n\nThe direction and the patches themselves look good to me. I agree with\nyour reasoning in v2 of 3/2; it makes much more sense than v1.\n\nThanks.\n\n-Peff\n"}]}