{"thread":{"id":"45639","subject":"[PATCH] Make git log work for git CWD outside of work tree","startedAt":"2017-04-09T02:32:27Z","lastAt":"2017-04-17T10:30:35Z","messageCount":15,"participants":["Danny Sauer","Johannes Schindelin","Junio C Hamano","Duy Nguyen","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"316426","messageId":"20170409022128.21337-1-danny@dannysauer.com","threadId":"45639","inReplyTo":null,"subject":"[PATCH] Make git log work for git CWD outside of work tree","fromName":"Danny Sauer","fromEmail":"danny@dannysauer.com","sentAt":"2017-04-09T02:21:28Z","receivedAt":"2017-04-09T02:32:27Z","isPatch":true,"sender":{"key":"danny@dannysauer.com","avatar":null},"body":"Make git log's `--use-mailmap` argument works if the GIT_DIR & GIT_WORK_TREE\nenv vars are set and git is run from outside of work tree.  Without the\nNEED_WORK_TREE set on the log subcommand, .mailmap is silently not found.\n\nSigned-off-by: Danny Sauer <danny@dannysauer.com>\n---\n\nNotes:\n    I'm not entirely sure if this is the best way to fix it, as my git\n    internals knowledge is pretty weak. But this /seems/ reasonable to me, and\n    passes all of the current test cases.  If there's a more appropriate way to\n    make `--use-mailmap` work properly when `git log` is run outside of the\n    tree, I'd be excited about the opportunity to learn. :)\n\n git.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git.c b/git.c\nindex 8ff44f0..e147f01 100644\n--- a/git.c\n+++ b/git.c\n@@ -440,7 +440,7 @@ static struct cmd_struct commands[] = {\n \t{ \"init\", cmd_init_db },\n \t{ \"init-db\", cmd_init_db },\n \t{ \"interpret-trailers\", cmd_interpret_trailers, RUN_SETUP_GENTLY },\n-\t{ \"log\", cmd_log, RUN_SETUP },\n+\t{ \"log\", cmd_log, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"ls-files\", cmd_ls_files, RUN_SETUP | SUPPORT_SUPER_PREFIX },\n \t{ \"ls-remote\", cmd_ls_remote, RUN_SETUP_GENTLY },\n \t{ \"ls-tree\", cmd_ls_tree, RUN_SETUP },\n-- \n2.7.4\n\n"},{"id":"316433","messageId":"alpine.DEB.2.20.1704091238560.4268@virtualbox","threadId":"45639","inReplyTo":"20170409022128.21337-1-danny@dannysauer.com","subject":"Re: [PATCH] Make git log work for git CWD outside of work tree","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-04-09T10:54:46Z","receivedAt":"2017-04-09T10:55:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Danny,\n\nOn Sat, 8 Apr 2017, Danny Sauer wrote:\n\n> Make git log's `--use-mailmap` argument works if the GIT_DIR &\n> GIT_WORK_TREE env vars are set and git is run from outside of work tree.\n> Without the NEED_WORK_TREE set on the log subcommand, .mailmap is\n> silently not found.\n\nA laudable goal. How about adding a test case, say, to t/t4203-mailmap.sh,\nto ensure that Git won't regress on your fix?\n\n> diff --git a/git.c b/git.c\n> index 8ff44f0..e147f01 100644\n> --- a/git.c\n> +++ b/git.c\n> @@ -440,7 +440,7 @@ static struct cmd_struct commands[] = {\n>  \t{ \"init\", cmd_init_db },\n>  \t{ \"init-db\", cmd_init_db },\n>  \t{ \"interpret-trailers\", cmd_interpret_trailers, RUN_SETUP_GENTLY },\n> -\t{ \"log\", cmd_log, RUN_SETUP },\n> +\t{ \"log\", cmd_log, RUN_SETUP | NEED_WORK_TREE },\n>  \t{ \"ls-files\", cmd_ls_files, RUN_SETUP | SUPPORT_SUPER_PREFIX },\n>  \t{ \"ls-remote\", cmd_ls_remote, RUN_SETUP_GENTLY },\n>  \t{ \"ls-tree\", cmd_ls_tree, RUN_SETUP },\n\nThis may work for you, but it does not work for me, as I often call `git\nlog` in a bare repository. And that call works, and it should keep\nworking.\n\nInstead, I think, you need to figure out why the .mailmap file is not read\ncorrectly when you use the GIT_DIR & GIT_WORK_TREE approach. My vague\nhunch is that you need to replace the \".mailmap\" in read_mailmap()\n(defined in mailmap.c):\n\n        err |= read_mailmap_file(map, \".mailmap\", repo_abbrev);\n\nby something like mkpath(\"%s/%s\", get_git_work_tree(), \".mailmap\"), but\nprobably only after testing that we're not in a bare repository (which\nwould also fix a bug, I suspect, as `git log` in a bare repository\nprobably heeds a .mailmap file in the current directory, which is\nincorrect). I.e. something like:\n\n\tif (!is_bare_repository()) {\n\t\tconst char *path = mkpath(\"%s/%s\",\n\t\t\t\t\t  get_git_work_tree(), \".mailmap\")\n\t\terr |= read_mailmap_file(map, path, repo_abbrev);\n\t}\n\nBut you really want to add the test case first, with\n`test_expect_failure`, to demonstrate what is currently broken, and then\ntriumphantly setting it to `test_expect_success` with your patch.\n\nCiao,\nJohannes\n"},{"id":"316437","messageId":"413a1456-cac6-56c8-ea45-38f14cf958ae@dannysauer.com","threadId":"45639","inReplyTo":"alpine.DEB.2.20.1704091238560.4268@virtualbox","subject":"Re: [PATCH] Make git log work for git CWD outside of work tree","fromName":"Danny Sauer","fromEmail":"danny@dannysauer.com","sentAt":"2017-04-09T14:15:36Z","receivedAt":"2017-04-09T14:15:44Z","isPatch":true,"sender":{"key":"danny@dannysauer.com","avatar":null},"body":"On 4/9/2017 5:54 AM, Johannes Schindelin wrote:\n> Hi Danny,\n>\n> On Sat, 8 Apr 2017, Danny Sauer wrote:\n>> diff --git a/git.c b/git.c\n>> index 8ff44f0..e147f01 100644\n>> --- a/git.c\n>> +++ b/git.c\n>> @@ -440,7 +440,7 @@ static struct cmd_struct commands[] = {\n>>  \t{ \"init\", cmd_init_db },\n>>  \t{ \"init-db\", cmd_init_db },\n>>  \t{ \"interpret-trailers\", cmd_interpret_trailers, RUN_SETUP_GENTLY },\n>> -\t{ \"log\", cmd_log, RUN_SETUP },\n>> +\t{ \"log\", cmd_log, RUN_SETUP | NEED_WORK_TREE },\n>>  \t{ \"ls-files\", cmd_ls_files, RUN_SETUP | SUPPORT_SUPER_PREFIX },\n>>  \t{ \"ls-remote\", cmd_ls_remote, RUN_SETUP_GENTLY },\n>>  \t{ \"ls-tree\", cmd_ls_tree, RUN_SETUP },\n> This may work for you, but it does not work for me, as I often call `git\n> log` in a bare repository. And that call works, and it should keep\n> working.\n\nI was wondering about that situation, but honestly I wasn't positive how\nto test it and sort of just went on the hope that the read_mailmap_blob\nmechanism would continue handling it.  So, I guess there's another test\ncase which would be useful, as passing the test cases is what gave me\nthat undeserved confidence. :)  And I'm going to need to read a bit more\nto figure out what a bare repository actually is...\n\n> Instead, I think, you need to figure out why the .mailmap file is not read\n> correctly when you use the GIT_DIR & GIT_WORK_TREE approach. My vague\n> hunch is that you need to replace the \".mailmap\" in read_mailmap()\n> (defined in mailmap.c):\n>\n>         err |= read_mailmap_file(map, \".mailmap\", repo_abbrev);\n>\n> by something like mkpath(\"%s/%s\", get_git_work_tree(), \".mailmap\"), but\n> probably only after testing that we're not in a bare repository (which\n> would also fix a bug, I suspect, as `git log` in a bare repository\n> probably heeds a .mailmap file in the current directory, which is\n> incorrect). I.e. something like:\n>\n> \tif (!is_bare_repository()) {\n> \t\tconst char *path = mkpath(\"%s/%s\",\n> \t\t\t\t\t  get_git_work_tree(), \".mailmap\")\n> \t\terr |= read_mailmap_file(map, path, repo_abbrev);\n> \t}\n\nThat's feedback I was looking for - what's the \"right\" way to get the\npath to the file.  The call as-is works in any subdirectory of a\nrepository, because there's some logic buried along the way which puts\nyou in the top level of the repo if you started underneath.  But if\nyou're outside, it does the same chdir to verify that the top level is a\nreal repo and then returns you to where you started (which is perfectly\nreasonable).\n\nIn comparing how shortlog works v/s log, the main difference I could see\nis the use of RUN_SETUP_GENTLY instead of RUN_SETUP as the flag, which\nitself seems to only differ by passing in a true value for nongit_ok in\nrun_setup, ultimately leading to \"some different directory checks\" -\nwhich I somewhat got lost in due partially to not fully knowing what a\nbare repository is. :)  But changing that didn't make it work in my\n(poorly-formalized) testing.\n\nI'm on-board with a proper test case being the first situation which\nneeds rectifying.  It's off to learn stuff for me, then.  Thanks!\n\n--Danny\n\n"},{"id":"316453","messageId":"xmqqvaqdqfhe.fsf@gitster.mtv.corp.google.com","threadId":"45639","inReplyTo":"413a1456-cac6-56c8-ea45-38f14cf958ae@dannysauer.com","subject":"Re: [PATCH] Make git log work for git CWD outside of work tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-10T00:21:49Z","receivedAt":"2017-04-10T00:21:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Danny Sauer <danny@dannysauer.com> writes:\n\n> That's feedback I was looking for - what's the \"right\" way to get the\n> path to the file.\n\nMy knee-jerk reaction is that you SHOULDN'T have to do anything\nspecial to \"get the path\".\n\nI do not have the code in front of me (this is my \"down\" week after\nall), so let me give the general outline of how to reason about\nthis, with the knowledge of code evolution of the system.\n\nIn the beginning, Git worked only from the top level of the working\ntree.  If you have your project in ~/myproject with a file \"f\" in a\ndirectory \"d\", that file is ~/myproject/d/f\", and you would do\n\n    $ cd ~/myproject\n    $ git add d/f\n\nto add it.  If Git wanted to access the index, it just accessed\n\".git/index\" as a relative path.  If Git wanted to access a file at\nthe top of the working tree, say \".gitignore\", it just accessed\n\".gitignore\" as a relatlvei path.  Because it only worked from the\ntop.\n\nThen we added a support for running Git from a subdirectory, so that\nyou can say \"cd ~/myproject/d && git add f\".  In order to keep the\nexisting code that wants to access the index as \".git/index\" and\nwants to access the top-level \".gitignore\" as \".gitignore\" working,\nthe support to run Git from a subdirectory is designed this way:\n\n    - Each main program of subcommand (e.g. cmd_log()) receives a\n      parameter \"prefix\", which tells what subdirectory you were in\n      when you started Git.\n\n    - Before running the main program of subcommand, Git chdir(2)s\n      up to the top level of the working tree.\n\n    - The main program of subcommand receives the command line from\n      the user intact.  It is responsible for prepending the prefix\n      to the path the user gave it from the command line.\n\nSo if you did \"cd ~/myproject/d && git add f\", Git goes up to\n\"~/myproject\", passes argv=(\"f\", NULL) and prefix=\"d/\" to cmd_add().\n\nAdding to the index wants to read and update the index, which is\nstill done by opening \".git/index\" (relative to the toplevel as\nbefore), and inspecting the top-level .gitigore file is done by\nopening \".gitignore\" (relative to the toplevel as before).  And\nthe cmd_add() forms the path \"d/f\" by using the prefix \"d/\" and the\nuser-supplied pathname \"f\".\n\nWhen we first added a support for having the (equivalent of) \".git/\"\ndirectory outside the working tree by setting GIT_DIR environment\nvariable, again, you can only use Git from the top-level of the\nworking tree.  Instead of \"~/myproject/.git\", you can keep your\nrepository metainformation in say \"~/mypro.git/\" and point at it\nwith GIT_DIR environment variable, and said\n\n    $ export GIT_DIR=~/mypro.git\n    $ cd ~/myproject\n    $ git add d/f\n\nLater we also added GIT_WORK_TREE environment variable to be used\ntogether with GIT_DIR so that you can start from ~/myproject/d, very\nsimilarly to how you worked from a subdirectory without these\nenvironment variables, i.e.\n\n    $ export GIT_DIR=~/mypro.git GIT_WORK_TREE=~/myproject\n    $ cd ~/myproject/d\n    $ git add f\n\nThe way this support was added was to go to the top-level of the\nworking tree (i.e. \"~/myproject\" in this example) and passing the\nprefix (i.e. \"d\" in this example).\n\nNotice that in all of the above configurations, if a Git command\nknows a path to something that is relative to the top of the working\ntree (e.g. \".git/index\" in the ancient Git, \".gitignore\" at the\ntop-level that governs the entire working tree, or \".mailmap\"), it\nshould just be able to open that path without asking \"where is the\ntop of the working tree?\".\n\nSo if your directory arrangement is a variation of these basic\nconfigurations supported, e.g. if your git directory is\n~/myproject/.git and your working tree is ~/myproject, and you use\nthe GIT_DIR and GIT_WORK_TREE to point at them, regardless of which\nsubdirectory of $GIT_WORK_TREE you started with, Git should already\nhave done chdir(2) to ~/myproject/ before it starts cmd_log(), and\nit should be able to just open and read \".mailmap\" (of course, it\nneeds to limit itself to do so only when it is working with a\nrepository with a working tree).\n\nIf your arrangement is even more exotic, e.g. you have these two\nvariables set, and then are running from OUTSIDE the working tree,\nmy knee-jerk reaction is that you should get your head examined, as\nit is totally unclear what \"git add foo\" would mean if you did this:\n\n    $ export GIT_DIR=~/myproject/.git GIT_WORK_TREE=~/myproject\n    $ cd ~/myproject/../somewhere/else\n    $ git add foo\n\nBut it should still \"work\" in the sense that the above command\nshould notice that you are trying to add \"../somewhere/else/foo\" to\nthe index, which is obviously nonsense, and die with a message.\n\nSimilarly, if you replace \"git add foo\" with \"git log\", it still\nshould work in the above, i.e.\n\n    $ export GIT_DIR=~/myproject/.git GIT_WORK_TREE=~/myproject\n    $ cd ~/myproject/../somewhere/else\n    $ git log\n\nIf Git is not chdir(2)ing to ~/myproject before calling cmd_log()\nin the above (again, this is my down week so I didn't and will not\ncheck with the code myself), we may want to call that a bug and fix\nit, so that you do not have to do anything special to get to the\npath of \".mailmap\" that is at the top-level.\n\n"},{"id":"316480","messageId":"CACsJy8CLBY22j3EjR4PW3n+K6PWUzb-HCgxTVeCGpwtApZF-6g@mail.gmail.com","threadId":"45639","inReplyTo":"xmqqvaqdqfhe.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Make git log work for git CWD outside of work tree","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-10T12:01:00Z","receivedAt":"2017-04-10T12:01:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Apr 10, 2017 at 7:21 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> If your arrangement is even more exotic, e.g. you have these two\n> variables set, and then are running from OUTSIDE the working tree,\n> my knee-jerk reaction is that you should get your head examined, as\n> it is totally unclear what \"git add foo\" would mean if you did this:\n>\n>     $ export GIT_DIR=~/myproject/.git GIT_WORK_TREE=~/myproject\n>     $ cd ~/myproject/../somewhere/else\n>     $ git add foo\n>\n> But it should still \"work\" in the sense that the above command\n> should notice that you are trying to add \"../somewhere/else/foo\" to\n> the index, which is obviously nonsense, and die with a message.\n>\n> Similarly, if you replace \"git add foo\" with \"git log\", it still\n> should work in the above, i.e.\n>\n>     $ export GIT_DIR=~/myproject/.git GIT_WORK_TREE=~/myproject\n>     $ cd ~/myproject/../somewhere/else\n>     $ git log\n>\n> If Git is not chdir(2)ing to ~/myproject before calling cmd_log()\n> in the above (again, this is my down week so I didn't and will not\n> check with the code myself), we may want to call that a bug and fix\n> it, so that you do not have to do anything special to get to the\n> path of \".mailmap\" that is at the top-level.\n\nThe behavior is \"documented\" in t1510 since f3bb8b4b84 (Merge branch\n'nd/setup' - 2010-12-28)\n\n\"11. When user's cwd is outside worktree, cwd remains unchanged,\n    prefix is NULL.\"\n\nThis behavior probably started long before my topic though, mine was\nmore of documentation, making worktree detection more consistent. It's\nthe same case with defining GIT_DIR without GIT_WORK_TREE, I think:\nscripts started to depend on a behavior that we did not clearly\ndefine, by the time we knew what we wanted and we kept the old\nbehavior forever.\n\nI think it's just safer to go with Johannes' suggestion.\n\nAn alternative is, when you have found out you need to read .mailmap,\nyou call setup_work_tree() then, which prepares the worktree for you\n(including moving back to cwd) or dies if worktree does not exist, or\nno-op if worktree has already been asked by somebody. Many commands do\nlazy worktree initialization this way.\n-- \nDuy\n"},{"id":"316502","messageId":"20170410171352.s7r7tzheadxjlulw@sigill.intra.peff.net","threadId":"45639","inReplyTo":"CACsJy8CLBY22j3EjR4PW3n+K6PWUzb-HCgxTVeCGpwtApZF-6g@mail.gmail.com","subject":"Re: [PATCH] Make git log work for git CWD outside of work tree","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-10T17:13:53Z","receivedAt":"2017-04-10T17:14:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 10, 2017 at 07:01:00PM +0700, Duy Nguyen wrote:\n\n> > Similarly, if you replace \"git add foo\" with \"git log\", it still\n> > should work in the above, i.e.\n> >\n> >     $ export GIT_DIR=~/myproject/.git GIT_WORK_TREE=~/myproject\n> >     $ cd ~/myproject/../somewhere/else\n> >     $ git log\n> >\n> > If Git is not chdir(2)ing to ~/myproject before calling cmd_log()\n> > in the above (again, this is my down week so I didn't and will not\n> > check with the code myself), we may want to call that a bug and fix\n> > it, so that you do not have to do anything special to get to the\n> > path of \".mailmap\" that is at the top-level.\n> \n> The behavior is \"documented\" in t1510 since f3bb8b4b84 (Merge branch\n> 'nd/setup' - 2010-12-28)\n> \n> \"11. When user's cwd is outside worktree, cwd remains unchanged,\n>     prefix is NULL.\"\n> \n> This behavior probably started long before my topic though, mine was\n> more of documentation, making worktree detection more consistent. It's\n> the same case with defining GIT_DIR without GIT_WORK_TREE, I think:\n> scripts started to depend on a behavior that we did not clearly\n> define, by the time we knew what we wanted and we kept the old\n> behavior forever.\n> \n> I think it's just safer to go with Johannes' suggestion.\n> \n> An alternative is, when you have found out you need to read .mailmap,\n> you call setup_work_tree() then, which prepares the worktree for you\n> (including moving back to cwd) or dies if worktree does not exist, or\n> no-op if worktree has already been asked by somebody. Many commands do\n> lazy worktree initialization this way.\n\nI think this is much more than just .mailmap, though. For instance, I\nhave noticed a similar problem with .gitattributes:\n\n  # set up a repo with an attribute\n  git init repo\n  (\n    cd repo &&\n    echo content >file &&\n    echo \"file diff=sed\" >.gitattributes &&\n    git add . &&\n    git commit -m foo &&\n    git config diff.sed.textconv 'sed s/^/foo:/'\n  )\n\nIf I run:\n\n  (cd repo && git log -p)\n\nthe diff for \"file\" shows:\n\n  +foo:content\n\nas expected. But:\n\n  GIT_WORK_TREE=$PWD/repo GIT_DIR=$PWD/repo/.git git log -p\n\ndoesn't (while writing this I also noticed that \"git log -p file\"\ndoesn't work because of the DWIM lookup. That one is more debatable, but\nI still think it makes more sense to move to $GIT_WORK_TREE).\n\n-Peff\n"},{"id":"316670","messageId":"CACsJy8B8osxd-0axJ9giaMYKuict2h1zW8TcYYuRFGXfBbdF0A@mail.gmail.com","threadId":"45639","inReplyTo":"20170410171352.s7r7tzheadxjlulw@sigill.intra.peff.net","subject":"Re: [PATCH] Make git log work for git CWD outside of work tree","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-12T06:30:31Z","receivedAt":"2017-04-12T06:31:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Apr 11, 2017 at 12:13 AM, Jeff King <peff@peff.net> wrote:\n> On Mon, Apr 10, 2017 at 07:01:00PM +0700, Duy Nguyen wrote:\n>> An alternative is, when you have found out you need to read .mailmap,\n>> you call setup_work_tree() then, which prepares the worktree for you\n>> (including moving back to cwd) or dies if worktree does not exist, or\n>> no-op if worktree has already been asked by somebody. Many commands do\n>> lazy worktree initialization this way.\n>\n> I think this is much more than just .mailmap, though. For instance, I\n> have noticed a similar problem with .gitattributes:\n\nUrgh. assuming that we should not read .gitattributes if there's no\nworktree to read from (similar to the \"defaults to .git\" situation),\nhow about\n\n - if mailmap stuff is requested, setup worktree, or die trying\n - if worktree is detected, but setup code does not jump to it, do it\n - if no worktree is detected, tell git-log to stop reading .gitattributes\n\nWe probablly want some \"if no wotktree then die()\" in .gitattributes\nand .gitignore code, just in case it's incorrectly and accidentally\nexecuted in exotic setup\n-- \nDuy\n"},{"id":"316672","messageId":"xmqqy3v6ow54.fsf@gitster.mtv.corp.google.com","threadId":"45639","inReplyTo":"CACsJy8B8osxd-0axJ9giaMYKuict2h1zW8TcYYuRFGXfBbdF0A@mail.gmail.com","subject":"Re: [PATCH] Make git log work for git CWD outside of work tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-12T08:41:43Z","receivedAt":"2017-04-12T08:41:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n>> I think this is much more than just .mailmap, though. For instance, I\n>> have noticed a similar problem with .gitattributes:\n>\n> Urgh. assuming that we should not read .gitattributes if there's no\n> worktree to read from (similar to the \"defaults to .git\" situation),\n> how about\n>\n>  - if mailmap stuff is requested, setup worktree, or die trying\n>  - if worktree is detected, but setup code does not jump to it, do it\n>  - if no worktree is detected, tell git-log to stop reading .gitattributes\n\nMy gut reaction is that we are doing something wrong once we start\nsaying \"if mailmap stuff is requested\".  This is not about .mailmap\nbut is about how sanely the paths relative to the root of the\nworking tree (which includes a path in the index, or comparing\n$commit:$path with $path in the working tree) can be named by any\nsubcommand of Git.\n\nCan't we model this after how setup_git_directory_gently() gives the\nsubcommands a choice?  While the majority of subcommands do not call\nthe _gently() variant and die when we are not in a repository, the\nrest do use it and continue after learning that they are outside a\nrepository.  \n\nPerhaps we want a new bit GOTO_WORK_TREE_ROOT that is orthogonal to\nNEED_WORK_TREE to tell the codepath that calls cmd_foo() to always\nmove to the root of the working tree (if there is one), regaredless\nof the behaviour f3bb8b4b84 documents.  I have a strong suspicion\nthat we didn't _care_ about a silly use case where GIT_WORK_TREE is\nspecified and the command is started from somewhere completely\nunrelated to that location, and the users with such a silly use case\ndidn't care either what Git does to the files in the working tree,\ni.e. what you quoted in your previous message\n\n    \"11. When user's cwd is outside worktree, cwd remains unchanged,\n        prefix is NULL.\"\n\n    This behavior probably started long before my topic though, mine was\n    more of documentation, making worktree detection more consistent. It's\n\nwas not describing the design, but just describing whatever random\nthing the code happened to be doing.\n"},{"id":"316677","messageId":"CACsJy8DPFzgxqvPzpMbmoM4sMP7oSZ=eO6DJa+dv4sY=QKHjoA@mail.gmail.com","threadId":"45639","inReplyTo":"xmqqy3v6ow54.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Make git log work for git CWD outside of work tree","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-12T11:13:49Z","receivedAt":"2017-04-12T11:14:26Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 12, 2017 at 3:41 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Duy Nguyen <pclouds@gmail.com> writes:\n>\n>>> I think this is much more than just .mailmap, though. For instance, I\n>>> have noticed a similar problem with .gitattributes:\n>>\n>> Urgh. assuming that we should not read .gitattributes if there's no\n>> worktree to read from (similar to the \"defaults to .git\" situation),\n>> how about\n>>\n>>  - if mailmap stuff is requested, setup worktree, or die trying\n>>  - if worktree is detected, but setup code does not jump to it, do it\n>>  - if no worktree is detected, tell git-log to stop reading .gitattributes\n>\n> My gut reaction is that we are doing something wrong once we start\n> saying \"if mailmap stuff is requested\".  This is not about .mailmap\n> but is about how sanely the paths relative to the root of the\n> working tree (which includes a path in the index, or comparing\n> $commit:$path with $path in the working tree) can be named by any\n> subcommand of Git.\n\nI probably should phrased that as \"if any worktree stuff is\nrequested\". I was under an impression that you need to pass a command\nline option to use mailmap, but I dind't check the code\n\n> Can't we model this after how setup_git_directory_gently() gives the\n> subcommands a choice?  While the majority of subcommands do not call\n> the _gently() variant and die when we are not in a repository, the\n> rest do use it and continue after learning that they are outside a\n> repository.\n\nIt may work, but we need to be careful because paths as command line\narguments will not be relative to cwd anymore. If some code assumes\ncwd unchanged, they're in trouble. I guess they're in trouble either\nway because of the \"sometimes chdir, sometimes not\" current behavior.\n\n> Perhaps we want a new bit GOTO_WORK_TREE_ROOT that is orthogonal to\n> NEED_WORK_TREE to tell the codepath that calls cmd_foo() to always\n> move to the root of the working tree (if there is one), regaredless\n> of the behaviour f3bb8b4b84 documents.  I have a strong suspicion\n> that we didn't _care_ about a silly use case where GIT_WORK_TREE is\n> specified and the command is started from somewhere completely\n> unrelated to that location, and the users with such a silly use case\n> didn't care either what Git does to the files in the working tree,\n> i.e. what you quoted in your previous message\n>\n>     \"11. When user's cwd is outside worktree, cwd remains unchanged,\n>         prefix is NULL.\"\n>\n>     This behavior probably started long before my topic though, mine was\n>     more of documentation, making worktree detection more consistent. It's\n>\n> was not describing the design, but just describing whatever random\n> thing the code happened to be doing.\n\nI never said otherwise :) The only thing I was worried was how long it\nhad behaved that way, long enough that scripts started to depend on\nit? If we can get rid of special cases in setup code, I will be the\nfirst one to be happier.\n-- \nDuy\n"},{"id":"316682","messageId":"20170412125340.do7m2pjyignw6fjh@sigill.intra.peff.net","threadId":"45639","inReplyTo":"CACsJy8B8osxd-0axJ9giaMYKuict2h1zW8TcYYuRFGXfBbdF0A@mail.gmail.com","subject":"Re: [PATCH] Make git log work for git CWD outside of work tree","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-12T12:53:40Z","receivedAt":"2017-04-12T12:53:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 12, 2017 at 01:30:31PM +0700, Duy Nguyen wrote:\n\n> On Tue, Apr 11, 2017 at 12:13 AM, Jeff King <peff@peff.net> wrote:\n> > On Mon, Apr 10, 2017 at 07:01:00PM +0700, Duy Nguyen wrote:\n> >> An alternative is, when you have found out you need to read .mailmap,\n> >> you call setup_work_tree() then, which prepares the worktree for you\n> >> (including moving back to cwd) or dies if worktree does not exist, or\n> >> no-op if worktree has already been asked by somebody. Many commands do\n> >> lazy worktree initialization this way.\n> >\n> > I think this is much more than just .mailmap, though. For instance, I\n> > have noticed a similar problem with .gitattributes:\n> \n> Urgh. assuming that we should not read .gitattributes if there's no\n> worktree to read from (similar to the \"defaults to .git\" situation),\n> how about\n> \n>  - if mailmap stuff is requested, setup worktree, or die trying\n>  - if worktree is detected, but setup code does not jump to it, do it\n>  - if no worktree is detected, tell git-log to stop reading .gitattributes\n> \n> We probablly want some \"if no wotktree then die()\" in .gitattributes\n> and .gitignore code, just in case it's incorrectly and accidentally\n> executed in exotic setup\n\nI didn't check what we do with attributes when there is no worktree. The\nbehavior you describe above sounds right. But note that in my test we\n_do_ have a worktree, but just look in the wrong location. So doing a\nchdir to $GIT_WORK_TREE would just work.\n\n-Peff\n"},{"id":"316683","messageId":"20170412130145.jjnyait5234qsmys@sigill.intra.peff.net","threadId":"45639","inReplyTo":"CACsJy8DPFzgxqvPzpMbmoM4sMP7oSZ=eO6DJa+dv4sY=QKHjoA@mail.gmail.com","subject":"Re: [PATCH] Make git log work for git CWD outside of work tree","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-12T13:01:45Z","receivedAt":"2017-04-12T13:02:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 12, 2017 at 06:13:49PM +0700, Duy Nguyen wrote:\n\n> > Can't we model this after how setup_git_directory_gently() gives the\n> > subcommands a choice?  While the majority of subcommands do not call\n> > the _gently() variant and die when we are not in a repository, the\n> > rest do use it and continue after learning that they are outside a\n> > repository.\n> \n> It may work, but we need to be careful because paths as command line\n> arguments will not be relative to cwd anymore. If some code assumes\n> cwd unchanged, they're in trouble. I guess they're in trouble either\n> way because of the \"sometimes chdir, sometimes not\" current behavior.\n\nExactly. The code itself must respect \"prefix\", and if it doesn't, it's\nbroken. So I don't think a code switch makes any sense here. It's not\nthe code which needs to care, it's the user who might be relying on the\nexternally visible behavior.\n\nI can't think of a case where the existing behavior actually makes sense\nfor the user, though. They've provided a $GIT_WORK_TREE, but somehow\nwant their out-of-worktree relative paths to still work? What are they\nusing the paths for? Surely:\n\n  cd /out-of-tree\n  echo content >foo\n  GIT_DIR=/repo/.git GIT_WORK_TREE=/repo git add foo\n\nshouldn't work?\n\nLikewise reading .mailmap and .gitattributes from /out-of-tree is simply\na bug.\n\nI dunno. Maybe I am missing some subtle case, but it's not clear to me\nwhat the user would be trying to do by having git stay in the original\ndirectory.\n\n-Peff\n"},{"id":"316684","messageId":"CACsJy8BbHDsk4qLhVA=0QJFPutcGJpGwWrjs0_DiscDM9Q9+pw@mail.gmail.com","threadId":"45639","inReplyTo":"20170412130145.jjnyait5234qsmys@sigill.intra.peff.net","subject":"Re: [PATCH] Make git log work for git CWD outside of work tree","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-12T13:11:22Z","receivedAt":"2017-04-12T13:12:00Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 12, 2017 at 8:01 PM, Jeff King <peff@peff.net> wrote:\n> I dunno. Maybe I am missing some subtle case, but it's not clear to me\n> what the user would be trying to do by having git stay in the original\n> directory.\n\nNot sure if it really happens. But if we jump from worktree is inside\noriginal cwd and we have to jump to worktree, we set empty prefix, not\n\"../../../\" one. So we can't refer back to files relative to original\ncwd with prefix. I was kinda hoping \"super prefix\" would solve it, but\nthat one seems designed specifically for submodules.\n-- \nDuy\n"},{"id":"316788","messageId":"20170413212909.eb7zrhs7rzk6nxl6@sigill.intra.peff.net","threadId":"45639","inReplyTo":"CACsJy8BbHDsk4qLhVA=0QJFPutcGJpGwWrjs0_DiscDM9Q9+pw@mail.gmail.com","subject":"Re: [PATCH] Make git log work for git CWD outside of work tree","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-13T21:29:10Z","receivedAt":"2017-04-13T21:29:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 12, 2017 at 08:11:22PM +0700, Duy Nguyen wrote:\n\n> On Wed, Apr 12, 2017 at 8:01 PM, Jeff King <peff@peff.net> wrote:\n> > I dunno. Maybe I am missing some subtle case, but it's not clear to me\n> > what the user would be trying to do by having git stay in the original\n> > directory.\n> \n> Not sure if it really happens. But if we jump from worktree is inside\n> original cwd and we have to jump to worktree, we set empty prefix, not\n> \"../../../\" one. So we can't refer back to files relative to original\n> cwd with prefix. I was kinda hoping \"super prefix\" would solve it, but\n> that one seems designed specifically for submodules.\n\nAh, right. I think the issue is that \"prefix\" really serves two uses.\nFor things like command-line arguments, we use to find the original path\nafter we've moved. But we also use it for \"where in the working tree\nare we\".\n\nSo with an out-of-tree cwd, we'd want to set the first one to the real\npath (\"../../whatever\" or even just an absolute path), but the second\none would probably be empty (i.e., just pretend that we started from the\ntop-level).\n\nBut that would require first refactoring all of the cmd_* functions to\nhandle those prefixes separately.\n\n-Peff\n"},{"id":"316972","messageId":"xmqqlgqzlvbx.fsf@gitster.mtv.corp.google.com","threadId":"45639","inReplyTo":"20170413212909.eb7zrhs7rzk6nxl6@sigill.intra.peff.net","subject":"Re: [PATCH] Make git log work for git CWD outside of work tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-17T00:41:06Z","receivedAt":"2017-04-17T00:41:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> cwd with prefix. I was kinda hoping \"super prefix\" would solve it, but\n>> that one seems designed specifically for submodules.\n>\n> Ah, right. I think the issue is that \"prefix\" really serves two uses.\n> For things like command-line arguments, we use to find the original path\n> after we've moved. But we also use it for \"where in the working tree\n> are we\".\n>\n> So with an out-of-tree cwd, we'd want to set the first one to the real\n> path (\"../../whatever\" or even just an absolute path), but the second\n> one would probably be empty (i.e., just pretend that we started from the\n> top-level).\n>\n> But that would require first refactoring all of the cmd_* functions to\n> handle those prefixes separately.\n\nYes, I am still kinda hoping that the \"super prefix\" thing will\naddress the separation of the two uses, though.\n"},{"id":"317002","messageId":"CACsJy8ATXZZ27wHFHCUUoAbAjOzaaOUDrn=enSu8dsaCOk=fLQ@mail.gmail.com","threadId":"45639","inReplyTo":"20170413212909.eb7zrhs7rzk6nxl6@sigill.intra.peff.net","subject":"Re: [PATCH] Make git log work for git CWD outside of work tree","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-04-17T10:29:58Z","receivedAt":"2017-04-17T10:30:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Apr 14, 2017 at 4:29 AM, Jeff King <peff@peff.net> wrote:\n> On Wed, Apr 12, 2017 at 08:11:22PM +0700, Duy Nguyen wrote:\n>\n>> On Wed, Apr 12, 2017 at 8:01 PM, Jeff King <peff@peff.net> wrote:\n>> > I dunno. Maybe I am missing some subtle case, but it's not clear to me\n>> > what the user would be trying to do by having git stay in the original\n>> > directory.\n>>\n>> Not sure if it really happens. But if we jump from worktree is inside\n>> original cwd and we have to jump to worktree, we set empty prefix, not\n>> \"../../../\" one. So we can't refer back to files relative to original\n>> cwd with prefix. I was kinda hoping \"super prefix\" would solve it, but\n>> that one seems designed specifically for submodules.\n>\n> Ah, right. I think the issue is that \"prefix\" really serves two uses.\n> For things like command-line arguments, we use to find the original path\n> after we've moved. But we also use it for \"where in the working tree\n> are we\".\n>\n> So with an out-of-tree cwd, we'd want to set the first one to the real\n> path (\"../../whatever\" or even just an absolute path), but the second\n> one would probably be empty (i.e., just pretend that we started from the\n> top-level).\n>\n> But that would require first refactoring all of the cmd_* functions to\n> handle those prefixes separately.\n\nYeah. I probably shouldn't start another series until all others of\nmine settle. But if anyone is changing cmd_*, I suggest we take this\nopportunity to pass \"struct struct startup_info *\" in the as the only\noption. argv/argv could be stored there as well. And you can add\n\"cwd_prefix\" to present that \"../../whatever\" (I would avoid the name\n\"super prefix\" until things settle), or just keep the original \"cwd\"\nin there and let people do whatever they want with it.\n-- \nDuy\n"}]}