{"thread":{"id":"27350","subject":"git difftool does does not respect current working directory","startedAt":"2011-05-14T14:25:40Z","lastAt":"2011-05-25T04:19:28Z","messageCount":21,"participants":["Frédéric Heitzmann","Junio C Hamano","David Aguilar","Michael J Gruber","Ævar Arnfjörð Bjarmason"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"167848","messageId":"loom.20110514T160931-46@post.gmane.org","threadId":"27350","inReplyTo":null,"subject":"git difftool does does not respect current working directory","fromName":"Frédéric Heitzmann","fromEmail":"frederic.heitzmann@gmail.com","sentAt":"2011-05-14T14:25:40Z","receivedAt":"2011-05-14T14:25:40Z","isPatch":false,"sender":{"key":"frederic.heitzmann@gmail.com","avatar":null},"body":"Hello.\n\nIt is useful to compare the modified version of a file to HEAD's version, in\norder to review changes before committing. gitk is fine for this but I often use\ngit difftool -t gvimdiff, so that I can rewrite code right into my diff editor.\nDoing this, it is very likely that I open some more files (e.g. foo.h\ncorresponding to foo.c) in gvimdiff.\nUnfortunately, 'git difftool' does not keep the current working directory while\nlaunching gvimdiff.\n\n=> Is it done on purpose ?\nIf not, it is probably a good idea to fix this.\n\nI will be more than happy to contribute but I have some hard time to get\nfamiliar with git source code.\nAny help to locate the proper pieces of code would be realy appreciated.\n\n--\nFred\n"},{"id":"167961","messageId":"7v1uzznr09.fsf@alter.siamese.dyndns.org","threadId":"27350","inReplyTo":"loom.20110514T160931-46@post.gmane.org","subject":"Re: git difftool does does not respect current working directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-16T05:39:18Z","receivedAt":"2011-05-16T05:39:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Frédéric Heitzmann writes:\n\n> Unfortunately, 'git difftool' does not keep the current working directory while\n> launching gvimdiff.\n>\n> => Is it done on purpose ?\n> If not, it is probably a good idea to fix this.\n\nI will not comment on \"on purpose?\" part, as I do not use difftool myself.\n\nBut the right set of questions to ask is not the above, but these:\n\n - Is it on purpose that difftool runs its diff viewer from the top of the\n working tree?\n\n - Is there any existing user who depends on that current behaviour? IOW,\n would anybody suffer if difftool suddenly starts to run the diff viewer\n from the subdirectory that the user started \"git difftool\" from?\n\nIf the answers to both of them are No, then it might be a good idea to\nchange the behaviour.\n"},{"id":"168292","messageId":"20110520035856.GA13582@gmail.com","threadId":"27350","inReplyTo":"7v1uzznr09.fsf@alter.siamese.dyndns.org","subject":"Re: git difftool does does not respect current working directory","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-05-20T03:59:00Z","receivedAt":"2011-05-20T03:59:00Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"\nHello,\n\nOn Sun, May 15, 2011 at 10:39:18PM -0700, Junio C Hamano wrote:\n> Frédéric Heitzmann writes:\n> \n> > Unfortunately, 'git difftool' does not keep the current working directory while\n> > launching gvimdiff.\n> >\n> > => Is it done on purpose ?\n> > If not, it is probably a good idea to fix this.\n> \n> I will not comment on \"on purpose?\" part, as I do not use difftool myself.\n> \n> But the right set of questions to ask is not the above, but these:\n> \n> - Is it on purpose that difftool runs its diff viewer from the top of the\n> working tree?\n> \n> - Is there any existing user who depends on that current behaviour? IOW,\n> would anybody suffer if difftool suddenly starts to run the diff viewer\n> from the subdirectory that the user started \"git difftool\" from?\n> \n> If the answers to both of them are No, then it might be a good idea to\n> change the behaviour.\n\nAnother thing to consider is ensuring that there is a\nconsistency between 'diff' and 'difftool'.\n\nWhen you run 'git diff' from a subdirectory git will show you\na diff against the entire tree. In this respect, 'difftool' is\nconsistent. In that sense, yes, this is very much \"on purpose\".\n\nIs there something that isn't working because of the internal\nchdir? I'm not sure if you want to change the chdir behavior\nfor aesthetic purposes or if there's something it is\npreventing you from doing. If what you're trying to accomplish\nis to have 'difftool' only show you changes within the current\ndirectory then you can accomplish that today by passing \".\",\ne.g. \"git difftool .\"\n\n\nImplementation details:\n\ndifftool is a thin wrapper around 'git diff'. Specifically,\nit is implemented as a $GIT_EXTERNAL_DIFF script which means\nthat it inherits most of its behavior from 'diff'.\nThe \"chdir to root\" behavior actually happens inside of\n'git diff'.\n\nCan we can change the behavior? Sure, anything is possible.\nThe question is *should* we change the behavior?\nEven though I highly doubt there are any scripts relying on it,\nI don't think we gain much by doing so. The downside to\nchanging it is that we lose consistency, which is not so good.\nI hope that's a compelling enough argument :-)\n\nI hope the \".\" thing helps.\n\nCheers,\n-- \n\t\t\t\t\tDavid\n"},{"id":"168293","messageId":"20110520041045.GB13582@gmail.com","threadId":"27350","inReplyTo":"20110520035856.GA13582@gmail.com","subject":"Re: git difftool does does not respect current working directory","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-05-20T04:10:46Z","receivedAt":"2011-05-20T04:10:46Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Thu, May 19, 2011 at 08:59:00PM -0700, David Aguilar wrote:\n> \n> Hello,\n> \n> On Sun, May 15, 2011 at 10:39:18PM -0700, Junio C Hamano wrote:\n> > Frédéric Heitzmann writes:\n> > \n> > > Unfortunately, 'git difftool' does not keep the current working directory while\n> > > launching gvimdiff.\n> > >\n> > > => Is it done on purpose ?\n> > > If not, it is probably a good idea to fix this.\n> > \n> > [...snip...]\n> > \n> > If the answers to both of them are No, then it might be a good idea to\n> > change the behaviour.\n> \n> Another thing to consider is ensuring that there is a\n> consistency between 'diff' and 'difftool'.\n> \n> When you run 'git diff' from a subdirectory git will show you\n> a diff against the entire tree. In this respect, 'difftool' is\n> consistent. In that sense, yes, this is very much \"on purpose\".\n> \n> [...snip...]\n\nI just realized that I was focusing on the \"whole tree diff\"\naspect which really is orthogonal to the \"chdir to root\"\ndone by 'git diff' when running $GIT_EXTERNAL_DIFF scripts.\n\nI imagine you want to be able to use vimdiff while diffing\nand open files, etc., from the current directory?\n\nIn that case, maybe we can have the best of both worlds --\nwhole-tree diff and keeping the current directory.\nThat would be kinda like what 'git status' does when it shows\nyou relative paths above and beside the current directory.\n\nWe would have to change the way $GIT_EXTERNAL_DIFF works so\nthat it preserves the current directory and constructs\npaths relative to it. Patches welcome :-)\n\nI don't really know whether it would be a good thing to do,\nthough, since I have not dug too deep into how the extdiff\ncode is structured.\n-- \n\t\t\t\t\tDavid\n"},{"id":"168295","messageId":"7vwrhm3scl.fsf@alter.siamese.dyndns.org","threadId":"27350","inReplyTo":"20110520041045.GB13582@gmail.com","subject":"Re: git difftool does does not respect current working directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-20T04:31:54Z","receivedAt":"2011-05-20T04:31:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> We would have to change the way $GIT_EXTERNAL_DIFF works so\n> that it preserves the current directory and constructs\n> paths relative to it. Patches welcome :-)\n\nI am afraild that would break a lot more than difftool.\n\nIf we really wanted to change the behaviour, the external diff interface\nneeds to export the value of prefix (i.e. what the original subdirectory\nwas), and the script that is spawned as $GIT_EXTERNAL_DIFF (optionally\noptionally) take it into account, perhaps by cd'ing back to that\nsubdirectory and possibly moving or renaming the temporary files to suit\nits needs (I think recently we also saw a request to rename the temporary\nfiles).\n\nOr something like that.\n"},{"id":"168296","messageId":"20110520044851.GD13582@gmail.com","threadId":"27350","inReplyTo":"7vwrhm3scl.fsf@alter.siamese.dyndns.org","subject":"Re: git difftool does does not respect current working directory","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-05-20T04:48:52Z","receivedAt":"2011-05-20T04:48:52Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Thu, May 19, 2011 at 09:31:54PM -0700, Junio C Hamano wrote:\n> David Aguilar writes:\n> \n> > We would have to change the way $GIT_EXTERNAL_DIFF works so\n> > that it preserves the current directory and constructs\n> > paths relative to it. Patches welcome :-)\n> \n> I am afraild that would break a lot more than difftool.\n> \n> If we really wanted to change the behaviour, the external diff interface\n> needs to export the value of prefix (i.e. what the original subdirectory\n> was), and the script that is spawned as $GIT_EXTERNAL_DIFF (optionally\n> optionally) take it into account, perhaps by cd'ing back to that\n> subdirectory and possibly moving or renaming the temporary files to suit\n> its needs (I think recently we also saw a request to rename the temporary\n> files).\n> \n> Or something like that.\n\nYup, yup. That's a lot of machinery for a relatively small\ngain. Simple is simple, simple is good. Thanks for\noutlining how someone could implement it, though.\n\nI won't do it myself but if someone is motivated enough then\nyour email at least gives an idea about how to go about doing\nit. git-difftool--helper could chdir to $prefix and diff each\nfile with $(git rev-parse --show-cdup)/$path as the path since\nit may no longer be at the root.\n\nThis seems very messy so I don't really want to sound too\nencouraging about going down this route. I probably\nshouldn't have encouraged looking at the temporary files\nthing in the other thread either.\n\nThanks,\n-- \n\t\t\t\t\tDavid\n"},{"id":"168370","messageId":"4DD7874A.2050604@gmail.com","threadId":"27350","inReplyTo":"20110520044851.GD13582@gmail.com","subject":"Re: git difftool does does not respect current working directory","fromName":"Frédéric Heitzmann","fromEmail":"frederic.heitzmann@gmail.com","sentAt":"2011-05-21T09:35:06Z","receivedAt":"2011-05-21T09:35:06Z","isPatch":false,"sender":{"key":"frederic.heitzmann@gmail.com","avatar":null},"body":"Reading your replies, my understanding is :\n- difftool is consistent with diff, and chdir to root directory. It is \nseems indeed very common to have diffs showing from the root directory.\n- on the overhand, openning gvimdiff via difftool and having a new cwd \nis for sure not consistent with usual gvim text editing.\n\nI am afraid I am going to need some gvim trick like :\n$ git difftool -x \"gvimdiff -f -d -c 'wincmd l' -c 'cd $PWD' \" my_file\n\nNot sure that it is less messy though ;-)\nIf there is no stronger need to adapt git-difftool, for gvimdiff or any \nother difftool, we could probably settle for it.\n\nThanks for you help.\n--\nFred\n\nLe 20/05/2011 06:48, David Aguilar a écrit :\n> On Thu, May 19, 2011 at 09:31:54PM -0700, Junio C Hamano wrote:\n>> David Aguilar writes:\n>>\n>>> We would have to change the way $GIT_EXTERNAL_DIFF works so\n>>> that it preserves the current directory and constructs\n>>> paths relative to it. Patches welcome :-)\n>> I am afraild that would break a lot more than difftool.\n>>\n>> If we really wanted to change the behaviour, the external diff interface\n>> needs to export the value of prefix (i.e. what the original subdirectory\n>> was), and the script that is spawned as $GIT_EXTERNAL_DIFF (optionally\n>> optionally) take it into account, perhaps by cd'ing back to that\n>> subdirectory and possibly moving or renaming the temporary files to suit\n>> its needs (I think recently we also saw a request to rename the temporary\n>> files).\n>>\n>> Or something like that.\n> Yup, yup. That's a lot of machinery for a relatively small\n> gain. Simple is simple, simple is good. Thanks for\n> outlining how someone could implement it, though.\n>\n> I won't do it myself but if someone is motivated enough then\n> your email at least gives an idea about how to go about doing\n> it. git-difftool--helper could chdir to $prefix and diff each\n> file with $(git rev-parse --show-cdup)/$path as the path since\n> it may no longer be at the root.\n>\n> This seems very messy so I don't really want to sound too\n> encouraging about going down this route. I probably\n> shouldn't have encouraged looking at the temporary files\n> thing in the other thread either.\n>\n> Thanks,\n"},{"id":"168437","messageId":"20110522061446.GA49297@gmail.com","threadId":"27350","inReplyTo":"4DD7874A.2050604@gmail.com","subject":"Re: git difftool does does not respect current working directory","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-05-22T06:14:48Z","receivedAt":"2011-05-22T06:14:48Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sat, May 21, 2011 at 11:35:06AM +0200, Frédéric Heitzmann wrote:\n> Reading your replies, my understanding is :\n> - difftool is consistent with diff, and chdir to root directory. It\n> is seems indeed very common to have diffs showing from the root\n> directory.\n> - on the overhand, openning gvimdiff via difftool and having a new\n> cwd is for sure not consistent with usual gvim text editing.\n> \n> I am afraid I am going to need some gvim trick like :\n> $ git difftool -x \"gvimdiff -f -d -c 'wincmd l' -c 'cd $PWD' \" my_file\n> \n> Not sure that it is less messy though ;-)\n> If there is no stronger need to adapt git-difftool, for gvimdiff or\n> any other difftool, we could probably settle for it.\n\nI think updating git-difftool--helper.sh to pass a chdir to vim\nmight be just the thing to do. git-difftool.perl can be\nupdated to set $GIT_DIFFTOOL_PWD so that the helper can use it\nas -c 'cd $GIT_DIFFTOOL_PWD'. I'll see if I can whip up a patch\nin a lil bit.\n-- \n\t\t\t\t\tDavid\n"},{"id":"168438","messageId":"7vwrhjxn4t.fsf@alter.siamese.dyndns.org","threadId":"27350","inReplyTo":"20110522061446.GA49297@gmail.com","subject":"Re: git difftool does does not respect current working directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-22T06:30:58Z","receivedAt":"2011-05-22T06:30:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> I think updating git-difftool--helper.sh to pass a chdir to vim\n> might be just the thing to do. git-difftool.perl can be\n> updated to set $GIT_DIFFTOOL_PWD so that the helper can use it\n> as -c 'cd $GIT_DIFFTOOL_PWD'. I'll see if I can whip up a patch\n> in a lil bit.\n\nHmm, would this benefit from sharing some concepts with 7cf16a1\n(handle_alias: provide GIT_PREFIX to !alias, 2011-04-27)?\n\nIf it helps, we might want to uniformly give this information to all\nexternal processes and programs, including hooks.\n"},{"id":"168439","messageId":"20110522065051.GB49297@gmail.com","threadId":"27350","inReplyTo":"7vwrhjxn4t.fsf@alter.siamese.dyndns.org","subject":"Re: git difftool does does not respect current working directory","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-05-22T06:50:53Z","receivedAt":"2011-05-22T06:50:53Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sat, May 21, 2011 at 11:30:58PM -0700, Junio C Hamano wrote:\n> David Aguilar writes:\n> \n> > I think updating git-difftool--helper.sh to pass a chdir to vim\n> > might be just the thing to do. git-difftool.perl can be\n> > updated to set $GIT_DIFFTOOL_PWD so that the helper can use it\n> > as -c 'cd $GIT_DIFFTOOL_PWD'. I'll see if I can whip up a patch\n> > in a lil bit.\n> \n> Hmm, would this benefit from sharing some concepts with 7cf16a1\n> (handle_alias: provide GIT_PREFIX to !alias, 2011-04-27)?\n> \n> If it helps, we might want to uniformly give this information to all\n> external processes and programs, including hooks.\n\nYes, this would be very helpful.\nWithout this my patch would have to touch git-difftool.perl,\ngit-mergetool.sh, and git-mergetool--helper.sh.\n\nWith $GIT_PREFIX it would touch git-mergetool--helper.sh only.\n-- \n\t\t\t\t\tDavid\n"},{"id":"168442","messageId":"1306058229-93800-1-git-send-email-davvid@gmail.com","threadId":"27350","inReplyTo":"7vwrhjxn4t.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/3] setup: Provide GIT_PREFIX to built-ins","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-05-22T09:57:07Z","receivedAt":"2011-05-22T09:57:07Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"GIT_PREFIX was added in 7cf16a14f5c070f7b14cf28023769450133172ae so that\naliases can know the directory from which a !alias was called.\n\nKnowing the prefix relative to the root is helpful in other programs\nso export it to built-ins as well.\n\nSigned-off-by: David Aguilar \n---\n setup.c | 6 ++++++\n t/t1020-subdirectory.sh | 16 ++++++++++++++++\n 2 files changed, 22 insertions(+), 0 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex b6e6b5a..fc169a4 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -603,6 +603,12 @@ const char *setup_git_directory_gently(int *nongit_ok)\n \tconst char *prefix;\n \n \tprefix = setup_git_directory_gently_1(nongit_ok);\n+\t/* Provide the prefix to all external processes and programs */\n+\tif (prefix)\n+\t\tsetenv(\"GIT_PREFIX\", prefix, 1);\n+\telse\n+\t\tunsetenv(\"GIT_PREFIX\");\n+\n \tif (startup_info) {\n \t\tstartup_info->have_repository = !nongit_ok || !*nongit_ok;\n \t\tstartup_info->prefix = prefix;\ndiff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh\nindex ddc3921..a85b594 100755\n--- a/t/t1020-subdirectory.sh\n+++ b/t/t1020-subdirectory.sh\n@@ -139,6 +139,22 @@ test_expect_success 'GIT_PREFIX for !alias' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'GIT_PREFIX for built-ins' '\n+\t# Use GIT_EXTERNAL_DIFF to test that the \"diff\" built-in\n+\t# receives the GIT_PREFIX variable.\n+\tprintf \"dir/\" >expect &&\n+\tprintf \"#!/bin/sh\\n\" >diff &&\n+\tprintf \"printf \\\"\\$GIT_PREFIX\\\"\\n\" >>diff &&\n+\tchmod +x diff &&\n+\t(\n+\t\tcd dir &&\n+\t\tprintf \"change\" >two &&\n+\t\tenv GIT_EXTERNAL_DIFF=./diff git diff >../actual\n+\t\tgit checkout -- two\n+\t) &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'no file/rev ambiguity check inside .git' '\n \tgit commit -a -m 1 &&\n \t(\n-- \n1.7.5.2.317.g391b14\n"},{"id":"168444","messageId":"1306058229-93800-2-git-send-email-davvid@gmail.com","threadId":"27350","inReplyTo":"1306058229-93800-1-git-send-email-davvid@gmail.com","subject":"[PATCH 2/3] git: Remove handling for GIT_PREFIX","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-05-22T09:57:08Z","receivedAt":"2011-05-22T09:57:08Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"handle_alias() no longer needs to set GIT_PREFIX since it is defined\nin setup_git_directory_gently(). Remove the duplicated effort and use\nrun_command_v_opt() since there is no need to setup the environment.\n\nSigned-off-by: David Aguilar \n---\n git.c | 10 +---------\n 1 files changed, 1 insertions(+), 9 deletions(-)\n\ndiff --git a/git.c b/git.c\nindex a5ef3c6..60a9403 100644\n--- a/git.c\n+++ b/git.c\n@@ -185,8 +185,6 @@ static int handle_alias(int *argcp, const char ***argv)\n \t\tif (alias_string[0] == '!') {\n \t\t\tconst char **alias_argv;\n \t\t\tint argc = *argcp, i;\n-\t\t\tstruct strbuf sb = STRBUF_INIT;\n-\t\t\tconst char *env[2];\n \n \t\t\tcommit_pager_choice();\n \n@@ -197,13 +195,7 @@ static int handle_alias(int *argcp, const char ***argv)\n \t\t\t\talias_argv[i] = (*argv)[i];\n \t\t\talias_argv[argc] = NULL;\n \n-\t\t\tstrbuf_addstr(&sb, \"GIT_PREFIX=\");\n-\t\t\tif (subdir)\n-\t\t\t\tstrbuf_addstr(&sb, subdir);\n-\t\t\tenv[0] = sb.buf;\n-\t\t\tenv[1] = NULL;\n-\t\t\tret = run_command_v_opt_cd_env(alias_argv, RUN_USING_SHELL, NULL, env);\n-\t\t\tstrbuf_release(&sb);\n+\t\t\tret = run_command_v_opt(alias_argv, RUN_USING_SHELL);\n \t\t\tif (ret >= 0) /* normal exit */\n \t\t\t\texit(ret);\n \n-- \n1.7.5.2.317.g391b14\n"},{"id":"168443","messageId":"1306058229-93800-3-git-send-email-davvid@gmail.com","threadId":"27350","inReplyTo":"1306058229-93800-1-git-send-email-davvid@gmail.com","subject":"[PATCH 3/3] git-mergetool--lib: Make vimdiff retain the current directory","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-05-22T09:57:09Z","receivedAt":"2011-05-22T09:57:09Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"When using difftool with vimdiff it can be unexpected that\nthe current directory changes to the root of the project.\nTell vim to chdir to the value of $GIT_PREFIX to fix this.\n\nCare is taken to quote the variable so that vim expands it.\nThis avoids problems when directory names contain spaces.\n\nSigned-off-by: David Aguilar \nReported-by: Frédéric Heitzmann \n---\n git-mergetool--lib.sh | 13 +++++++++++++\n 1 files changed, 13 insertions(+), 0 deletions(-)\n\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 4db9212..ece6a08 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -187,7 +187,9 @@ run_merge_tool () {\n \t\t\tfi\n \t\t\tcheck_unchanged\n \t\telse\n+\t\t\tresolve_git_prefix\n \t\t\t\"$merge_tool_path\" -R -f -d -c \"wincmd l\" \\\n+\t\t\t\t-c 'cd $GIT_PREFIX' \\\n \t\t\t\t\"$LOCAL\" \"$REMOTE\"\n \t\tfi\n \t\t;;\n@@ -198,7 +200,9 @@ run_merge_tool () {\n \t\t\t\t\"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n \t\t\tcheck_unchanged\n \t\telse\n+\t\t\tresolve_git_prefix\n \t\t\t\"$merge_tool_path\" -R -f -d -c \"wincmd l\" \\\n+\t\t\t\t-c 'cd $GIT_PREFIX' \\\n \t\t\t\t\"$LOCAL\" \"$REMOTE\"\n \t\tfi\n \t\t;;\n@@ -437,3 +441,12 @@ get_merge_tool () {\n \tfi\n \techo \"$merge_tool\"\n }\n+\n+resolve_git_prefix() {\n+\t# If GIT_PREFIX is empty then we cannot use it in tools\n+\t# that expect to be able to chdir() to its value.\n+\tif test -z \"$GIT_PREFIX\"; then\n+\t\tGIT_PREFIX=.\n+\t\texport GIT_PREFIX\n+\tfi\n+}\n-- \n1.7.5.2.317.g391b14\n"},{"id":"168481","messageId":"4DDA0069.9010500@drmicha.warpmail.net","threadId":"27350","inReplyTo":"1306058229-93800-3-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 3/3] git-mergetool--lib: Make vimdiff retain the current directory","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-23T06:36:25Z","receivedAt":"2011-05-23T06:36:25Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"David Aguilar venit, vidit, dixit 22.05.2011 11:57:\n> When using difftool with vimdiff it can be unexpected that\n> the current directory changes to the root of the project.\n> Tell vim to chdir to the value of $GIT_PREFIX to fix this.\n> \n> Care is taken to quote the variable so that vim expands it.\n> This avoids problems when directory names contain spaces.\n> \n> Signed-off-by: David Aguilar \n> Reported-by: Frédéric Heitzmann \n> ---\n> git-mergetool--lib.sh | 13 +++++++++++++\n> 1 files changed, 13 insertions(+), 0 deletions(-)\n> \n> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\n> index 4db9212..ece6a08 100644\n> --- a/git-mergetool--lib.sh\n> +++ b/git-mergetool--lib.sh\n> @@ -187,7 +187,9 @@ run_merge_tool () {\n> \t\t\tfi\n> \t\t\tcheck_unchanged\n> \t\telse\n> +\t\t\tresolve_git_prefix\n> \t\t\t\"$merge_tool_path\" -R -f -d -c \"wincmd l\" \\\n> +\t\t\t\t-c 'cd $GIT_PREFIX' \\\n> \t\t\t\t\"$LOCAL\" \"$REMOTE\"\n> \t\tfi\n> \t\t;;\n> @@ -198,7 +200,9 @@ run_merge_tool () {\n> \t\t\t\t\"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n> \t\t\tcheck_unchanged\n> \t\telse\n> +\t\t\tresolve_git_prefix\n> \t\t\t\"$merge_tool_path\" -R -f -d -c \"wincmd l\" \\\n> +\t\t\t\t-c 'cd $GIT_PREFIX' \\\n> \t\t\t\t\"$LOCAL\" \"$REMOTE\"\n> \t\tfi\n> \t\t;;\n> @@ -437,3 +441,12 @@ get_merge_tool () {\n> \tfi\n> \techo \"$merge_tool\"\n> }\n> +\n> +resolve_git_prefix() {\n> +\t# If GIT_PREFIX is empty then we cannot use it in tools\n> +\t# that expect to be able to chdir() to its value.\n> +\tif test -z \"$GIT_PREFIX\"; then\n> +\t\tGIT_PREFIX=.\n> +\t\texport GIT_PREFIX\n> +\tfi\n> +}\n\nHmmm. Maybe we should export \".\" when there is no prefix? Maybe it's not\ntoo late to change that aspect of GIT_PREFIX. We went through some\niteration back then for !alias.\n\nMichael\n"},{"id":"168489","messageId":"FE7878D1-20E4-4CD4-B3FB-96322AA75855@gmail.com","threadId":"27350","inReplyTo":"4DDA0044.2060207@drmicha.warpmail.net","subject":"Re: [PATCH 1/3] setup: Provide GIT_PREFIX to built-ins","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-05-23T08:40:48Z","receivedAt":"2011-05-23T08:40:48Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Added git@vger to the cc list. I sent y'all two copies of these patches because I forgot to cc the list the first time...\n\nOn May 22, 2011, at 11:35 PM, Michael J Gruber wrote:\n\n> David Aguilar venit, vidit, dixit 22.05.2011 11:54:\n>> GIT_PREFIX was added in 7cf16a14f5c070f7b14cf28023769450133172ae so that\n>> aliases can know the directory from which a !alias was called.\n>> \n>> Knowing the prefix relative to the root is helpful in other programs\n>> so export it to built-ins as well.\n>> \n>> Signed-off-by: David Aguilar \n>> ---\n>> setup.c | 6 ++++++\n>> t/t1020-subdirectory.sh | 16 ++++++++++++++++\n>> 2 files changed, 22 insertions(+), 0 deletions(-)\n>> \n>> diff --git a/setup.c b/setup.c\n>> index b6e6b5a..fc169a4 100644\n>> --- a/setup.c\n>> +++ b/setup.c\n>> @@ -603,6 +603,12 @@ const char *setup_git_directory_gently(int *nongit_ok)\n>> const char *prefix;\n>> \n>> prefix = setup_git_directory_gently_1(nongit_ok);\n>> + /* Provide the prefix to all external processes and programs */\n>> + if (prefix)\n>> + setenv(\"GIT_PREFIX\", prefix, 1);\n>> + else\n>> + unsetenv(\"GIT_PREFIX\");\n>> +\n> \n> Do we really want to unset it? This is different from the existing\n> behaviour (but not more useful). But see also my comment on 3/3: We may\n> want to do something different which is also more useful.\n\nI thought the behavior was actually the same.\n\nThe current alias code sets the value GIT_PREFIX= in the strbuf. When prefix is empty nothing else is added to the strbuf. The run_command function actually checks for FOO= with empty after the equals sign and translates it into unsetenv. That allows code to unset vars using that interface.\n\nIf we can do something better that'd be good. Unsetting the variable also protects us from whatever randomness might be in there, which was my primary concern.\n\n> \n>> if (startup_info) {\n>> startup_info->have_repository = !nongit_ok || !*nongit_ok;\n>> startup_info->prefix = prefix;\n>> diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh\n>> index ddc3921..a85b594 100755\n>> --- a/t/t1020-subdirectory.sh\n>> +++ b/t/t1020-subdirectory.sh\n>> @@ -139,6 +139,22 @@ test_expect_success 'GIT_PREFIX for !alias' '\n>> test_cmp expect actual\n>> '\n>> \n>> +test_expect_success 'GIT_PREFIX for built-ins' '\n>> + # Use GIT_EXTERNAL_DIFF to test that the \"diff\" built-in\n>> + # receives the GIT_PREFIX variable.\n>> + printf \"dir/\" >expect &&\n>> + printf \"#!/bin/sh\\n\" >diff &&\n>> + printf \"printf \\\"\\$GIT_PREFIX\\\"\\n\" >>diff &&\n>> + chmod +x diff &&\n>> + (\n>> + cd dir &&\n>> + printf \"change\" >two &&\n>> + env GIT_EXTERNAL_DIFF=./diff git diff >../actual\n> \n> In passsing, this also tests the fact that GIT_EXTERNAL_DIFF is relative\n> to the repo root and not to cwd...\n\nThat's true. Another thing is that this only affects built-ins. I wanted to set the variable for git-foo external programs too but that means adding a call to setup_git_directory_gently() which we currently do not do in that codepath. I guess external scripts can call rev-parse --show-prefix themselves? Or is this worth making consistent?\n\n> \n>> + git checkout -- two\n>> + ) &&\n>> + test_cmp expect actual\n>> +'\n>> +\n>> test_expect_success 'no file/rev ambiguity check inside .git' '\n>> git commit -a -m 1 &&\n>> (\n> \n> Overall I think it's a good change, btw. But it leaves it up to the\n> (script) user to know whether git has actually changed the cwd or not,\n> i.e.: Is $(pwd) where the user called us from or $(pwd)/$GIT_PREFIX?\n> That may may be a non-issue, though.\n> \n> Michael\n\nIt's a non-issue for my use case but I can see it being confusing.\n\nFor example, mergetool--lib's merge mode codepath can be run from subdirectories. The diff mode codepaths all assume that we are at the root (because git diff put us there).\n\nThanks (and please let me know if my unsetenv analysis is incorrect),\n-- \n David"},{"id":"168491","messageId":"4DDA2FD9.5070807@drmicha.warpmail.net","threadId":"27350","inReplyTo":"FE7878D1-20E4-4CD4-B3FB-96322AA75855@gmail.com","subject":"Re: [PATCH 1/3] setup: Provide GIT_PREFIX to built-ins","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-23T09:58:49Z","receivedAt":"2011-05-23T09:58:49Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"David Aguilar venit, vidit, dixit 23.05.2011 10:40:\n> Added git@vger to the cc list. I sent y'all two copies of these patches because I forgot to cc the list the first time...\n> \n> On May 22, 2011, at 11:35 PM, Michael J Gruber wrote:\n> \n>> David Aguilar venit, vidit, dixit 22.05.2011 11:54:\n>>> GIT_PREFIX was added in 7cf16a14f5c070f7b14cf28023769450133172ae so that\n>>> aliases can know the directory from which a !alias was called.\n>>>\n>>> Knowing the prefix relative to the root is helpful in other programs\n>>> so export it to built-ins as well.\n>>>\n>>> Signed-off-by: David Aguilar \n>>> ---\n>>> setup.c | 6 ++++++\n>>> t/t1020-subdirectory.sh | 16 ++++++++++++++++\n>>> 2 files changed, 22 insertions(+), 0 deletions(-)\n>>>\n>>> diff --git a/setup.c b/setup.c\n>>> index b6e6b5a..fc169a4 100644\n>>> --- a/setup.c\n>>> +++ b/setup.c\n>>> @@ -603,6 +603,12 @@ const char *setup_git_directory_gently(int *nongit_ok)\n>>> const char *prefix;\n>>>\n>>> prefix = setup_git_directory_gently_1(nongit_ok);\n>>> + /* Provide the prefix to all external processes and programs */\n>>> + if (prefix)\n>>> + setenv(\"GIT_PREFIX\", prefix, 1);\n>>> + else\n>>> + unsetenv(\"GIT_PREFIX\");\n>>> +\n>>\n>> Do we really want to unset it? This is different from the existing\n>> behaviour (but not more useful). But see also my comment on 3/3: We may\n>> want to do something different which is also more useful.\n> \n> I thought the behavior was actually the same.\n> \n> The current alias code sets the value GIT_PREFIX= in the strbuf. When prefix is empty nothing else is added to the strbuf. The run_command function actually checks for FOO= with empty after the equals sign and translates it into unsetenv. That allows code to unset vars using that interface.\n> \n> If we can do something better that'd be good. Unsetting the variable also protects us from whatever randomness might be in there, which was my primary concern.\n> \n>>\n>>> if (startup_info) {\n>>> startup_info->have_repository = !nongit_ok || !*nongit_ok;\n>>> startup_info->prefix = prefix;\n>>> diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh\n>>> index ddc3921..a85b594 100755\n>>> --- a/t/t1020-subdirectory.sh\n>>> +++ b/t/t1020-subdirectory.sh\n>>> @@ -139,6 +139,22 @@ test_expect_success 'GIT_PREFIX for !alias' '\n>>> test_cmp expect actual\n>>> '\n>>>\n>>> +test_expect_success 'GIT_PREFIX for built-ins' '\n>>> + # Use GIT_EXTERNAL_DIFF to test that the \"diff\" built-in\n>>> + # receives the GIT_PREFIX variable.\n>>> + printf \"dir/\" >expect &&\n>>> + printf \"#!/bin/sh\\n\" >diff &&\n>>> + printf \"printf \\\"\\$GIT_PREFIX\\\"\\n\" >>diff &&\n>>> + chmod +x diff &&\n>>> + (\n>>> + cd dir &&\n>>> + printf \"change\" >two &&\n>>> + env GIT_EXTERNAL_DIFF=./diff git diff >../actual\n>>\n>> In passsing, this also tests the fact that GIT_EXTERNAL_DIFF is relative\n>> to the repo root and not to cwd...\n> \n> That's true. Another thing is that this only affects built-ins. I wanted to set the variable for git-foo external programs too but that means adding a call to setup_git_directory_gently() which we currently do not do in that codepath. I guess external scripts can call rev-parse --show-prefix themselves? Or is this worth making consistent?\n> \n>>\n>>> + git checkout -- two\n>>> + ) &&\n>>> + test_cmp expect actual\n>>> +'\n>>> +\n>>> test_expect_success 'no file/rev ambiguity check inside .git' '\n>>> git commit -a -m 1 &&\n>>> (\n>>\n>> Overall I think it's a good change, btw. But it leaves it up to the\n>> (script) user to know whether git has actually changed the cwd or not,\n>> i.e.: Is $(pwd) where the user called us from or $(pwd)/$GIT_PREFIX?\n>> That may may be a non-issue, though.\n>>\n>> Michael\n> \n> It's a non-issue for my use case but I can see it being confusing.\n> \n> For example, mergetool--lib's merge mode codepath can be run from subdirectories. The diff mode codepaths all assume that we are at the root (because git diff put us there).\n> \n> Thanks (and please let me know if my unsetenv analysis is incorrect),\n\nOur run_command() would convert \"GIT_PREFIX\" into an unsetenv(), but it\nleaves \"GIT_PREFIX=\" to be a putenv(). E.g., the alias\n\nenv = !sh -c 'set -u && echo $GIT_PREFIX'\n\ngives an empty result but errors out when you misspell GIT_PREFIX\nintentionally.\n\nI don't mind either way. \"set -u\" is a good script checker, and I seem\nto remember that on Windows, we do something extra do keep the\ndistinction between unset and null. Can't find it right now.\n\nMichael\n"},{"id":"168495","messageId":"BANLkTi=ssDA=y1CnMAZtvk6dTyMmd4LjrQ@mail.gmail.com","threadId":"27350","inReplyTo":"1306058229-93800-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 1/3] setup: Provide GIT_PREFIX to built-ins","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2011-05-23T12:09:40Z","receivedAt":"2011-05-23T12:09:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, May 22, 2011 at 11:57, David Aguilar wrote:\n> +       printf \"#!/bin/sh\\n\" >diff &&\n> +       printf \"printf \\\"\\$GIT_PREFIX\\\"\\n\" >>diff &&\n\nIf you're going to use /bin/sh (which might be a non-POSIX shell) it's\nprobably better to use echo than rely on printf understanding \\n.\n"},{"id":"168507","messageId":"7v8vtxweoh.fsf@alter.siamese.dyndns.org","threadId":"27350","inReplyTo":"FE7878D1-20E4-4CD4-B3FB-96322AA75855@gmail.com","subject":"Re: [PATCH 1/3] setup: Provide GIT_PREFIX to built-ins","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-23T16:43:26Z","receivedAt":"2011-05-23T16:43:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> I guess external scripts can call rev-parse --show-prefix themselves?\n\nThat has always been the case, I think, and it shouldn't be a problem.\n\nThe real reason you want the new GIT_PREFIX for alias/hooks is otherwise\nthey would not have a way to even say --show-prefix to figure it out\nthemselves.\n\n>> Overall I think it's a good change, btw. But it leaves it up to the\n>> (script) user to know whether git has actually changed the cwd or not,\n>> i.e.: Is $(pwd) where the user called us from or $(pwd)/$GIT_PREFIX?\n\nAs long as there is a way for a script to figure it out when it wants to\nknow, I think it should be Ok.\n\nIsn't it just the matter of reading --show-prefix and comparing it with\nwhat came in $GIT_PREFIX?\n"},{"id":"168519","messageId":"7vipt1ur15.fsf@alter.siamese.dyndns.org","threadId":"27350","inReplyTo":"1306058229-93800-3-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 3/3] git-mergetool--lib: Make vimdiff retain the current directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-23T19:59:34Z","receivedAt":"2011-05-23T19:59:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> +resolve_git_prefix() {\n> +\t# If GIT_PREFIX is empty then we cannot use it in tools\n> +\t# that expect to be able to chdir() to its value.\n> +\tif test -z \"$GIT_PREFIX\"; then\n> +\t\tGIT_PREFIX=.\n> +\t\texport GIT_PREFIX\n> +\tfi\n> +}\n\nDoes this \"export\" have to be conditional? Otherwise, it would be simpler\nto do this upfront at the beginning:\n\n : GIT_PREFIX=${GIT_PREFIX:-.}\n"},{"id":"168572","messageId":"4DDB5CFE.4090409@drmicha.warpmail.net","threadId":"27350","inReplyTo":"7v8vtxweoh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] setup: Provide GIT_PREFIX to built-ins","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-24T07:23:42Z","receivedAt":"2011-05-24T07:23:42Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 23.05.2011 18:43:\n> David Aguilar writes:\n> \n>> I guess external scripts can call rev-parse --show-prefix themselves?\n> \n> That has always been the case, I think, and it shouldn't be a problem.\n> \n> The real reason you want the new GIT_PREFIX for alias/hooks is otherwise\n> they would not have a way to even say --show-prefix to figure it out\n> themselves.\n> \n>>> Overall I think it's a good change, btw. But it leaves it up to the\n>>> (script) user to know whether git has actually changed the cwd or not,\n>>> i.e.: Is $(pwd) where the user called us from or $(pwd)/$GIT_PREFIX?\n> \n> As long as there is a way for a script to figure it out when it wants to\n> know, I think it should be Ok.\n> \n> Isn't it just the matter of reading --show-prefix and comparing it with\n> what came in $GIT_PREFIX?\n\nYep, one is before and one is after any eventual cd'ing which git may\ndo. I just wanted to point out the difference. And the technical\ndifference (env var. vs. rev-parse option) is due to that difference\n(and thus natural).\n\nMichael\n"},{"id":"168626","messageId":"20110525041926.GB21810@gmail.com","threadId":"27350","inReplyTo":"BANLkTi=ssDA=y1CnMAZtvk6dTyMmd4LjrQ@mail.gmail.com","subject":"Re: [PATCH 1/3] setup: Provide GIT_PREFIX to built-ins","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-05-25T04:19:28Z","receivedAt":"2011-05-25T04:19:28Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Mon, May 23, 2011 at 02:09:40PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> On Sun, May 22, 2011 at 11:57, David Aguilar wrote:\n> > +       printf \"#!/bin/sh\\n\" >diff &&\n> > +       printf \"printf \\\"\\$GIT_PREFIX\\\"\\n\" >>diff &&\n> \n> If you're going to use /bin/sh (which might be a non-POSIX shell) it's\n> probably better to use echo than rely on printf understanding \\n.\n\nI'll reroll a v2 of these patches using echo instead of printf.\nThe mergetool--lib patch will make the test -z \"$GIT_PREFIX\"\ncheck happen unconditionally as you suggested, Junio.\n\nAnother thought was that I could have implemented the\nmergetool--lib patch without $GIT_PREFIX at all and just called\nrev-parse --show-prefix explicitly. The change has merits,\nthough, and I'll consider mergetool--lib not using rev-parse\nas an optimization. Afterall, fork+exec is expensive on\nWindows so doing without an additional call is nicer for our\nmsysgit brothers.\n\nThank you both.\n-- \n\t\t\t\t\tDavid\n"}]}