{"thread":{"id":"13522","subject":"[PATCH] Export GIT_DIR after setting it","startedAt":"2008-05-14T23:23:21Z","lastAt":"2008-05-20T16:17:00Z","messageCount":9,"participants":["martin f. krafft","Junio C Hamano","Björn Steinbrink"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"77002","messageId":"1210807401-11201-1-git-send-email-madduck@madduck.net","threadId":"13522","inReplyTo":null,"subject":"[PATCH] Export GIT_DIR after setting it","fromName":"martin f. krafft","fromEmail":"madduck@madduck.net","sentAt":"2008-05-14T23:23:21Z","receivedAt":"2008-05-14T23:23:21Z","isPatch":true,"sender":{"key":"madduck@madduck.net","avatar":null},"body":"git-sh-setup might set GIT_DIR, but not export it. When git-pull, for\ninstance, calls cd_to_toplevel, it changes the working directory, and later\ncalls git-ls-files, which does *not* inherit GIT_DIR since it's not imported.\nIt thus does the detection again, but in a different environment, since the\nworking directory changed. This breaks stuff subtly, especially when\ncore.worktree is set.\n\nThe patch simply exports GIT_DIR and makes it work such that git-ls-files\ndoesn't redo the work (wrongly).\n\nThanks to Björn Steinbrink for his help.\n\nSigned-off-by: martin f. krafft <madduck@madduck.net>\n---\n git-sh-setup.sh |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex a44b1c7..de90f07 100755\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -128,6 +128,7 @@ get_author_ident_from_commit () {\n if test -z \"$NONGIT_OK\"\n then\n \tGIT_DIR=$(git rev-parse --git-dir) || exit\n+\texport GIT_DIR\n \tif [ -z \"$SUBDIRECTORY_OK\" ]\n \tthen\n \t\ttest -z \"$(git rev-parse --show-cdup)\" || {\n-- \n1.5.5.1\n"},{"id":"77011","messageId":"7vod78i9r7.fsf@gitster.siamese.dyndns.org","threadId":"13522","inReplyTo":"1210807401-11201-1-git-send-email-madduck@madduck.net","subject":"Re: [PATCH] Export GIT_DIR after setting it","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-15T02:25:32Z","receivedAt":"2008-05-15T02:25:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"martin f. krafft\" <madduck@madduck.net> writes:\n\n> git-sh-setup might set GIT_DIR, but not export it. When git-pull, for\n> instance, calls cd_to_toplevel, it changes the working directory, and later\n> calls git-ls-files, which does *not* inherit GIT_DIR since it's not imported.\n\n\"Not exporting GIT_DIR\" was very much deliberately done when git-sh-setup\nwas introduced, and it is caller(includer)'s responsibility to export\nGIT_DIR when necessary.  Depending on callers, some did not want to export\nGIT_DIR, because exporting GIT_DIR means a bit more than that you are at\nthe toplevel of the tree (e.g. it tells the command not to do the usual\ndiscovery of .git directory) and they have places in their codepath they\ncannot cd up when running a git command internally, and/or they do not\nwant to cd up but they know what they run does GIT_DIR discovery on their\nown.  I do not recall which exact callers they were, though.  Do people\nrecall the details?\n\nMany scripted Porcelains were rewritten in C, and the need to be careful\nand selective about when to export and when not to export might have been\nremoved already in which case it would be Ok to solve whatever you are\ntrying to solve like this patch does, but this change needs very careful\nvetting to make sure that you did not break other scripts with this\nchange.\n\nThis arrangement predates separate work-tree by many months.  It could be\nthat what needs fixing is the separate work-tree code.  In any case, this\npatch is a bit worrying.\n"},{"id":"77038","messageId":"20080515101523.GA31719@lapse.madduck.net","threadId":"13522","inReplyTo":"7vod78i9r7.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Export GIT_DIR after setting it","fromName":"martin f. krafft","fromEmail":"madduck@madduck.net","sentAt":"2008-05-15T10:15:23Z","receivedAt":"2008-05-15T10:15:23Z","isPatch":true,"sender":{"key":"madduck@madduck.net","avatar":null},"body":"Thank you, Junio, for taking the time to reply to this!\n\nalso sprach Junio C Hamano <gitster@pobox.com> [2008.05.15.0325 +0100]:\n> trying to solve like this patch does, but this change needs very\n> careful vetting to make sure that you did not break other scripts\n> with this change.\n\nAbsolutely agreed. It occured to me as I lied down to sleep that\nthis fix could quite possibly have repercussions. And it's been in\nmy head all the walk to my work this morning. I ended up thinking\nabout it in this way:\n\nIf GIT_DIR is exported by git-sh-setup and we can assure that\ngit-sh-setup gets it right, then it's effectively the same as if the\nuser had set it explicitly, before calling the shell script: all\nexternal commands called by the shell script will have GIT_DIR set\nappropriately in all cases then.\n\nThe only problem I see now is when an external command (or the shell\nscript) can't properly deal with GIT_DIR being set, but then that's\na whole different bug.\n\nI understand you're worried about this, but I can't really see\nspecifics, now having thought about this for a bit.\n\n> This arrangement predates separate work-tree by many months.  It\n> could be that what needs fixing is the separate work-tree code.\n\nOh yeah, and I've been meaning to look into that for a long time.\nSigh.\n\n-- \nmartin | http://madduck.net/ | http://two.sentenc.es/\n \n\"she was rather too intelligent and competent-looking to be\n considered entirely beautiful, but all the more attractive because\n of it.\"\n                           -- george spencer-brown, \"a lion's teeth\"\n \nspamtraps: madduck.bogus@madduck.net\n"},{"id":"77055","messageId":"7vlk2bh45u.fsf@gitster.siamese.dyndns.org","threadId":"13522","inReplyTo":"20080515101523.GA31719@lapse.madduck.net","subject":"Re: [PATCH] Export GIT_DIR after setting it","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-15T17:23:57Z","receivedAt":"2008-05-15T17:23:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"martin f. krafft\" <madduck@madduck.net> writes:\n\n> The only problem I see now is when an external command (or the shell\n> script) can't properly deal with GIT_DIR being set, but then that's\n> a whole different bug.\n\nOne thing that we did not have to worry about when git-sh-setup was\ninvented is GIT_WORK_TREE and its cousin core.worktree.  When the user\nuses GIT_DIR _but_ wants to work from a subdirectory of the checked out\nwork tree, the user _must_ tell git where the top of the work tree is; in\nother words, setting and exporting only GIT_DIR is a misconfiguration.\n\nI have a suspicion that \"the whole different bug\" is what bit you --\nperhaps some places need to also set and export GIT_WORK_TREE as well when\nthe do GIT_DIR.\n"},{"id":"77058","messageId":"20080515175555.GA13003@atjola.homenet","threadId":"13522","inReplyTo":"7vlk2bh45u.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Export GIT_DIR after setting it","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2008-05-15T17:55:55Z","receivedAt":"2008-05-15T17:55:55Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2008.05.15 10:23:57 -0700, Junio C Hamano wrote:\n> \"martin f. krafft\" <madduck@madduck.net> writes:\n> \n> > The only problem I see now is when an external command (or the shell\n> > script) can't properly deal with GIT_DIR being set, but then that's\n> > a whole different bug.\n> \n> One thing that we did not have to worry about when git-sh-setup was\n> invented is GIT_WORK_TREE and its cousin core.worktree.  When the user\n> uses GIT_DIR _but_ wants to work from a subdirectory of the checked out\n> work tree, the user _must_ tell git where the top of the work tree is; in\n> other words, setting and exporting only GIT_DIR is a misconfiguration.\n> \n> I have a suspicion that \"the whole different bug\" is what bit you --\n> perhaps some places need to also set and export GIT_WORK_TREE as well when\n> the do GIT_DIR.\n\nFor completeness, here's an actual example of how it breaks:\ndoener@atjola:g $ git_fake_bare_checkout() {\n>                 url=\"$1\"\n>                 repo=\"$2\"\n>                 worktree=\"$3\"\n>                 git clone --no-checkout \"$url\" \"$repo\"\n>                 cd \"$repo\"\n>                 mkdir -p \"$worktree\"\n>                 git read-tree HEAD\n>                 git checkout-index -a --prefix=\"$worktree\" || true\n>                 git config core.worktree \"$worktree\"\n>                 mv .git/* .\n>                 rmdir .git\n>         }\ndoener@atjola:g $ git_fake_bare_checkout\ngit://git.madduck.net/etc/git.git git.git ../\nInitialized empty Git repository in /home/doener/g/git.git/.git/\nReceiving objects: 100% (6/6), done.\nremote: Counting objects: 6, done.\nremote: Compressing objects: 100% (4/4), done.\nremote: Total 6 (delta 0), reused 0 (delta 0)\ndoener@atjola:git.git (master) $ git fetch\ndoener@atjola:git.git (master) $ git pull\nfatal: Not a git repository\nfatal: Not a git repository\nfatal: Not a git repository\n\nSo the git directory is not called .git but git.git, with core.worktree\nset to \"../\". When \"git fetch\" is called directly, it correctly finds\nthat the git dir is \".\" Same for \"git pull\", but as GIT_DIR is neither\nset in the environment, nor exported by git-pull, the commands that get\nexecuted by git-pull do not find the git dir, because git-pull does\ncd_to_toplevel first, and obviously the other commands won't look for\ngit.git, but just .git.\n\nIt kind of feels like a bug that git-pull does not export GIT_DIR there,\nbut you could probably also argue that it is wrong not to have GIT_DIR\nset in the environment when using a non-standard name for the git dir.\nHm?\n\nBjörn\n"},{"id":"77059","messageId":"20080515182806.GA14799@lapse.madduck.net","threadId":"13522","inReplyTo":"20080515175555.GA13003@atjola.homenet","subject":"Re: [PATCH] Export GIT_DIR after setting it","fromName":"martin f. krafft","fromEmail":"madduck@madduck.net","sentAt":"2008-05-15T18:28:06Z","receivedAt":"2008-05-15T18:28:06Z","isPatch":true,"sender":{"key":"madduck@madduck.net","avatar":null},"body":"also sprach Björn Steinbrink <B.Steinbrink@gmx.de> [2008.05.15.1855 +0100]:\n> It kind of feels like a bug that git-pull does not export GIT_DIR there,\n> but you could probably also argue that it is wrong not to have GIT_DIR\n> set in the environment when using a non-standard name for the git dir.\n> Hm?\n\nAh, but it is a standard name: .\n\nIf git does not find .git, it *does* seem to look into the current\ndirectory too; that's why commands work inside bare repos...\n\n-- \nmartin | http://madduck.net/ | http://two.sentenc.es/\n \nclick the start menu and select 'shut down.'\n \nspamtraps: madduck.bogus@madduck.net\n"},{"id":"77060","messageId":"20080515184423.GA13535@atjola.homenet","threadId":"13522","inReplyTo":"20080515182806.GA14799@lapse.madduck.net","subject":"Re: [PATCH] Export GIT_DIR after setting it","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2008-05-15T18:44:23Z","receivedAt":"2008-05-15T18:44:23Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2008.05.15 19:28:06 +0100, martin f. krafft wrote:\n> also sprach Björn Steinbrink <B.Steinbrink@gmx.de> [2008.05.15.1855 +0100]:\n> > It kind of feels like a bug that git-pull does not export GIT_DIR there,\n> > but you could probably also argue that it is wrong not to have GIT_DIR\n> > set in the environment when using a non-standard name for the git dir.\n> > Hm?\n> \n> Ah, but it is a standard name: .\n> \n> If git does not find .git, it *does* seem to look into the current\n> directory too; that's why commands work inside bare repos...\n\nYeah, but this is not a bare repo. As far as I understood Junio, it is\nat least an error to use GIT_DIR without GIT_WORK_TREE (or\ncore.worktree), so I'm just thinking that it may also be an error to use\nGIT_WORK_TREE (or core.worktree) without GIT_DIR.\n\nBjörn\n"},{"id":"77160","messageId":"20080516215025.GA8250@lapse.madduck.net","threadId":"13522","inReplyTo":"7vlk2bh45u.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Export GIT_DIR after setting it","fromName":"martin f. krafft","fromEmail":"madduck@madduck.net","sentAt":"2008-05-16T21:50:25Z","receivedAt":"2008-05-16T21:50:25Z","isPatch":true,"sender":{"key":"madduck@madduck.net","avatar":null},"body":"also sprach Junio C Hamano <gitster@pobox.com> [2008.05.15.1823 +0100]:\n> I have a suspicion that \"the whole different bug\" is what bit you\n> -- perhaps some places need to also set and export GIT_WORK_TREE\n> as well when the do GIT_DIR.\n\nProbably also true, worktree support is still riddled with a lot of\nsmall little bugs... but I don't see how this would actually solve\nthe problem that caused me to write this patch...\n\n-- \nmartin | http://madduck.net/ | http://two.sentenc.es/\n \nwhatever you do will be insignificant,\nbut it is very important that you do it.\n                                                     -- mahatma gandhi\n \nspamtraps: madduck.bogus@madduck.net\n"},{"id":"77324","messageId":"20080520161700.GA16629@lapse.madduck.net","threadId":"13522","inReplyTo":"7vod78i9r7.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Export GIT_DIR after setting it","fromName":"martin f. krafft","fromEmail":"madduck@madduck.net","sentAt":"2008-05-20T16:17:00Z","receivedAt":"2008-05-20T16:17:00Z","isPatch":true,"sender":{"key":"madduck@madduck.net","avatar":null},"body":"also sprach Junio C Hamano <gitster@pobox.com> [2008.05.15.0325 +0100]:\n> In any case, this patch is a bit worrying.\n\nYour gut feeling is a good one!\n\nSee the following typescript. You'll notice that the new files\ncreated in wc2 pushed and pulled get merged into the first wc at the\nwrong location, and index and working dir get out of sync. This only\nhappens when I export GIT_DIR in git-sh-setup. Ouch.\n\nArguably, this is a bug in git-merge though!\n\n% GIT_DIR=repo.git git --bare init\nInitialized empty Git repository in repo.git/\n% mkdir wc && cd wc && git init\nInitialized empty Git repository in .git/\n% git remote add origin `pwd`/../repo.git\n% git config branch.master.remote origin\n% git config branch.master.merge refs/heads/master\n% touch a; git add a; git commit -m.\nCreated initial commit c80aa71: .\n 0 files changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 a\n% git push origin master\nCounting objects: 3, done.\nWriting objects: 100% (3/3), 196 bytes, done.\nTotal 3 (delta 0), reused 0 (delta 0)\nUnpacking objects: 100% (3/3), done.\nTo /home/madduck/.tmp/cdt.SLg15374/wc/../repo.git\n * [new branch]      master -> master\n% mkdir foo && touch foo/a && git add foo/a && git commit -m.\nCreated commit 8ccd80a: .\n 0 files changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 foo/a\n% git push\nCounting objects: 3, done.\nCompressing objects: 100% (2/2), done.\nUnpacking objects: 100% (2/2), done.\nWriting objects: 100% (2/2), 247 bytes, done.\nTotal 2 (delta 0), reused 0 (delta 0)\nTo /home/madduck/.tmp/cdt.SLg15374/wc/../repo.git\n   c80aa71..8ccd80a  master -> master\n% cd ../\n% git clone repo.git wc2\nInitialized empty Git repository in /home/madduck/.tmp/cdt.SLg15374/wc2/.git/\n% cd wc2\n% cd foo && mkdir bar && touch bar/a && git add bar/a && git commit -m.\nCreated commit cba76e8: .\n 0 files changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 foo/bar/a\n% git push\nCounting objects: 5, done.\nCompressing objects: 100% (3/3), done.\nUnpacking objects: 100% (3/3), done.\nWriting objects: 100% (3/3), 315 bytes, done.\nTotal 3 (delta 0), reused 0 (delta 0)\nTo /home/madduck/.tmp/cdt.SLg15374/repo.git\n   8ccd80a..cba76e8  master -> master\n% cd ../../wc/foo \n% ls\na\n% git pull\nremote: Counting objects: 5, done.\nremote: Compressing objects: 100% (3/3), done.\nremote: Total 3 (delta 0), reused 0 (delta 0)\nUnpacking objects: 100% (3/3), done.\nFrom /home/madduck/.tmp/cdt.SLg15374/wc/../repo\n   8ccd80a..cba76e8  master     -> origin/master\nUpdating 8ccd80a..cba76e8\nfoo/a: needs update\nFast forward\n 0 files changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 foo/bar/a\n% ls\na  foo\n% git status\n# On branch master\n# Changed but not updated:\n#   (use \"git add/rm <file>...\" to update what will be committed)\n#\n#       deleted:    bar/a\n#\n# Untracked files:\n#   (use \"git add <file>...\" to include in what will be committed)\n#\n#       foo/\nno changes added to commit (use \"git add\" and/or \"git commit -a\")\n% git diff\ndiff --git a/foo/bar/a b/foo/bar/a\ndeleted file mode 100644\nindex e69de29..0000000\n\n-- \nmartin | http://madduck.net/ | http://two.sentenc.es/\n \nmicro$oft windoze: proof that p. t. barnum was correct.\n \nspamtraps: madduck.bogus@madduck.net\n"}]}