{"thread":{"id":"9049","subject":"git-config: replaces ~/.gitconfig symlink with real file","startedAt":"2007-07-15T21:27:59Z","lastAt":"2007-07-27T16:50:23Z","messageCount":21,"participants":["Bradford Smith","Johannes Schindelin","Nikolai Weibull","Junio C Hamano","Catalin Marinas","Matthieu Moy","Fredrik Tolf","Bradford C. Smith","Morten Welinder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"47459","messageId":"f158199e0707151427h52da3e38rae3be6e44e27e918@mail.gmail.com","threadId":"9049","inReplyTo":null,"subject":"git-config: replaces ~/.gitconfig symlink with real file","fromName":"Bradford Smith","fromEmail":"bradford.carl.smith@gmail.com","sentAt":"2007-07-15T21:27:59Z","receivedAt":"2007-07-15T21:27:59Z","isPatch":false,"sender":{"key":"bradford.carl.smith@gmail.com","avatar":"https://gravatar.com/avatar/699930ad8ca562e38156ee60270b2fee830cc0cb5d94c2f70c6db4a705723c54?d=mp&s=160"},"body":"Since the number of dot-files and dot-directories that I have in my\nhome directory these days is somewhat overwhelming, I like to keep\nthose I directly edit all together in an ~/etc directory so I can\neasily back them up and/or copy them in bulk to new accounts.  So,\nseveral of my home dot-files are just symlinks to something in ~/etc,\nincluding ~/.gitconfig.\n\nHowever, when I tried running 'git-config --global color.diff auto'\ntoday, it removed my symlink and replaced it with a real file.  This\nleft me briefly a bit confused when the changes I had made didn't show\nup in ~/etc/gitconfig, but git-config reported them anyway.\n\nIf I were to fix this, I'd be tempted to use realpath(3) to follow the\nsymlink, but I don't think it's very reliably available\ncross-platform.  Certainly, it isn't used anywhere in the current git\ncode.  Can anyone suggest a more portable fix?\n\nThanks,\n\nBradford\n"},{"id":"47478","messageId":"Pine.LNX.4.64.0707160029120.14781@racer.site","threadId":"9049","inReplyTo":"f158199e0707151427h52da3e38rae3be6e44e27e918@mail.gmail.com","subject":"Re: git-config: replaces ~/.gitconfig symlink with real file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-15T23:30:25Z","receivedAt":"2007-07-15T23:30:25Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 15 Jul 2007, Bradford Smith wrote:\n\n> If I were to fix this, I'd be tempted to use realpath(3) to follow the \n> symlink, but I don't think it's very reliably available cross-platform.  \n> Certainly, it isn't used anywhere in the current git code.  Can anyone \n> suggest a more portable fix?\n\nI'd use readlink(2) and test for EINVAL to fall back to the current \nbehaviour.\n\nHth,\nDscho\n"},{"id":"47525","messageId":"dbfc82860707160237v6772b5b8o541f2045ccd824d5@mail.gmail.com","threadId":"9049","inReplyTo":"f158199e0707151427h52da3e38rae3be6e44e27e918@mail.gmail.com","subject":"Re: git-config: replaces ~/.gitconfig symlink with real file","fromName":"Nikolai Weibull","fromEmail":"now@bitwi.se","sentAt":"2007-07-16T09:37:22Z","receivedAt":"2007-07-16T09:37:22Z","isPatch":false,"sender":{"key":"now@bitwi.se","avatar":"https://gravatar.com/avatar/d9242f067845cf9a72be23e4213c3b6e53492178e5df97372088a441af846133?d=mp&s=160"},"body":"On 7/15/07, Bradford Smith <bradford.carl.smith@gmail.com> wrote:\n> Since the number of dot-files and dot-directories that I have in my\n> home directory these days is somewhat overwhelming, I like to keep\n> those I directly edit all together in an ~/etc directory so I can\n> easily back them up and/or copy them in bulk to new accounts.  So,\n> several of my home dot-files are just symlinks to something in ~/etc,\n> including ~/.gitconfig.\n\nHow about adding an environment variable telling Git where to find\nuser-global .gitconfig instead?\n\n  nikolai\n"},{"id":"47535","messageId":"f158199e0707160433v27fe7073w9c550712c41c32e8@mail.gmail.com","threadId":"9049","inReplyTo":"dbfc82860707160237v6772b5b8o541f2045ccd824d5@mail.gmail.com","subject":"Re: git-config: replaces ~/.gitconfig symlink with real file","fromName":"Bradford Smith","fromEmail":"bradford.carl.smith@gmail.com","sentAt":"2007-07-16T11:33:56Z","receivedAt":"2007-07-16T11:33:56Z","isPatch":false,"sender":{"key":"bradford.carl.smith@gmail.com","avatar":"https://gravatar.com/avatar/699930ad8ca562e38156ee60270b2fee830cc0cb5d94c2f70c6db4a705723c54?d=mp&s=160"},"body":"On 7/16/07, Nikolai Weibull <now@bitwi.se> wrote:\n> On 7/15/07, Bradford Smith <bradford.carl.smith@gmail.com> wrote:\n> > Since the number of dot-files and dot-directories that I have in my\n> > home directory these days is somewhat overwhelming, I like to keep\n> > those I directly edit all together in an ~/etc directory so I can\n> > easily back them up and/or copy them in bulk to new accounts.  So,\n> > several of my home dot-files are just symlinks to something in ~/etc,\n> > including ~/.gitconfig.\n>\n> How about adding an environment variable telling Git where to find\n> user-global .gitconfig instead?\n> > home directory these days is somewhat overwhelming, I like to keep\n> > those I directly edit all together in an ~/etc directory so I can\n> > easily back them up and/or copy them in bulk to new accounts.  So,\n> > several of my home dot-files are just symlinks to something in ~/etc,\n> > including ~/.gitconfig.\n>\n> How about adding an environment variable telling Git where to find\n> user-global .gitconfig instead?\n\nThanks for suggesting that.\n\nActually, by looking at the code I discovered I could use the\nenvironment variable GIT_CONFIG to specify where the configuration\nfile is, and I have already changed my setup to use this.\nUnfortunately, I found the documentation for this variable in\ngit-config(1) confusing or I would have used it before.  If I get the\nchance, I'll submit a patch for git-config.txt, and maybe for git.txt\nas well, since it lists lots of other environment variables but not\nGIT_CONFIG or GIT_CONFIG_LOCAL.\n\nThanks,\n\nBradford\n"},{"id":"47546","messageId":"f158199e0707160626j1025ab2cp3339ca6ab91d9af0@mail.gmail.com","threadId":"9049","inReplyTo":"f158199e0707160433v27fe7073w9c550712c41c32e8@mail.gmail.com","subject":"Re: git-config: replaces ~/.gitconfig symlink with real file","fromName":"Bradford Smith","fromEmail":"bradford.carl.smith@gmail.com","sentAt":"2007-07-16T13:26:01Z","receivedAt":"2007-07-16T13:26:01Z","isPatch":false,"sender":{"key":"bradford.carl.smith@gmail.com","avatar":"https://gravatar.com/avatar/699930ad8ca562e38156ee60270b2fee830cc0cb5d94c2f70c6db4a705723c54?d=mp&s=160"},"body":"On 7/16/07, Bradford Smith <bradford.carl.smith@gmail.com> wrote:\n> On 7/16/07, Nikolai Weibull <now@bitwi.se> wrote:\n> > On 7/15/07, Bradford Smith <bradford.carl.smith@gmail.com> wrote:\n> > > Since the number of dot-files and dot-directories that I have in my\n> > > home directory these days is somewhat overwhelming, I like to keep\n> > > those I directly edit all together in an ~/etc directory so I can\n> > > easily back them up and/or copy them in bulk to new accounts.  So,\n> > > several of my home dot-files are just symlinks to something in ~/etc,\n> > > including ~/.gitconfig.\n> >\n> > How about adding an environment variable telling Git where to find\n> > user-global .gitconfig instead?\n> > > home directory these days is somewhat overwhelming, I like to keep\n> > > those I directly edit all together in an ~/etc directory so I can\n> > > easily back them up and/or copy them in bulk to new accounts.  So,\n> > > several of my home dot-files are just symlinks to something in ~/etc,\n> > > including ~/.gitconfig.\n> >\n> > How about adding an environment variable telling Git where to find\n> > user-global .gitconfig instead?\n>\n> Thanks for suggesting that.\n>\n> Actually, by looking at the code I discovered I could use the\n> environment variable GIT_CONFIG to specify where the configuration\n> file is, and I have already changed my setup to use this.\n> Unfortunately, I found the documentation for this variable in\n> git-config(1) confusing or I would have used it before.  If I get the\n> chance, I'll submit a patch for git-config.txt, and maybe for git.txt\n> as well, since it lists lots of other environment variables but not\n> GIT_CONFIG or GIT_CONFIG_LOCAL.\n>\n> Thanks,\n>\n> Bradford\n>\n\nDrat!  The documentation wasn't as wrong as I had hoped.  If I set\nGIT_CONFIG, git will ignore $(prefix)/etc/gitconfig and ~/.git/config,\nwhich isn't what I want.  So, I guess I need to add a GIT_CONFIG_HOME\nenvironment variable.  If I get that done, I'll send a patch to the\nlist including doc updates.\n\nOf course, if someone else wants to do it first, I won't complain. B')\n\nThanks,\n\nBradford\n"},{"id":"47603","messageId":"7vps2s2chy.fsf@assigned-by-dhcp.cox.net","threadId":"9049","inReplyTo":"f158199e0707160626j1025ab2cp3339ca6ab91d9af0@mail.gmail.com","subject":"Re: git-config: replaces ~/.gitconfig symlink with real file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-16T22:46:17Z","receivedAt":"2007-07-16T22:46:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Bradford Smith\" <bradford.carl.smith@gmail.com> writes:\n\n> ...  So, I guess I need to add a GIT_CONFIG_HOME\n> environment variable.\n\nI suspect that is going down a wrong path.\n\nWe use the sequence:\n\n\tfd = creat(\"temporary location\");\n        write(fd, ...);\n        close(fd);\n        rename(\"temporary location\", \"final location\");\n\nin quite a lot of codepaths.  I think they can be factored out,\nto take the \"final location\" (and perhaps a suggested temporary\ndirectory) as an parameter, and that code can check that \"final\nlocation\" is a symlink to somewhere else and create the\ntemporary next to the target file.\n"},{"id":"47632","messageId":"tnx4pk39mju.fsf@arm.com","threadId":"9049","inReplyTo":"f158199e0707151427h52da3e38rae3be6e44e27e918@mail.gmail.com","subject":"Re: git-config: replaces ~/.gitconfig symlink with real file","fromName":"Catalin Marinas","fromEmail":"catalin.marinas@arm.com","sentAt":"2007-07-17T13:39:33Z","receivedAt":"2007-07-17T13:39:33Z","isPatch":false,"sender":{"key":"catalin.marinas@arm.com","avatar":null},"body":"\"Bradford Smith\" <bradford.carl.smith@gmail.com> wrote:\n> However, when I tried running 'git-config --global color.diff auto'\n> today, it removed my symlink and replaced it with a real file.  This\n> left me briefly a bit confused when the changes I had made didn't show\n> up in ~/etc/gitconfig, but git-config reported them anyway.\n\nAnother problem I have with 'git config --global' is that it changes\nthe access permission bits of ~/.gitconfig. Since I use the same file\nto store global StGIT configuration like SMTP username and password,\nI'd like to make its access 0600 but it always goes back to 0644 after\n'git config --global'.\n\nMaybe fixing the symlink case would solve my problem as well.\n\nThanks.\n\n-- \nCatalin\n"},{"id":"47633","messageId":"Pine.LNX.4.64.0707170834040.14781@racer.site","threadId":"9049","inReplyTo":"f158199e0707160626j1025ab2cp3339ca6ab91d9af0@mail.gmail.com","subject":"Re: git-config: replaces ~/.gitconfig symlink with real file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-17T13:56:37Z","receivedAt":"2007-07-17T13:56:37Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 16 Jul 2007, Bradford Smith wrote:\n\n> So, I guess I need to add a GIT_CONFIG_HOME environment variable.  If I \n> get that done, I'll send a patch to the list including doc updates.\n\nAlternatively, you could actually not ignore my hint at readlink(2) and \nhave a proper fix, instead of playing games with environment variables.\n\nHth,\nDscho\n"},{"id":"47634","messageId":"vpqbqebt8ak.fsf@bauges.imag.fr","threadId":"9049","inReplyTo":"Pine.LNX.4.64.0707170834040.14781@racer.site","subject":"Re: git-config: replaces ~/.gitconfig symlink with real file","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-07-17T14:27:15Z","receivedAt":"2007-07-17T14:27:15Z","isPatch":false,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi,\n>\n> On Mon, 16 Jul 2007, Bradford Smith wrote:\n>\n>> So, I guess I need to add a GIT_CONFIG_HOME environment variable.  If I \n>> get that done, I'll send a patch to the list including doc updates.\n>\n> Alternatively, you could actually not ignore my hint at readlink(2) and \n> have a proper fix, instead of playing games with environment variables.\n\nI second that.\n\nUsing an environment variable means having a configuration which is\nabout git in my shell's config file, and that's a source of a lot of\ntroubles. Murphy's law implies that one day, the environment variable\nwon't be set properly (because you changed your shell, because you\nlaunch git from something which isn't a shell, because you logged-in\nin a way that didn't read the config file in which the variable was\nset, ...).\n\nI can do with it, like many other software require an environment\nvariable, but I find the symlink trick much more robust.\n\n-- \nMatthieu\n"},{"id":"47638","messageId":"Pine.LNX.4.64.0707171708210.14781@racer.site","threadId":"9049","inReplyTo":"tnx4pk39mju.fsf@arm.com","subject":"Re: git-config: replaces ~/.gitconfig symlink with real file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-17T16:09:42Z","receivedAt":"2007-07-17T16:09:42Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 17 Jul 2007, Catalin Marinas wrote:\n\n> \"Bradford Smith\" <bradford.carl.smith@gmail.com> wrote:\n> > However, when I tried running 'git-config --global color.diff auto'\n> > today, it removed my symlink and replaced it with a real file.  This\n> > left me briefly a bit confused when the changes I had made didn't show\n> > up in ~/etc/gitconfig, but git-config reported them anyway.\n> \n> Another problem I have with 'git config --global' is that it changes\n> the access permission bits of ~/.gitconfig. Since I use the same file\n> to store global StGIT configuration like SMTP username and password,\n> I'd like to make its access 0600 but it always goes back to 0644 after\n> 'git config --global'.\n> \n> Maybe fixing the symlink case would solve my problem as well.\n\nMore likely not.  The way to solve it would be to follow the link if the \ntarget path is one.  As such, the _file_ would be rewritten.\n\nSo your problem is unrelated, and would need a separate fix.\n\nCiao,\nDscho\n"},{"id":"47671","messageId":"m3wswyojj2.fsf@pc7.dolda2000.com","threadId":"9049","inReplyTo":"Pine.LNX.4.64.0707170834040.14781@racer.site","subject":"Re: git-config: replaces ~/.gitconfig symlink with real file","fromName":"Fredrik Tolf","fromEmail":"fredrik@dolda2000.com","sentAt":"2007-07-17T20:35:45Z","receivedAt":"2007-07-17T20:35:45Z","isPatch":false,"sender":{"key":"fredrik@dolda2000.com","avatar":null},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi,\n>\n> On Mon, 16 Jul 2007, Bradford Smith wrote:\n>\n>> So, I guess I need to add a GIT_CONFIG_HOME environment variable.  If I \n>> get that done, I'll send a patch to the list including doc updates.\n>\n> Alternatively, you could actually not ignore my hint at readlink(2) and \n> have a proper fix, instead of playing games with environment variables.\n\nWouldn't it be nicer to avoid a lot of the complexity in checking\nsymlinks, environment variables and what not, and just overwrite the\nfile in place (with open(..., O_TRUNC | O_CREAT))? Does it happen\nterribly often that git-config crashes in the middle and leaves the\nfile broken?\n\nFredrik Tolf\n"},{"id":"47672","messageId":"Pine.LNX.4.64.0707172145590.14781@racer.site","threadId":"9049","inReplyTo":"m3wswyojj2.fsf@pc7.dolda2000.com","subject":"Re: git-config: replaces ~/.gitconfig symlink with real file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-17T20:48:08Z","receivedAt":"2007-07-17T20:48:08Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 17 Jul 2007, Fredrik Tolf wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > On Mon, 16 Jul 2007, Bradford Smith wrote:\n> >\n> >> So, I guess I need to add a GIT_CONFIG_HOME environment variable.  If I \n> >> get that done, I'll send a patch to the list including doc updates.\n> >\n> > Alternatively, you could actually not ignore my hint at readlink(2) and \n> > have a proper fix, instead of playing games with environment variables.\n> \n> Wouldn't it be nicer to avoid a lot of the complexity in checking \n> symlinks, environment variables and what not, and just overwrite the \n> file in place (with open(..., O_TRUNC | O_CREAT))? Does it happen \n> terribly often that git-config crashes in the middle and leaves the file \n> broken?\n\nNo, it does not.  But when it does, I am not only annoyed.  I am PISSED!\n\nThe way we do it is the only safe way to do it, and I gladly spend some \nextra cycles for that.  Too often, a small hard disk glitch (or just an \nempty laptop battery!) took some important data into the void.  Too often, \nI _cursed_ at the machine, even if it was the programmers' fault.\n\nCiao,\nDscho\n"},{"id":"48571","messageId":"11853821932079-git-send-email-bradford.carl.smith@gmail.com","threadId":"9049","inReplyTo":"7vps2s2chy.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH 0/2] git-config should not replace symlink","fromName":"Bradford C. Smith","fromEmail":"bradford.carl.smith@gmail.com","sentAt":"2007-07-25T16:49:51Z","receivedAt":"2007-07-25T16:49:51Z","isPatch":true,"sender":{"key":"bradford.carl.smith@gmail.com","avatar":"https://gravatar.com/avatar/699930ad8ca562e38156ee60270b2fee830cc0cb5d94c2f70c6db4a705723c54?d=mp&s=160"},"body":"These patches fix a problem that caused git-config to replace my\n~/.gitconfig symlink with a real file.\n\n[PATCH 1/2] resolve symlinks when creating lockfiles\n[PATCH 2/2] use lockfile.c routines in git_commit_set_multivar()\n"},{"id":"48570","messageId":"11853821951367-git-send-email-bradford.carl.smith@gmail.com","threadId":"9049","inReplyTo":"11853821932079-git-send-email-bradford.carl.smith@gmail.com","subject":"[PATCH 1/2] resolve symlinks when creating lockfiles","fromName":"Bradford C. Smith","fromEmail":"bradford.carl.smith@gmail.com","sentAt":"2007-07-25T16:49:52Z","receivedAt":"2007-07-25T16:49:52Z","isPatch":true,"sender":{"key":"bradford.carl.smith@gmail.com","avatar":"https://gravatar.com/avatar/699930ad8ca562e38156ee60270b2fee830cc0cb5d94c2f70c6db4a705723c54?d=mp&s=160"},"body":"From: Bradford C. Smith <bradford.carl.smith@gmail.com>\n\nWithout this fix, the lockfile code will replace a symlink with a real file.\n\nSigned-off-by: \"Bradford C. Smith\" <bradford.carl.smith@gmail.com>\n---\n lockfile.c |   87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 files changed, 86 insertions(+), 1 deletions(-)\n\ndiff --git a/lockfile.c b/lockfile.c\nindex fb8f13b..4c35224 100644\n--- a/lockfile.c\n+++ b/lockfile.c\n@@ -25,10 +25,95 @@ static void remove_lock_file_on_signal(int signo)\n \traise(signo);\n }\n \n+/**\n+ * p = absolute or relative path name\n+ *\n+ * Return a pointer into p showing the beginning of the last path name\n+ * element.  If p is empty or the root directory (\"/\"), just return p.\n+ */\n+static char * last_path_elm(char * p)\n+{\n+\tint\tp_len = strlen(p);\n+\tchar *\tr;\n+\n+\tif (p_len < 1) return p;\n+\t/* r points to last non-null character in p */\n+\tr = p + p_len - 1;\n+\t/* first skip any trailing slashes */\n+\twhile (*r == '/' && r > p) r--;\n+\t/* then go back to the first non-slash */\n+\twhile (r > p && *(r-1) != '/') r--;\n+\treturn r;\n+}\n+\n+/**\n+ * p = char array containing path to existing file or symlink\n+ * s = size of p\n+ *\n+ * If p indicates a valid symlink to an existing file, overwrite p with\n+ * the path to the real file.  Otherwise, leave p unmodified.\n+ *\n+ * Always returns p in any case.\n+ *\n+ * NOTE: This is a best-effort routine.  It will give no indication of\n+ * failure if it is unable to fully resolve p.  However, it is\n+ * guaranteed to leave p in one of the following states if there isn't\n+ * enough room in p or some other failure occurs:\n+ *\n+ * 1. unmodified\n+ *      OR\n+ * 2. path to a different symlink in a chain that eventually leads to a\n+ *    real file or directory.\n+ */\n+static char * resolve_symlink(char * p, size_t s)\n+{\n+\tstruct stat st;\n+\tchar link[PATH_MAX];\n+\tint link_len;\n+\n+\t/* To avoid an infinite loop of symlinks, try a normal stat()\n+\t * first.  This will fail if p is a symlink that cannot be\n+\t * resolved, so we won't waste our time following a bad link. */\n+\tif (stat(p, &st)) return p;\n+\t/* if I can stat() the file, I sure ought to be able to lstat()\n+\t * it, but if something bizarre happens, just return p.  */\n+\tif (lstat(p, &st)) return p;\n+\t/* if not a link, return p unmodified */\n+\tif (!S_ISLNK(st.st_mode)) return p;\n+\tlink_len = st.st_size;\n+\t/* link is too big, so just return p */\n+\tif (link_len >= sizeof(link)) return p;\n+\t/* fail if readlink fails, and just return p */\n+\tif (link_len != readlink(p, link, sizeof(link))) return p;\n+\t/* readlink never null-terminates */\n+\tlink[link_len] = '\\0';\n+\tif (link[0] == '/') {\n+\t\t/* absolute path simply replaces p */\n+\t\t/* fail if link won't fit in p */\n+\t\tif (link_len >= s) return p;\n+\t\tstrcpy(p, link);\n+\t} else {\n+\t\t/* link is relative path, so we must replace the last\n+\t\t * element of p with it. */\n+\t\tchar * r = last_path_elm(p);\n+\t\t/* make sure there's room in p for us to replace the\n+\t\t * last element with the link contents */\n+\t\tif (r - p + link_len >= s) return p;\n+\t\tstrcpy(r, link);\n+\t}\n+\t/* try again in case we've resolved to another symlink */\n+\treturn resolve_symlink(p, s);\n+}\n+\n static int lock_file(struct lock_file *lk, const char *path)\n {\n \tint fd;\n-\tsprintf(lk->filename, \"%s.lock\", path);\n+\tif (strlen(path) >= sizeof(lk->filename)) return -1;\n+\tstrcpy(lk->filename, path);\n+\t/* subtract 5 from size to make sure there's room for adding\n+\t * \".lock\" for the lock file name */\n+\tresolve_symlink(lk->filename, sizeof(lk->filename)-5);\n+\tstrcat(lk->filename, \".lock\");\n \tfd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);\n \tif (0 <= fd) {\n \t\tif (!lock_file_list) {\n-- \n1.5.3.rc2.30.g1c06-dirty\n"},{"id":"48572","messageId":"11853821962210-git-send-email-bradford.carl.smith@gmail.com","threadId":"9049","inReplyTo":"11853821951367-git-send-email-bradford.carl.smith@gmail.com","subject":"[PATCH 2/2] use lockfile.c routines in git_commit_set_multivar()","fromName":"Bradford C. Smith","fromEmail":"bradford.carl.smith@gmail.com","sentAt":"2007-07-25T16:49:53Z","receivedAt":"2007-07-25T16:49:53Z","isPatch":true,"sender":{"key":"bradford.carl.smith@gmail.com","avatar":"https://gravatar.com/avatar/699930ad8ca562e38156ee60270b2fee830cc0cb5d94c2f70c6db4a705723c54?d=mp&s=160"},"body":"From: Bradford C. Smith <bradford.carl.smith@gmail.com>\n\nChanged git_commit_set_multivar() to use the routines provided by\nlockfile.c to reduce code duplication and ensure consistent behavior.\n\nSigned-off-by: \"Bradford C. Smith\" <bradford.carl.smith@gmail.com>\n---\n config.c |   28 ++++++++++++++++------------\n 1 files changed, 16 insertions(+), 12 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex f89a611..9101de9 100644\n--- a/config.c\n+++ b/config.c\n@@ -715,7 +715,7 @@ int git_config_set_multivar(const char* key, const char* value,\n \tint fd = -1, in_fd;\n \tint ret;\n \tchar* config_filename;\n-\tchar* lock_file;\n+\tstruct lock_file *lock = NULL;\n \tconst char* last_dot = strrchr(key, '.');\n \n \tconfig_filename = getenv(CONFIG_ENVIRONMENT);\n@@ -725,7 +725,6 @@ int git_config_set_multivar(const char* key, const char* value,\n \t\t\tconfig_filename  = git_path(\"config\");\n \t}\n \tconfig_filename = xstrdup(config_filename);\n-\tlock_file = xstrdup(mkpath(\"%s.lock\", config_filename));\n \n \t/*\n \t * Since \"key\" actually contains the section name and the real\n@@ -770,11 +769,12 @@ int git_config_set_multivar(const char* key, const char* value,\n \tstore.key[i] = 0;\n \n \t/*\n-\t * The lock_file serves a purpose in addition to locking: the new\n+\t * The lock serves a purpose in addition to locking: the new\n \t * contents of .git/config will be written into it.\n \t */\n-\tfd = open(lock_file, O_WRONLY | O_CREAT | O_EXCL, 0666);\n-\tif (fd < 0 || adjust_shared_perm(lock_file)) {\n+\tlock = xcalloc(sizeof(struct lock_file), 1);\n+\tfd = hold_lock_file_for_update(lock, config_filename, 0);\n+\tif (fd < 0) {\n \t\tfprintf(stderr, \"could not lock config file\\n\");\n \t\tfree(store.key);\n \t\tret = -1;\n@@ -914,25 +914,29 @@ int git_config_set_multivar(const char* key, const char* value,\n \t\t\t\tgoto write_err_out;\n \n \t\tmunmap(contents, contents_sz);\n-\t\tunlink(config_filename);\n \t}\n \n-\tif (rename(lock_file, config_filename) < 0) {\n-\t\tfprintf(stderr, \"Could not rename the lock file?\\n\");\n+\tif (close(fd) || commit_lock_file(lock) < 0) {\n+\t\tfprintf(stderr, \"Cannot commit config file!\\n\");\n \t\tret = 4;\n \t\tgoto out_free;\n \t}\n \n+\t/* fd is closed, so don't try to close it below. */\n+\tfd = -1;\n+\t/* lock is committed, so don't try to roll it back below.\n+\t * NOTE: Since lockfile.c keeps a linked list of all created\n+\t * lock files, it isn't safe to free(lock).  It's better to just\n+\t * leave it hanging around. */\n+\tlock = NULL;\n \tret = 0;\n \n out_free:\n \tif (0 <= fd)\n \t\tclose(fd);\n+\tif (lock)\n+\t\trollback_lock_file(lock);\n \tfree(config_filename);\n-\tif (lock_file) {\n-\t\tunlink(lock_file);\n-\t\tfree(lock_file);\n-\t}\n \treturn ret;\n \n write_err_out:\n-- \n1.5.3.rc2.30.g1c06-dirty\n"},{"id":"48610","messageId":"7vbqe0cazy.fsf@assigned-by-dhcp.cox.net","threadId":"9049","inReplyTo":"11853821951367-git-send-email-bradford.carl.smith@gmail.com","subject":"Re: [PATCH 1/2] resolve symlinks when creating lockfiles","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-25T23:35:45Z","receivedAt":"2007-07-25T23:35:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This probably is going in the right direction, but the code is\ntoo densely formatted and unreviewable.  Please imitate the\nlayout convention of the other parts of the code.\n\n> +/**\n> + * p = absolute or relative path name\n> + *\n> + * Return a pointer into p showing the beginning of the last path name\n> + * element.  If p is empty or the root directory (\"/\"), just return p.\n> + */\n\n\t/*\n         * multi-line comments look like this without the extra\n         * asterisk at the beginning of the first line.\n         */\n\n> +static char * last_path_elm(char * p)\n\nchar *last_path_elem(char *p)\n\n> +{\n> +\tint\tp_len = strlen(p);\n> +\tchar *\tr;\n> +\n> +\tif (p_len < 1) return p;\n\n        char *r;\n\tint p_len = strlen(p);\n\n        if (p_len < 1)\n                return p;\n\nAren't p and r of type \"const char *\", I wonder...\n\n> +\t/* r points to last non-null character in p */\n> +\tr = p + p_len - 1;\n> +\t/* first skip any trailing slashes */\n> +\twhile (*r == '/' && r > p) r--;\n\nThat is\n\n\tr = strrchr(p, '/');\n\nisn't it?\n\n> +/**\n> + * p = char array containing path to existing file or symlink\n> + * s = size of p\n> + *\n> + * If p indicates a valid symlink to an existing file, overwrite p with\n> + * the path to the real file.  Otherwise, leave p unmodified.\n\nI suspect some callers use lockfile interface to create a new\nfile.  There will be a symlink to not-yet-created real file,\nthat is.\n"},{"id":"48719","messageId":"11854712542350-git-send-email-bradford.carl.smith@gmail.com","threadId":"9049","inReplyTo":"7vbqe0cazy.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] fully resolve symlinks when creating lockfiles","fromName":"Bradford C. Smith","fromEmail":"bradford.carl.smith@gmail.com","sentAt":"2007-07-26T17:34:14Z","receivedAt":"2007-07-26T17:34:14Z","isPatch":true,"sender":{"key":"bradford.carl.smith@gmail.com","avatar":"https://gravatar.com/avatar/699930ad8ca562e38156ee60270b2fee830cc0cb5d94c2f70c6db4a705723c54?d=mp&s=160"},"body":"Make the code for resolving symlinks in lockfile.c more robust as\nfollows:\n\n1. Handle relative symlinks\n2. recursively resolve symlink chains up to OS limit\n\nSigned-off-by: Bradford C. Smith <bradford.carl.smith@gmail.com>\n---\n\nI have updated this patch as follows based partly on Junio's comments.\n\n\t1. Made comment and coding style consistent with existing git\n\t   code base.\n\t2. improved readability\n\t3. rebased to latest version of master (2007-07-26) and updated\n\t   commit message appropriately\n\t4. added warning messages for error conditions\n\t5. resolve symlinks to non-existent files\n\n lockfile.c |  128 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------\n 1 files changed, 114 insertions(+), 14 deletions(-)\n\ndiff --git a/lockfile.c b/lockfile.c\nindex 9202472..864ce73 100644\n--- a/lockfile.c\n+++ b/lockfile.c\n@@ -25,23 +25,123 @@ static void remove_lock_file_on_signal(int signo)\n \traise(signo);\n }\n \n+/*\n+ * p = absolute or relative path name\n+ *\n+ * Return a pointer into p showing the beginning of the last path name\n+ * element.  If p is empty or the root directory (\"/\"), just return p.\n+ */\n+static const char *last_path_elm(const char *p)\n+{\n+\t/* r starts pointing to null at the end of the string */\n+\tconst char *r = strchr(p, '\\0');\n+\n+\tif (r == p)\n+\t\treturn p; /* just return empty string */\n+\n+\tr--; /* back up to last non-null character */\n+\n+\t/* back up past trailing slashes, if any */\n+\twhile (r > p && *r == '/') {\n+\t\tr--;\n+\t}\n+\t/*\n+\t * then go backwards until I hit a slash, or the beginning of\n+\t * the string\n+\t */\n+\twhile (r > p && *(r-1) != '/') {\n+\t\tr--;\n+\t}\n+\treturn r;\n+}\n+\n+\n+/*\n+ * p = path that may be a symlink\n+ * s = full size of p\n+ *\n+ * If p is a symlink, attempt to overwrite p with a path to the real\n+ * file or directory (which may or may not exist), following a chain of\n+ * symlinks if necessary.  Otherwise, leave p unmodified.\n+ *\n+ * This is a best-effort routine.  If an error occurs, p will either be\n+ * left unmodified or will name a different symlink in a symlink chain\n+ * that started with p's initial contents.\n+ *\n+ * Always returns p.\n+ */\n+static char *resolve_symlink(char * p, size_t s)\n+{\n+\tstruct stat stb;\n+\tchar link[PATH_MAX];\n+\tint link_len;\n+\n+\t/*\n+\t * leave p unchanged if it doesn't appear to be a valid path to\n+\t * a symlink.\n+\t */\n+\tif (lstat(p, &stb) != 0 || !S_ISLNK(stb.st_mode)) {\n+\t\treturn p;\n+\t}\n+\t/*\n+\t * don't attempt to resolve a chain or loop of symlinks the OS\n+\t * cannot resolve.\n+\t */\n+\tif (stat(p, &stb) != 0 && ELOOP == errno) {\n+\t\twarning(\"%s: %s\", p, strerror(ELOOP));\n+\t\treturn p;\n+\t}\n+\n+\tlink_len = readlink(p, link, sizeof(link));\n+\tif (link_len < 0) {\n+\t\twarning(\"%s: %s\", p, strerror(errno));\n+\t\treturn p;\n+\t} else if (link_len < sizeof(link)) {\n+\t\t/* readlink() never null-terminates */\n+\t\tlink[link_len] = '\\0';\n+\t} else {\n+\t\twarning(\"%s: symlink too long\", p);\n+\t\treturn p;\n+\t}\n+\n+\tif (link[0] == '/') {\n+\t\t/* absolute path simply replaces p */\n+\t\tif (link_len < s) {\n+\t\t\tstrcpy(p, link);\n+\t\t} else {\n+\t\t\twarning(\"%s: symlink too long\", p);\n+\t\t\treturn p;\n+\t\t}\n+\t} else {\n+\t\t/*\n+\t\t * link is a relative path, so I must replace the last\n+\t\t * element of p with it.\n+\t\t */\n+\t\tchar *r = (char*)last_path_elm(p);\n+\t\tif (r - p + link_len < s) {\n+\t\t\tstrcpy(r, link);\n+\t\t} else {\n+\t\t\twarning(\"%s: symlink too long\", p);\n+\t\t\treturn p;\n+\t\t}\n+\t}\n+\t/* try again in case we've resolved to another symlink */\n+\treturn resolve_symlink(p, s);\n+}\n+\n+\n static int lock_file(struct lock_file *lk, const char *path)\n {\n \tint fd;\n-\tstruct stat st;\n-\n-\tif ((!lstat(path, &st)) && S_ISLNK(st.st_mode)) {\n-\t\tssize_t sz;\n-\t\tstatic char target[PATH_MAX];\n-\t\tsz = readlink(path, target, sizeof(target));\n-\t\tif (sz < 0)\n-\t\t\twarning(\"Cannot readlink %s\", path);\n-\t\telse if (target[0] != '/')\n-\t\t\twarning(\"Cannot lock target of relative symlink %s\", path);\n-\t\telse\n-\t\t\tpath = target;\n-\t}\n-\tsprintf(lk->filename, \"%s.lock\", path);\n+\n+\tif (strlen(path) >= sizeof(lk->filename)) return -1;\n+\tstrcpy(lk->filename, path);\n+\t/*\n+\t * subtract 5 from size to make sure there's room for adding\n+\t * \".lock\" for the lock file name\n+\t */\n+\tresolve_symlink(lk->filename, sizeof(lk->filename)-5);\n+\tstrcat(lk->filename, \".lock\");\n \tfd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);\n \tif (0 <= fd) {\n \t\tif (!lock_file_list) {\n-- \n1.5.3.rc3.9.g1b487\n"},{"id":"48727","messageId":"Pine.LNX.4.64.0707261934080.14781@racer.site","threadId":"9049","inReplyTo":"11854712542350-git-send-email-bradford.carl.smith@gmail.com","subject":"Re: [PATCH] fully resolve symlinks when creating lockfiles","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-26T18:35:17Z","receivedAt":"2007-07-26T18:35:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 26 Jul 2007, Bradford C. Smith wrote:\n\n> Make the code for resolving symlinks in lockfile.c more robust as\n> follows:\n> \n> 1. Handle relative symlinks\n> 2. recursively resolve symlink chains up to OS limit\n\nFWIW I like what it does, but how.  What is so wrong with just relying on \nis_absolute_path() and make_absolute_path()?  The code would be much \nshorter then, and we need those functions anyway, methinks.\n\nCiao,\nDscho\n"},{"id":"48732","messageId":"118833cc0707261234u59e30bchc274ae29569d8500@mail.gmail.com","threadId":"9049","inReplyTo":"11854712542350-git-send-email-bradford.carl.smith@gmail.com","subject":"Re: [PATCH] fully resolve symlinks when creating lockfiles","fromName":"Morten Welinder","fromEmail":"mwelinder@gmail.com","sentAt":"2007-07-26T19:34:50Z","receivedAt":"2007-07-26T19:34:50Z","isPatch":true,"sender":{"key":"mwelinder@gmail.com","avatar":null},"body":"Why the lstat and that stat in the beginning?  That's just asking for race\ncondition.  readlink will tell you if it wasn't a link, for example.\n\nMorten\n"},{"id":"48775","messageId":"7vk5sm2unt.fsf@assigned-by-dhcp.cox.net","threadId":"9049","inReplyTo":"11854712542350-git-send-email-bradford.carl.smith@gmail.com","subject":"Re: [PATCH] fully resolve symlinks when creating lockfiles","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-27T07:05:42Z","receivedAt":"2007-07-27T07:05:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Bradford C. Smith\" <bradford.carl.smith@gmail.com> writes:\n\n> Make the code for resolving symlinks in lockfile.c more robust as\n> follows:\n>\n> 1. Handle relative symlinks\n> 2. recursively resolve symlink chains up to OS limit\n\nI munged this patch with Morten's comments.  Will queue for\n'next'.  Further polishing will be done in 'next' as needed.\n"},{"id":"48805","messageId":"f158199e0707270950m638f6863t8272ca50430c304c@mail.gmail.com","threadId":"9049","inReplyTo":"118833cc0707261234u59e30bchc274ae29569d8500@mail.gmail.com","subject":"Re: [PATCH] fully resolve symlinks when creating lockfiles","fromName":"Bradford Smith","fromEmail":"bradford.carl.smith@gmail.com","sentAt":"2007-07-27T16:50:23Z","receivedAt":"2007-07-27T16:50:23Z","isPatch":true,"sender":{"key":"bradford.carl.smith@gmail.com","avatar":"https://gravatar.com/avatar/699930ad8ca562e38156ee60270b2fee830cc0cb5d94c2f70c6db4a705723c54?d=mp&s=160"},"body":"On 7/26/07, Morten Welinder <mwelinder@gmail.com> wrote:\n> Why the lstat and that stat in the beginning?  That's just asking for race\n> condition.  readlink will tell you if it wasn't a link, for example.\n\nHere's an example of the sort of thing I'm trying to avoid:\n\nfoo is a symlink to bar\nbar is a symlink back to foo\n\nreadlink() on either one will succeed, but I'll end up with infinite\nrecursion because I'll resolve foo to bar, then bar to foo, then foo\nback to bar, etc.\n\nTo avoid craziness like this the OS refuses to follow a chain of more\nthan a very small number of symlinks.  By experimentation, I found the\nlimit to be 8 on my Linux box.\n\nI am trying to avoid resolving symlinks manually that the OS would\nrefuse to resolve anyway.\n\nHowever, I'm quite open to suggestions for a better way to do it.\n\nThanks,\n\nBradford\n"}]}