{"thread":{"id":"47323","subject":"Problem with environment of hook execution when git is run with --work-tree / --git-dir","startedAt":"2017-11-26T07:19:54Z","lastAt":"2017-11-27T01:54:52Z","messageCount":3,"participants":["Michael Sloan","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"333521","messageId":"CAEDDsWdXQ1+UukvbfRoTPzY3Y9sOaxQ7nh+qL_Mcuy3=XKKh7w@mail.gmail.com","threadId":"47323","inReplyTo":null,"subject":"Problem with environment of hook execution when git is run with --work-tree / --git-dir","fromName":"Michael Sloan","fromEmail":"mgsloan@gmail.com","sentAt":"2017-11-26T07:19:25Z","receivedAt":"2017-11-26T07:19:54Z","isPatch":false,"sender":{"key":"mgsloan@gmail.com","avatar":null},"body":"Hi!\n\nI noticed a potential bug with the invocation of a pre-commit hook\nwhen running git with --work-tree and --git-dir.  In particular, I was\ninvestigating how hooks can still run git commands properly even when\nthe work-tree or git-dir is overridden via CLI args.  I put the\nfollowing in \"/home/mgsloan/.dotfiles-git/hooks/pre-commit\":\n\n    #!/bin/sh\n    env\n\nafter this, running \"git --work-tree=/home/mgsloan\n--git-dir=/home/mgsloan/.dotfiles-git commit\" has output with a bunch\nof variables, here are the important ones:\n\n    GIT_WORK_TREE=.\n    GIT_DIR=.dotfiles-git/\n    PWD=/home/mgsloan\n\nSo what's the problem with this choice of environment variables?\nWell, the problem is that if PWD is changed during the execution of\nthe script, then GIT_WORK_TREE and GIT_DIR will no longer work\nproperly. For example, if the pre-commit hook is\n\n    #!/bin/sh\n    cd some-dir\n    git status\n\nThis will fail with\n\n    Not a git repository: '.dotfiles-git'\n\nThere is another detail here, which is that when --git-dir /\n--work-tree is not specified, the no GIT_WORK_TREE / GIT_DIR\nenvironment variable is set.  This means that in this case, changing\nPWD in the hook will work fine as long as the search for .git will\nfind the right one.  Note that this also means that changing PWD in a\nscript can change which git repo the command is being run on, for\nexample, when the hook is interacting with a submodule.\n\nA half-fix to this would be to have the GIT_WORK_TREE and GIT_DIR set\nwhen running hooks use absolute paths.  However, this would not have\nthe same behavior as when git is used without --git-dir / --work-tree.\nAs described in the paragraph above, if PWD is relied upon to instead\ntarget a different git repo, then things break.\n\nNot sure what the total fix for this would be.   I think the\ninformation that needs to be conveyed to the hook's git invocations is\nthat \"the work-tree /home/mgsloan should be associated with the\ngit-dir /home/mgsloan/.dotfiles-git\".  Could have an env var like\n\n    GIT_DIR_MAPPINGS=\"/home/mgsloan!/home/mgsloan/.dotfiles-git\"\n\nThe idea is that this would be a list of mappings from GIT_WORK_TREE\nto GIT_DIR.  If this variable is set, then it will be followed when\ngit is searching parents of PWD for \".git\" directories.  I chose \"!\"\nrather arbitrarily here.  \"->\" would look nicer, but people might\nforget to escape it when programmatically setting this var.\n\nWhat do y'all think of this idea?\n\nSome of you might be wondering what I'm doing with my work tree being\nmy home directory.  It is the approach suggested here -\nhttps://developer.atlassian.com/blog/2016/02/best-way-to-store-dotfiles-git-bare-repo/\n- for versioning your configuration files directly.\n\nApologies if this has already been discussed, I could not find a good\nway to search the mailinglist archives.\n\nThanks!\n-Michael\n"},{"id":"333562","messageId":"xmqqtvxgfrlp.fsf@gitster.mtv.corp.google.com","threadId":"47323","inReplyTo":"CAEDDsWdXQ1+UukvbfRoTPzY3Y9sOaxQ7nh+qL_Mcuy3=XKKh7w@mail.gmail.com","subject":"Re: Problem with environment of hook execution when git is run with --work-tree / --git-dir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-27T01:16:50Z","receivedAt":"2017-11-27T01:16:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Sloan <mgsloan@gmail.com> writes:\n\n> So what's the problem with this choice of environment variables?\n> Well, the problem is that if PWD is changed during the execution of\n> the script, then GIT_WORK_TREE and GIT_DIR will no longer work\n> properly. For example, if the pre-commit hook is\n>\n>     #!/bin/sh\n>     cd some-dir\n>     git status\n>\n> This will fail with\n>\n>     Not a git repository: '.dotfiles-git'\n\nThat is to be expected.  It's up to the script to make them absolute\nif it cannot cope with relative paths.\n\n> There is another detail here, which is that when --git-dir /\n> --work-tree is not specified, the no GIT_WORK_TREE / GIT_DIR\n> environment variable is set.  This means that in this case, changing\n> PWD in the hook will work fine as long as the search for .git will\n> find the right one.\n\nThat also is working as designed.\n"},{"id":"333566","messageId":"CAEDDsWeLYGXn_ZeCAxGJjOUzfm_FZjZYWu9f=dAOAfe-2LcCZQ@mail.gmail.com","threadId":"47323","inReplyTo":"xmqqtvxgfrlp.fsf@gitster.mtv.corp.google.com","subject":"Re: Problem with environment of hook execution when git is run with --work-tree / --git-dir","fromName":"Michael Sloan","fromEmail":"mgsloan@gmail.com","sentAt":"2017-11-27T01:54:25Z","receivedAt":"2017-11-27T01:54:52Z","isPatch":false,"sender":{"key":"mgsloan@gmail.com","avatar":null},"body":"On Sun, Nov 26, 2017 at 5:16 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Michael Sloan <mgsloan@gmail.com> writes:\n>\n>> So what's the problem with this choice of environment variables?\n>> Well, the problem is that if PWD is changed during the execution of\n>> the script, then GIT_WORK_TREE and GIT_DIR will no longer work\n>> properly. For example, if the pre-commit hook is\n>>\n>>     #!/bin/sh\n>>     cd some-dir\n>>     git status\n>>\n>> This will fail with\n>>\n>>     Not a git repository: '.dotfiles-git'\n>\n> That is to be expected.  It's up to the script to make them absolute\n> if it cannot cope with relative paths.\n>\n>> There is another detail here, which is that when --git-dir /\n>> --work-tree is not specified, the no GIT_WORK_TREE / GIT_DIR\n>> environment variable is set.  This means that in this case, changing\n>> PWD in the hook will work fine as long as the search for .git will\n>> find the right one.\n>\n> That also is working as designed.\n\nHmm, I do not think that this is good for the reliability of hooks.\nIt means that hooks written with the common case assumption (no\nGIT_WORK_TREE / GIT_DIR set) will fail when run in these rarer\nconfigurations.  I imagine that many authors of hooks are entirely\nunaware of work-tree / git-dir options.  I know that I used git for\nsomething like 8 years before encountering a use for them, or really\nbeing aware.  Perhaps hooks authors are savvier than your average\nuser.\n\nIt seems to me like something similar to my suggested GIT_DIR_MAPPINGS\ncould be quite powerful for this circumstance as well as others.  I\nguess a temporary hack would be to create a \".git\" file that specifies\nthe git-dir, but this doesn't work if something is already called\n\".git\".\n\nI do not have a concrete example of this causing problems in practice,\nI've just observed that it is a potential gotcha for --git-dir +\nhooks.  I can understand keeping things simple even if it means making\nit much harder for hooks authors to write correct code.  It seems\nbothersome to me that git hooks have to be crafted so carefully to\nsupport variations in environment.\n\nIf we could go back to when hooks were introduced and add a\n\"GIT_IN_HOOK=1\" and have it require manual specification of\n--work-tree and -git-dir, that might have been the best option.\nHowever, doing that now would break everyone's hooks, so not really\npractical.\n\nAlso, thanks for all your work on git, overall it is excellent software :)\n\n-Michael\n"}]}