{"thread":{"id":"43852","subject":"[PATCH] rev-parse: respect core.hooksPath in --git-path","startedAt":"2016-08-15T12:43:46Z","lastAt":"2016-08-17T15:40:50Z","messageCount":11,"participants":["Johannes Schindelin","Jeff King","Remi Galan Alfonso","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"299337","messageId":"520a941f7472ac1cb4fa41e6bba33a0afc2f5999.1471264971.git.johannes.schindelin@gmx.de","threadId":"43852","inReplyTo":null,"subject":"[PATCH] rev-parse: respect core.hooksPath in --git-path","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-15T12:43:18Z","receivedAt":"2016-08-15T12:43:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The idea of the --git-path option is not only to avoid having to\nprefix paths with the output of --git-dir all the time, but also to\nrespect overrides for specific common paths inside the .git directory\n(e.g. `git rev-parse --git-path objects` will report the value of\nthe environment variable GIT_OBJECT_DIRECTORY, if set).\n\nWhen introducing the core.hooksPath setting, we forgot to adjust\ngit_path() accordingly. This patch fixes that.\n\nSigned-off-by: Johannes Schindelin \n---\nPublished-As: https://github.com/dscho/git/releases/tag/git-path-hooks-v1\nFetch-It-Via: git fetch https://github.com/dscho/git git-path-hooks-v1\n\n path.c | 2 ++\n t/t1350-config-hooks-path.sh | 6 ++++++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/path.c b/path.c\nindex 17551c4..fe3c4d9 100644\n--- a/path.c\n+++ b/path.c\n@@ -380,6 +380,8 @@ static void adjust_git_path(struct strbuf *buf, int git_dir_len)\n \t\t\t get_index_file(), strlen(get_index_file()));\n \telse if (git_db_env && dir_prefix(base, \"objects\"))\n \t\treplace_dir(buf, git_dir_len + 7, get_object_directory());\n+\telse if (git_hooks_path && dir_prefix(base, \"hooks\"))\n+\t\treplace_dir(buf, git_dir_len + 5, git_hooks_path);\n \telse if (git_common_dir_env)\n \t\tupdate_common_dir(buf, git_dir_len, NULL);\n }\ndiff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh\nindex 5e3fb3a..f1f9aee 100755\n--- a/t/t1350-config-hooks-path.sh\n+++ b/t/t1350-config-hooks-path.sh\n@@ -34,4 +34,10 @@ test_expect_success 'Check that various forms of specifying core.hooksPath work'\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git rev-parse --git-path hooks' '\n+\tgit config core.hooksPath .git/custom-hooks &&\n+\tgit rev-parse --git-path hooks/abc >actual &&\n+\ttest .git/custom-hooks/abc = \"$(cat actual)\"\n+'\n+\n test_done\n-- \n2.9.2.691.g78954f3\n\nbase-commit: 726cc2ba12c4573ab2e623077479c51019e1f3cd\n"},{"id":"299339","messageId":"20160815125006.qdssqgd4rzjw4vi5@sigill.intra.peff.net","threadId":"43852","inReplyTo":"520a941f7472ac1cb4fa41e6bba33a0afc2f5999.1471264971.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] rev-parse: respect core.hooksPath in --git-path","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-15T12:50:07Z","receivedAt":"2016-08-15T12:50:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 15, 2016 at 02:43:18PM +0200, Johannes Schindelin wrote:\n\n> The idea of the --git-path option is not only to avoid having to\n> prefix paths with the output of --git-dir all the time, but also to\n> respect overrides for specific common paths inside the .git directory\n> (e.g. `git rev-parse --git-path objects` will report the value of\n> the environment variable GIT_OBJECT_DIRECTORY, if set).\n> \n> When introducing the core.hooksPath setting, we forgot to adjust\n> git_path() accordingly. This patch fixes that.\n\nMakes sense.\n\nI think you can squash in:\n\ndiff --git a/run-command.c b/run-command.c\nindex 33bc63a..5a4dbb6 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -824,10 +824,7 @@ const char *find_hook(const char *name)\n \tstatic struct strbuf path = STRBUF_INIT;\n \n \tstrbuf_reset(&path);\n-\tif (git_hooks_path)\n-\t\tstrbuf_addf(&path, \"%s/%s\", git_hooks_path, name);\n-\telse\n-\t\tstrbuf_git_path(&path, \"hooks/%s\", name);\n+\tstrbuf_git_path(&path, \"hooks/%s\", name);\n \tif (access(path.buf, X_OK) < 0)\n \t\treturn NULL;\n \treturn path.buf;\n\nas strbuf_git_path() handles this now.\n\n-Peff\n"},{"id":"299449","messageId":"alpine.DEB.2.20.1608161440290.4924@virtualbox","threadId":"43852","inReplyTo":"20160815125006.qdssqgd4rzjw4vi5@sigill.intra.peff.net","subject":"Re: [PATCH] rev-parse: respect core.hooksPath in --git-path","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-16T12:42:34Z","receivedAt":"2016-08-16T12:42:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Mon, 15 Aug 2016, Jeff King wrote:\n\n> On Mon, Aug 15, 2016 at 02:43:18PM +0200, Johannes Schindelin wrote:\n> \n> > The idea of the --git-path option is not only to avoid having to\n> > prefix paths with the output of --git-dir all the time, but also to\n> > respect overrides for specific common paths inside the .git directory\n> > (e.g. `git rev-parse --git-path objects` will report the value of\n> > the environment variable GIT_OBJECT_DIRECTORY, if set).\n> > \n> > When introducing the core.hooksPath setting, we forgot to adjust\n> > git_path() accordingly. This patch fixes that.\n> \n> Makes sense.\n> \n> I think you can squash in:\n> \n> diff --git a/run-command.c b/run-command.c\n> index 33bc63a..5a4dbb6 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -824,10 +824,7 @@ const char *find_hook(const char *name)\n> \tstatic struct strbuf path = STRBUF_INIT;\n> \n> \tstrbuf_reset(&path);\n> -\tif (git_hooks_path)\n> -\t\tstrbuf_addf(&path, \"%s/%s\", git_hooks_path, name);\n> -\telse\n> -\t\tstrbuf_git_path(&path, \"hooks/%s\", name);\n> +\tstrbuf_git_path(&path, \"hooks/%s\", name);\n> \tif (access(path.buf, X_OK) < 0)\n> \t\treturn NULL;\n> \treturn path.buf;\n> \n> as strbuf_git_path() handles this now.\n\nGood point.\n\nBTW in light of the discussion we are having elsewhere I just need to\npoint out that it was *dramatically* faster for me to edit run-command.c,\nfind \"hooks/\" and adjust the code manually than it would have been to save\nthe diff and apply it.\n\nThat's because I do not have advanced tooling on top of email (and I also\ncould not handle mutt, so I am stuck with a not-really-scriptable email\nclient).\n\nJust sayin'.\n\nCiao,\nDscho\n"},{"id":"299450","messageId":"20160816130648.rfkebvmmk2uwfle2@sigill.intra.peff.net","threadId":"43852","inReplyTo":"alpine.DEB.2.20.1608161440290.4924@virtualbox","subject":"Re: [PATCH] rev-parse: respect core.hooksPath in --git-path","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-16T13:06:48Z","receivedAt":"2016-08-16T13:09:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 16, 2016 at 02:42:34PM +0200, Johannes Schindelin wrote:\n\n> > I think you can squash in:\n> > \n> > diff --git a/run-command.c b/run-command.c\n> [...]\n> \n> Good point.\n> \n> BTW in light of the discussion we are having elsewhere I just need to\n> point out that it was *dramatically* faster for me to edit run-command.c,\n> find \"hooks/\" and adjust the code manually than it would have been to save\n> the diff and apply it.\n> \n> That's because I do not have advanced tooling on top of email (and I also\n> could not handle mutt, so I am stuck with a not-really-scriptable email\n> client).\n> \n> Just sayin'.\n\nThere's a flip side, too. It was faster for me to paste in the diff than\nit would have been to create a branch, push it to GitHub, and make a PR\non your PR (and then later remember to deal with my PR and branch so as\nnot to leave them hanging around cluttering up my fork).\n\nI'm definitely sympathetic to people with less exacting email clients,\nbut I'm not convinced that the GitHub flow is that great if you don't do\nthe review there in the first place (and even when you do, it's actually\nnot that great for suggesting things in patch form).\n\nI wonder if public-inbox could have helped here. I think:\n\n mid=20160815125006.qdssqgd4rzjw4vi5@sigill.intra.peff.net\n curl http://public-inbox.org/git/$mid/raw | git apply\n\nwould work, but the question remains of how you find the right\nmessage-id. You can generally pick it out of the MUA's message headers\nmanually, though I think some MUAs even make seeing the extended headers\nhard. But that's going to be a similar problem with any solution that\nisn't your MUA: how do you feed the context of the discussion happening\nin one place to another tool.\n\n-Peff\n"},{"id":"299454","messageId":"ce8891377cec31cada49208f2d192dda86658e40.1471353054.git.johannes.schindelin@gmx.de","threadId":"43852","inReplyTo":"520a941f7472ac1cb4fa41e6bba33a0afc2f5999.1471264971.git.johannes.schindelin@gmx.de","subject":"[PATCH v2] rev-parse: respect core.hooksPath in --git-path","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-16T13:14:27Z","receivedAt":"2016-08-16T13:18:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The idea of the --git-path option is not only to avoid having to\nprefix paths with the output of --git-dir all the time, but also to\nrespect overrides for specific common paths inside the .git directory\n(e.g. `git rev-parse --git-path objects` will report the value of the\nenvironment variable GIT_OBJECT_DIRECTORY, if set).\n\nWhen introducing the core.hooksPath setting, we forgot to adjust\ngit_path() accordingly. This patch fixes that.\n\nWhile at it, revert the special-casing of core.hooksPath in\nrun-command.c, as it is now no longer needed.\n\nSigned-off-by: Johannes Schindelin \n---\nPublished-As: https://github.com/dscho/git/releases/tag/git-path-hooks-v2\nFetch-It-Via: git fetch https://github.com/dscho/git git-path-hooks-v2\nInterdiff vs v1:\n\n diff --git a/run-command.c b/run-command.c\n index 33bc63a..5a4dbb6 100644\n --- a/run-command.c\n +++ b/run-command.c\n @@ -824,10 +824,7 @@ const char *find_hook(const char *name)\n \tstatic struct strbuf path = STRBUF_INIT;\n \n \tstrbuf_reset(&path);\n -\tif (git_hooks_path)\n -\t\tstrbuf_addf(&path, \"%s/%s\", git_hooks_path, name);\n -\telse\n -\t\tstrbuf_git_path(&path, \"hooks/%s\", name);\n +\tstrbuf_git_path(&path, \"hooks/%s\", name);\n \tif (access(path.buf, X_OK) < 0)\n \t\treturn NULL;\n \treturn path.buf;\n\n\n path.c | 2 ++\n run-command.c | 5 +----\n t/t1350-config-hooks-path.sh | 6 ++++++\n 3 files changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex 17551c4..fe3c4d9 100644\n--- a/path.c\n+++ b/path.c\n@@ -380,6 +380,8 @@ static void adjust_git_path(struct strbuf *buf, int git_dir_len)\n \t\t\t get_index_file(), strlen(get_index_file()));\n \telse if (git_db_env && dir_prefix(base, \"objects\"))\n \t\treplace_dir(buf, git_dir_len + 7, get_object_directory());\n+\telse if (git_hooks_path && dir_prefix(base, \"hooks\"))\n+\t\treplace_dir(buf, git_dir_len + 5, git_hooks_path);\n \telse if (git_common_dir_env)\n \t\tupdate_common_dir(buf, git_dir_len, NULL);\n }\ndiff --git a/run-command.c b/run-command.c\nindex 33bc63a..5a4dbb6 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -824,10 +824,7 @@ const char *find_hook(const char *name)\n \tstatic struct strbuf path = STRBUF_INIT;\n \n \tstrbuf_reset(&path);\n-\tif (git_hooks_path)\n-\t\tstrbuf_addf(&path, \"%s/%s\", git_hooks_path, name);\n-\telse\n-\t\tstrbuf_git_path(&path, \"hooks/%s\", name);\n+\tstrbuf_git_path(&path, \"hooks/%s\", name);\n \tif (access(path.buf, X_OK) < 0)\n \t\treturn NULL;\n \treturn path.buf;\ndiff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh\nindex 5e3fb3a..f1f9aee 100755\n--- a/t/t1350-config-hooks-path.sh\n+++ b/t/t1350-config-hooks-path.sh\n@@ -34,4 +34,10 @@ test_expect_success 'Check that various forms of specifying core.hooksPath work'\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git rev-parse --git-path hooks' '\n+\tgit config core.hooksPath .git/custom-hooks &&\n+\tgit rev-parse --git-path hooks/abc >actual &&\n+\ttest .git/custom-hooks/abc = \"$(cat actual)\"\n+'\n+\n test_done\n-- \n2.9.2.691.g78954f3\n\nbase-commit: 07c92928f2b782330df6e78dd9d019e984d820a7\n"},{"id":"299460","messageId":"1134106438.1228353.1471361364992.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"43852","inReplyTo":"ce8891377cec31cada49208f2d192dda86658e40.1471353054.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2] rev-parse: respect core.hooksPath in --git-path","fromName":"Remi Galan Alfonso","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2016-08-16T15:29:24Z","receivedAt":"2016-08-16T15:10:43Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"Hi Johannes,\n\nJohannes Schindelin writes:\n> diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh\n> index 5e3fb3a..f1f9aee 100755\n> --- a/t/t1350-config-hooks-path.sh\n> +++ b/t/t1350-config-hooks-path.sh\n> @@ -34,4 +34,10 @@ test_expect_success 'Check that various forms of specifying core.hooksPath work'\n> test_cmp expect actual\n> '\n> \n> +test_expect_success 'git rev-parse --git-path hooks' '\n> + git config core.hooksPath .git/custom-hooks &&\n\nAny reason to not use 'test_config' here?\n\nThanks,\nRémi\n"},{"id":"299469","messageId":"alpine.DEB.2.20.1608161739280.4924@virtualbox","threadId":"43852","inReplyTo":"1134106438.1228353.1471361364992.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v2] rev-parse: respect core.hooksPath in --git-path","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-16T15:55:15Z","receivedAt":"2016-08-16T16:02:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Rémi,\n\nOn Tue, 16 Aug 2016, Remi Galan Alfonso wrote:\n\n> Johannes Schindelin writes:\n> > diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh\n> > index 5e3fb3a..f1f9aee 100755\n> > --- a/t/t1350-config-hooks-path.sh\n> > +++ b/t/t1350-config-hooks-path.sh\n> > @@ -34,4 +34,10 @@ test_expect_success 'Check that various forms of specifying core.hooksPath work'\n> > test_cmp expect actual\n> > '\n> > \n> > +test_expect_success 'git rev-parse --git-path hooks' '\n> > + git config core.hooksPath .git/custom-hooks &&\n> \n> Any reason to not use 'test_config' here?\n\nYes: consistency. The rest of the script uses `git config`, not\n`test_config`.\n\nCiao,\nDscho"},{"id":"299471","messageId":"1654103856.1228422.1471365329111.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"43852","inReplyTo":"alpine.DEB.2.20.1608161739280.4924@virtualbox","subject":"Re: [PATCH v2] rev-parse: respect core.hooksPath in --git-path","fromName":"Remi Galan Alfonso","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2016-08-16T16:35:29Z","receivedAt":"2016-08-16T16:16:37Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"Johannes Schindelin writes:\n> Hi Rémi,\n> \n> On Tue, 16 Aug 2016, Remi Galan Alfonso wrote:\n> \n> > Johannes Schindelin writes:\n> > > diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh\n> > > index 5e3fb3a..f1f9aee 100755\n> > > --- a/t/t1350-config-hooks-path.sh\n> > > +++ b/t/t1350-config-hooks-path.sh\n> > > @@ -34,4 +34,10 @@ test_expect_success 'Check that various forms of specifying core.hooksPath work'\n> > > test_cmp expect actual\n> > > '\n> > > \n> > > +test_expect_success 'git rev-parse --git-path hooks' '\n> > > + git config core.hooksPath .git/custom-hooks &&\n> >\n> > Any reason to not use 'test_config' here?\n> \n> Yes: consistency. The rest of the script uses `git config`, not\n> `test_config`.\n\nFine by me, then. Sorry for the noise.\n\n> Ciao,\n> Dscho\n\nCiao,\nRémi\n"},{"id":"299502","messageId":"xmqq4m6kcvc6.fsf@gitster.mtv.corp.google.com","threadId":"43852","inReplyTo":"1654103856.1228422.1471365329111.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v2] rev-parse: respect core.hooksPath in --git-path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-16T20:28:09Z","receivedAt":"2016-08-16T20:28:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Remi Galan Alfonso \nwrites:\n\n> Johannes Schindelin writes:\n>> Hi Rémi,\n>> \n>> On Tue, 16 Aug 2016, Remi Galan Alfonso wrote:\n>> \n>> > Johannes Schindelin writes:\n>> > > diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh\n>> > > index 5e3fb3a..f1f9aee 100755\n>> > > --- a/t/t1350-config-hooks-path.sh\n>> > > +++ b/t/t1350-config-hooks-path.sh\n>> > > @@ -34,4 +34,10 @@ test_expect_success 'Check that various forms of specifying core.hooksPath work'\n>> > > test_cmp expect actual\n>> > > '\n>> > > \n>> > > +test_expect_success 'git rev-parse --git-path hooks' '\n>> > > + git config core.hooksPath .git/custom-hooks &&\n>> >\n>> > Any reason to not use 'test_config' here?\n>> \n>> Yes: consistency. The rest of the script uses `git config`, not\n>> `test_config`.\n>\n> Fine by me, then. Sorry for the noise.\n\nNo, thanks for reviewing. I'll take Dscho's patch as-is but once it\nhits 'next', it probably is a good idea to do a separate clean-up\npatch on top to use test_config where necessary.\n\nHaving said that, this entire script is about constantly changing\nthe value of that single configuration variable and see how the code\nperforms, so any new test added after existing ones is expected to\nignore left-over values in the variable and set it to a value of its\nown liking. So I suspect there is no existing \"git config\" call in\nthis script, with or without Dscho's patch, that would benefit from\ngetting converted to test_config.\n\n\n"},{"id":"299532","messageId":"594839500.1229861.1471423409171.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"43852","inReplyTo":"xmqq4m6kcvc6.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] rev-parse: respect core.hooksPath in --git-path","fromName":"Remi Galan Alfonso","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2016-08-17T08:43:29Z","receivedAt":"2016-08-17T08:22:32Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"Junio C Hamano writes:\n> Remi Galan Alfonso \n> writes:\n>\n>> Johannes Schindelin writes:\n>>> Hi Rémi,\n>>>\n>>> On Tue, 16 Aug 2016, Remi Galan Alfonso wrote:\n>>>\n>>> > Johannes Schindelin writes:\n>>> > > diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh\n>>> > > index 5e3fb3a..f1f9aee 100755\n>>> > > --- a/t/t1350-config-hooks-path.sh\n>>> > > +++ b/t/t1350-config-hooks-path.sh\n>>> > > @@ -34,4 +34,10 @@ test_expect_success 'Check that various forms of specifying core.hooksPath work'\n>>> > > test_cmp expect actual\n>>> > > '\n>>> > > \n>>> > > +test_expect_success 'git rev-parse --git-path hooks' '\n>>> > > + git config core.hooksPath .git/custom-hooks &&\n>>> >\n>>> > Any reason to not use 'test_config' here?\n>>>\n>>> Yes: consistency. The rest of the script uses `git config`, not\n>>> `test_config`.\n>>\n>> Fine by me, then. Sorry for the noise.\n>\n> No, thanks for reviewing. I'll take Dscho's patch as-is but once it\n> hits 'next', it probably is a good idea to do a separate clean-up\n> patch on top to use test_config where necessary.\n>\n> Having said that, this entire script is about constantly changing\n> the value of that single configuration variable and see how the code\n> performs, so any new test added after existing ones is expected to\n> ignore left-over values in the variable and set it to a value of its\n> own liking. So I suspect there is no existing \"git config\" call in\n> this script, with or without Dscho's patch, that would benefit from\n> getting converted to test_config.\n\nThanks for checking the ones in this file, considering the lack\nof benefits it might not be worth it to change it for now.\n\nI tried to see if the `git config` in other tests were in the\nsame case or not but the sheer amount made me reconsider. However\ntaking a look at a couple of random ones, there are some scripts\nthat would benefit from the conversion.\nFor example in t3200-branch there is:\n\n test_expect_success 'git branch with column.*' '\n \tgit config column.ui column &&\n \tgit config column.branch \"dense\" &&\n \tCOLUMNS=80 git branch >actual &&\n \tgit config --unset column.branch &&\n \tgit config --unset column.ui &&\n \tcat >expected <<\\EOF &&\n a/b/c bam foo l * master n o/p r\n abc bar j/k m/m master2 o/o q\n EOF\n \ttest_cmp expected actual\n '\n\nThe conversion would drop 2 lines in this particular case and\nwould avoid bleeding the config should `git branch` fail (not\nsure how possible that is...).\n\n(I can make a patch for t3200 but that won't be before\ndays/weeks, so if someone else wants to take care of it I won't\nmind)\n\nIf I have some time to kill, I'll try looking at a few others but\nI won't expect that any time soon.\n\nThanks,\nRémi\n"},{"id":"299556","messageId":"xmqq1t1nbdza.fsf@gitster.mtv.corp.google.com","threadId":"43852","inReplyTo":"594839500.1229861.1471423409171.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v2] rev-parse: respect core.hooksPath in --git-path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-17T15:40:41Z","receivedAt":"2016-08-17T15:40:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Remi Galan Alfonso \nwrites:\n\n> I tried to see if the `git config` in other tests were in the\n> same case or not but the sheer amount made me reconsider. However\n> taking a look at a couple of random ones, there are some scripts\n> that would benefit from the conversion.\n\nYes, that is why I said \"it is a good idea to do this where\nnecessary, but the test Dscho touched here does not need it\".\nIt is a given that there are tons of \"git config\" users as\nwe have a lot more tests that were written before test_config\nwas invented.\n\nIt is good to occasionally modernise ancient tests; we usually do so\nwhen we need to make some other changes to them (e.g. I added a\nfeature, and wanted to add a couple of new tests, but I found it\nunreadable and unmaintainable because it kept the ancient style, so\nI am updating the existing tests to more modern style first before\ndoing my changes--and then do my changes on top of the cleaned up\ncodebase).\n\n"}]}