# [BUG] git_path() returns relative paths 18 messages from 2013-07-27 to 2013-07-28. Participants: Ramkumar Ramachandra, Duy Nguyen, Fredrik Gustafsson, Jiang Xin. Thread: https://gitlist.dev/t/34550 ## Ramkumar Ramachandra, 2013-07-27 11:10 Subject: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CALkWK0m-4wtpTAcePPWFLT7iwXdcpp%2BnOy9oG2iFHLsp9XdKZQ%40mail.gmail.com ``` Hi, I noticed a regression in the latest master, and I've been trying to debug it for 30 minutes now. I'm still clueless about the root cause, but I'll list whatever I found so far: I suddenly noticed that I wasn't able to commit to a certain repository with submodules anymore. This was because git commit was opening a COMMIT_EDITMSG in the wrong path. To reproduce the problem, you need a setup like mine: ~/dotfiles is a repository containing submodules ~/.elisp is a symbolic link to ~/dotfiles/.elisp, a normal directory ~/.elisp/flx is the submodule repository to which I'm trying to commit The buffer-file-name of COMMIT_EDITMSG comes out as /home/artagnon/.git/modules/flx/.git/COMMIT_EDITMSG, which is completely wrong because ~ does not even contain a .git. So, I started debugging the issue with this patch: diff --git a/builtin/commit.c b/builtin/commit.c index 003bd7d..38a7c77 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -678,6 +678,9 @@ static int prepare_to_commit(const char hook_arg2 = ""; } + char *buf = get_current_dir_name(); + die("DBG: %s | %s", buf, git_path(commit_editmsg)); + s->fp = fopen(git_path(commit_editmsg), "w"); if (s->fp == NULL) die_errno(_("could not open '%s'"), git_path(commit_editmsg)); On master, commit returns: fatal: DBG: /home/artagnon/.elisp/flx | ../../.git/modules/.elisp/flx/COMMIT_EDITMSG When backported to v1.8.3.3, commit returns: fatal: DBG: /home/artagnon/.elisp/flx | /home/artagnon/dotfiles/.git/modules/.elisp/flx/COMMIT_EDITMSG I tried looking through the logs to see what has changed in path.c/environment.c, but have come up with nothing so far. I think I'll have to resort to using a hammer like bisect now. *scratches head* ``` ## Duy Nguyen, 2013-07-27 11:48 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CACsJy8B24FL8OriirWnxDb7rGJorn357rnqf709rOUDTdfUGCw%40mail.gmail.com In-Reply-To: ``` On Sat, Jul 27, 2013 at 6:10 PM, Ramkumar Ramachandra wrote: > I tried looking through the logs to see what has changed in > path.c/environment.c, but have come up with nothing so far. I think > I'll have to resort to using a hammer like bisect now. > > *scratches head* Try bisect with GIT_TRACE_SETUP=2 and watch "setup: git_dir: " line. Although $GIT_DIR can be relative and so can git_path(), there may be something else that leads to wrong result path. -- Duy ``` ## Ramkumar Ramachandra, 2013-07-27 12:05 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CALkWK0k-23rTohYvNWP_XbuUCSGUE%2BG_e_7cBbPP5C9OzsJ4XQ%40mail.gmail.com In-Reply-To: ``` Ramkumar Ramachandra wrote: > *scratches head* Just took a much-needed shower and came back. It was trivial to find. $ git log v1.8.3.4.. -- path.c e02ca72 (path.c: refactor relative_path(), not only strip prefix, 2013-06-25) is the offender. ``` ## Fredrik Gustafsson, 2013-07-27 12:14 Subject: Re: [BUG] git_path() returns relative paths Message-ID: <20130727121406.GB4940@paksenarrion.iveqy.com> URL: https://gitlist.dev/e/20130727121406.GB4940%40paksenarrion.iveqy.com In-Reply-To: ``` On Sat, Jul 27, 2013 at 04:40:12PM +0530, Ramkumar Ramachandra wrote: > Hi, > > I noticed a regression in the latest master, and I've been trying to > debug it for 30 minutes now. I'm still clueless about the root cause, > but I'll list whatever I found so far: > > I suddenly noticed that I wasn't able to commit to a certain > repository with submodules anymore. This was because git commit was > opening a COMMIT_EDITMSG in the wrong path. To reproduce the problem, > you need a setup like mine: When I hear submodules, latest master and path the only thing that comes to my mind that have touched these areas is 091a6eb0feed820a43663ca63dc2bc0bb247bbae "submodule: drop the top-level requirement". A first test could be to see if it's this patch does anything strange. However this patch is in git since 1.8.3.3 so it's almost a month old. -- Med vänliga hälsningar Fredrik Gustafsson tel: 0733-608274 e-post: iveqy@iveqy.com ``` ## Ramkumar Ramachandra, 2013-07-27 12:31 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CALkWK0kBesC2%3DkyygMCt61Fm5GGqX%2ByF6sbkCRcGYZ%3DovgtTAQ%40mail.gmail.com In-Reply-To: ``` Ramkumar Ramachandra wrote: > e02ca72 (path.c: refactor relative_path(), not only strip prefix, > 2013-06-25) is the offender. The problem is the callsite in setup.c:setup_work_tree(). When relative_path() is called with "/home/artagnon/dotfiles/.git/modules/.elisp/flx" and "/home/artagnon/dotfiles/.elisp/flx" as the first and second arguments, it sets sb to "../../.git/modules/.elisp/flx". Makes me wonder why setup_git_dir() doesn't just use git_dir; why does it need a relative path at all? ``` ## Duy Nguyen, 2013-07-27 12:47 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CACsJy8C5Du8zcqVeN7EGuyUxgSZaRq-X8y_hQvqQedYWfP8p%2BA%40mail.gmail.com In-Reply-To: ``` On Sat, Jul 27, 2013 at 7:31 PM, Ramkumar Ramachandra wrote: > Ramkumar Ramachandra wrote: >> e02ca72 (path.c: refactor relative_path(), not only strip prefix, >> 2013-06-25) is the offender. > > The problem is the callsite in setup.c:setup_work_tree(). When > relative_path() is called with > "/home/artagnon/dotfiles/.git/modules/.elisp/flx" and > "/home/artagnon/dotfiles/.elisp/flx" as the first and second > arguments, it sets sb to "../../.git/modules/.elisp/flx". > > Makes me wonder why setup_git_dir() doesn't just use git_dir; why does > it need a relative path at all? 044bbbc (Make git_dir a path relative to work_tree in setup_work_tree() - 2008-06-19) -- Duy ``` ## Ramkumar Ramachandra, 2013-07-27 13:26 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CALkWK0kx7NwzsC1-2JYwxjykMkCwz8RZ%3Dy3LqjkQWB6tuOWNtA%40mail.gmail.com In-Reply-To: <20130727121406.GB4940@paksenarrion.iveqy.com> ``` Fredrik Gustafsson wrote: > When I hear submodules The only reason this bug has something to do with submodules is because of the relative path in the gitfile (although I'm yet to corner the exact issue). Otherwise, it doesn't exercise any new code in submodule.c/ git-submodule.sh. ``` ## Ramkumar Ramachandra, 2013-07-27 14:33 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CALkWK0kWnge7fVtTXRgTiK1NbyN8X%3DbmqfPTc%2BMp%3DBCssPc1og%40mail.gmail.com In-Reply-To: ``` Duy Nguyen wrote: > 044bbbc (Make git_dir a path relative to work_tree in > setup_work_tree() - 2008-06-19) Okay, so it does seem to be a significant optimization. Frankly, e02ca72 only improves the relative_path() algorithm, and it's not really doing anything Wrong: it's has just uncovered a previously undiscovered bug. So far, we know that the line: s->fp = fopen(git_path(commit_editmsg), "w"); in commit.c:prepare_to_commit() isn't failing; so, the work_tree and git_dir do seem to be set correctly. The problem seems to be in launch_editor(). What's worse? Everything works just fine when I have a symbolic link to a directory in a normal repository; I still can't figure out what submodules have to do with any of this. What the hell is going on?! ``` ## Duy Nguyen, 2013-07-27 15:17 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CACsJy8CDg2ORKrpwiph%3DWdW5GPUYEwT1CSCttQ4fagQoMP4QGA%40mail.gmail.com In-Reply-To: ``` On Sat, Jul 27, 2013 at 9:33 PM, Ramkumar Ramachandra wrote: > Duy Nguyen wrote: >> 044bbbc (Make git_dir a path relative to work_tree in >> setup_work_tree() - 2008-06-19) > > Okay, so it does seem to be a significant optimization. Frankly, > e02ca72 only improves the relative_path() algorithm, and it's not > really doing anything Wrong: it's has just uncovered a previously > undiscovered bug. > > So far, we know that the line: > > s->fp = fopen(git_path(commit_editmsg), "w"); > > in commit.c:prepare_to_commit() isn't failing; so, the work_tree and > git_dir do seem to be set correctly. The problem seems to be in > launch_editor(). What's worse? Everything works just fine when I have > a symbolic link to a directory in a normal repository; I still can't > figure out what submodules have to do with any of this. > > What the hell is going on?! I was involved with this code (the gitdir setup code, not submodule) and am interested to know what's going on too. Could you produce a small script to reproduce it? -- Duy ``` ## Ramkumar Ramachandra, 2013-07-27 16:05 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CALkWK0%3DxuNGM54%3DKodfi9L8_yUOaXrWvqiMHbx2SRTH1CmZxLA%40mail.gmail.com In-Reply-To: ``` Duy Nguyen wrote: > I was involved with this code (the gitdir setup code, not submodule) > and am interested to know what's going on too. Could you produce a > small script to reproduce it? Here's your reduced testcase. Just point mygit to a HEAD build. #!/bin/sh mygit=~/src/git/git cd /tmp $mygit clone https://github.com/artagnon/clayoven cd clayoven $mygit submodule add https://github.com/lewang/flx .elisp/flx $mygit commit -a -m "Added submodule" cd /tmp ln -s clayoven/.elisp cd .elisp/flx EDITOR="emacs -Q" git commit --amend # buffer-file-name = "/tmp/.git/modules/.elisp/flx/COMMIT_EDITMSG" Note that this is emacs 24.3. I used -Q to make sure that none of my init magic (magit etc.) was responsible for changing directories or doing something equally stupid. However, considering that it's impossible to reproduce the problem with either cat or vim as the EDITOR, you might be inclined to classify this as an Emacs bug. In that case, why can't I reproduce it without submodules? I'm going off to eat cake before I tear my hair out in frustration. ``` ## Jiang Xin, 2013-07-27 23:25 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CANYiYbEiE%2BS3J3MR%3DYyK8Tn48zfmhzJjBjnuPtxzd6WtBU8duA%40mail.gmail.com In-Reply-To: ``` 2013/7/28 Ramkumar Ramachandra : > Duy Nguyen wrote: >> I was involved with this code (the gitdir setup code, not submodule) >> and am interested to know what's going on too. Could you produce a >> small script to reproduce it? > > Here's your reduced testcase. Just point mygit to a HEAD build. > > #!/bin/sh > > mygit=~/src/git/git > cd /tmp > $mygit clone https://github.com/artagnon/clayoven > cd clayoven > $mygit submodule add https://github.com/lewang/flx .elisp/flx > $mygit commit -a -m "Added submodule" > cd /tmp > ln -s clayoven/.elisp > cd .elisp/flx > EDITOR="emacs -Q" git commit --amend The keypoint is that you set GIT_EDITOR to "emacs -Q". Last night I can not reproduce the bug, because I use vim as the editor. Vim as a editor may call realpath before open file. I change the EDITOR(GIT_EDITOR) environment in your test script as follows: GIT_EDITOR="echo PWD: $(pwd); echo REALPATH: $(pwd -P); echo" \ git commit --amend And the result is : PWD: /tmp/.elisp/flx REALPATH: /private/tmp/clayoven/.elisp/flx ../../.git/modules/.elisp/flx/COMMIT_EDITMSG [master 248e49e] add usage information Author: Le Wang 1 file changed, 27 insertions(+) How to fix this bug, I have no idea now. -- Jiang Xin ``` ## Ramkumar Ramachandra, 2013-07-28 00:03 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CALkWK0%3DFA0g9W7T4%3DqULJS%2BFN%3DWduA3jDmpSq01xayr9SsncgA%40mail.gmail.com In-Reply-To: ``` Jiang Xin wrote: > I change the EDITOR(GIT_EDITOR) environment in your test script > as follows: > > GIT_EDITOR="echo PWD: $(pwd); echo REALPATH: $(pwd -P); echo" \ > git commit --amend See, what stumps be about this is the no-submodule case: #!/bin/sh mygit=~/src/git/git cd /tmp rm -rf clayoven lib $mygit clone https://github.com/artagnon/clayoven ln -s clayoven/lib cd lib/clayoven EDITOR="echo PWD: $(pwd); echo REALPATH: $(pwd -P); echo" \ git commit --amend # buffer-file-name = "/tmp/.git/modules/.elisp/flx/COMMIT_EDITMSG" >From the point of view of $EDITOR, how is this different? Yet, when you change EDITOR to "emacs -Q", it works fine. ``` ## Duy Nguyen, 2013-07-28 04:37 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CACsJy8AuAy7PnEU20k23gVRQoFrLhJrM6hfESZKshUA578c3Ew%40mail.gmail.com In-Reply-To: ``` On Sat, Jul 27, 2013 at 11:05 PM, Ramkumar Ramachandra wrote: > Here's your reduced testcase. Just point mygit to a HEAD build. > > #!/bin/sh > > mygit=~/src/git/git > cd /tmp > $mygit clone https://github.com/artagnon/clayoven > cd clayoven > $mygit submodule add https://github.com/lewang/flx .elisp/flx > $mygit commit -a -m "Added submodule" > cd /tmp > ln -s clayoven/.elisp > cd .elisp/flx > EDITOR="emacs -Q" git commit --amend > # buffer-file-name = "/tmp/.git/modules/.elisp/flx/COMMIT_EDITMSG" Here cwd is /tmp/.elisp/flx but "setup: cwd: " returns /tmp/clayoven/.elisp/flx. The COMMIT_EDITMSG path is ../../.git/modules/.elisp/flx/COMMIT_EDITMSG. If you follow the former cwd (without symlink resolution), you end up at /tmp/.git/modules. If you follow the latter cwd, you end up at /tmp/clayoven/.git/modules... I think instead of letting the kernel walk the path, emacs does it by itself. Still looking for documents about this behavior.. > Note that this is emacs 24.3. I used -Q to make sure that none of my > init magic (magit etc.) was responsible for changing directories or > doing something equally stupid. However, considering that it's > impossible to reproduce the problem with either cat or vim as the > EDITOR, you might be inclined to classify this as an Emacs bug. In > that case, why can't I reproduce it without submodules? > > I'm going off to eat cake before I tear my hair out in frustration. -- Duy ``` ## Duy Nguyen, 2013-07-28 05:00 Subject: Re: [BUG] git_path() returns relative paths Message-ID: <20130728050009.GA10844@lanh> URL: https://gitlist.dev/e/20130728050009.GA10844%40lanh In-Reply-To: ``` On Sat, Jul 27, 2013 at 09:35:40PM +0530, Ramkumar Ramachandra wrote: > Duy Nguyen wrote: > > I was involved with this code (the gitdir setup code, not submodule) > > and am interested to know what's going on too. Could you produce a > > small script to reproduce it? > > Here's your reduced testcase. Just point mygit to a HEAD build. > > #!/bin/sh > > mygit=~/src/git/git > cd /tmp > $mygit clone https://github.com/artagnon/clayoven > cd clayoven > $mygit submodule add https://github.com/lewang/flx .elisp/flx > $mygit commit -a -m "Added submodule" > cd /tmp > ln -s clayoven/.elisp > cd .elisp/flx > EDITOR="emacs -Q" git commit --amend > # buffer-file-name = "/tmp/.git/modules/.elisp/flx/COMMIT_EDITMSG" > > Note that this is emacs 24.3. I used -Q to make sure that none of my > init magic (magit etc.) was responsible for changing directories or > doing something equally stupid. However, considering that it's > impossible to reproduce the problem with either cat or vim as the > EDITOR, you might be inclined to classify this as an Emacs bug. In > that case, why can't I reproduce it without submodules? How about something like this as a workaround for emacs? -- 8< -- diff --git a/editor.c b/editor.c index 27bdecd..fda3e41 100644 --- a/editor.c +++ b/editor.c @@ -37,7 +37,8 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en return error("Terminal is dumb, but EDITOR unset"); if (strcmp(editor, ":")) { - const char *args[] = { editor, path, NULL }; + char *rpath = realpath(path, NULL); + const char *args[] = { editor, rpath, NULL }; struct child_process p; int ret, sig; @@ -51,6 +52,7 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en sigchain_push(SIGINT, SIG_IGN); sigchain_push(SIGQUIT, SIG_IGN); ret = finish_command(&p); + free(rpath); sig = ret - 128; sigchain_pop(SIGINT); sigchain_pop(SIGQUIT); -- 8< -- > > I'm going off to eat cake before I tear my hair out in frustration. ``` ## Ramkumar Ramachandra, 2013-07-28 08:49 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CALkWK0nFDyFqBPatsbQ-0mGXQ-6N5Xs9gKioknuGX9a7UiD4Fg%40mail.gmail.com In-Reply-To: ``` Duy Nguyen wrote: > I think instead of letting the kernel walk the path, emacs does it by > itself. If this were true, shouldn't we be able to reproduce the behavior with my no-submodules symlink testcase? How can it resolve symlinks in one case, and not in the other case? ``` ## Duy Nguyen, 2013-07-28 09:06 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CACsJy8CAfCVtTmgtEaix6RwYm4Sj7Ot_BjuObS%2BqvQ%3DQNnQ0OA%40mail.gmail.com In-Reply-To: ``` On Sun, Jul 28, 2013 at 3:49 PM, Ramkumar Ramachandra wrote: > Duy Nguyen wrote: >> I think instead of letting the kernel walk the path, emacs does it by >> itself. > > If this were true, shouldn't we be able to reproduce the behavior with > my no-submodules symlink testcase? How can it resolve symlinks in one > case, and not in the other case? In the no-submodules symlink test case, the path given to the editor is .git/COMMIT_EDITMSG, no following ".." back in the symlink target. This bug can be reproduced without git involved: mkdir z z/a z/b echo ha >z/a/file ln -s z/b cd b cat ../a/file emacs ../a/file # fail to open 'file' -- Duy ``` ## Ramkumar Ramachandra, 2013-07-28 09:16 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CALkWK0mFj3fy7k%3DLirZ9LotGAxt7ptSpzZpR0sCg4OPmPb1WjA%40mail.gmail.com In-Reply-To: ``` Duy Nguyen wrote: > In the no-submodules symlink test case, the path given to the editor > is .git/COMMIT_EDITMSG, no following ".." back in the symlink target. Thanks, and sorry about the stupidity. I just patched builtin/commit.c to check this. > This bug can be reproduced without git involved: I'm taking this up with Emacs-Devel; you are CC'ed. ``` ## Ramkumar Ramachandra, 2013-07-28 09:24 Subject: Re: [BUG] git_path() returns relative paths Message-ID: URL: https://gitlist.dev/e/CALkWK0n-a8XB_ZRmQBTxq2DQ79VfPYyuvPBsi%3DZrf_ThzUsoyw%40mail.gmail.com In-Reply-To: <20130728050009.GA10844@lanh> ``` Duy Nguyen wrote: > How about something like this as a workaround for emacs? Even if we do manage to patch Emacs now, we still need to support older versions: so yeah, this is an urgent candidate for maint. I'm waiting for the word from Emacs-Devel before writing out a commit message. ```