{"thread":{"id":"58481","subject":"fatal: options '--bare' and '--origin foo' cannot be used together","startedAt":"2022-09-21T19:33:16Z","lastAt":"2022-09-22T06:40:58Z","messageCount":11,"participants":["John A. Leuenhagen","Jeff King","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"463387","messageId":"20220921192333.yasrvnisccuk3ivg@Lima-X2","threadId":"58481","inReplyTo":null,"subject":"fatal: options '--bare' and '--origin foo' cannot be used together","fromName":"John A. Leuenhagen","fromEmail":"john@zlima12.com","sentAt":"2022-09-21T19:23:33Z","receivedAt":"2022-09-21T19:33:16Z","isPatch":false,"sender":{"key":"john@zlima12.com","avatar":null},"body":"Hi all,\n\nI often like to change the name of the remote that I clone repositories\nfrom, as \"origin\" is too generic for me when I have more than one\nremote.\n\nUsually, I only remember after I clone, and wind up doing `git remote\nrename origin foo`. I'd like to get in the habit of instead doing\n`git clone -o foo https://link.to/foo.git`. However, when I tried doing\nthis with --bare, I got the error that I've made the subject of this\nemail. Why can't I do this?\n\nWhen I try to do `git remote rename origin foo` in the bare repository\n(cloned without -o foo), I get a different error:\n\n>fatal: could not unset 'remote.foo.fetch'\n\nHowever, `git config remote.foo.fetch` indicates that this value is not\nset.\n\nWhy does this behavior differ from the equivalent commands on a non-bare\nrepository?\n-- \nJohn\n"},{"id":"463417","messageId":"Yyvq01W/r73xfgqf@coredump.intra.peff.net","threadId":"58481","inReplyTo":"20220921192333.yasrvnisccuk3ivg@Lima-X2","subject":"Re: fatal: options '--bare' and '--origin foo' cannot be used together","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-09-22T04:55:47Z","receivedAt":"2022-09-22T05:00:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 21, 2022 at 03:23:33PM -0400, John A. Leuenhagen wrote:\n\n> Usually, I only remember after I clone, and wind up doing `git remote\n> rename origin foo`. I'd like to get in the habit of instead doing\n> `git clone -o foo https://link.to/foo.git`. However, when I tried doing\n> this with --bare, I got the error that I've made the subject of this\n> email. Why can't I do this?\n\nI don't think there's a fundamental reason they cannot be mixed. In\nfact, you can do what you want with:\n\n git -c clone.defaultRemoteName=foo clone ...\n\n(which, btw, might give a better solution than remembering to use \"-o\"\neach time, since you can put that in your global git config).\n\nSo we are just complaining about the actual \"-o\" option. The conditional\nwhich catches that comes from 8434c2f1af (Build in clone, 2008-04-27),\nwhich is not much help. Before that, the same logic was in the shell\nversion, which was added in Junio's e6489a1bdf (clone: do not accept\nmore than one -o option., 2006-01-22), but there's no explanation given.\n\nI do think the remote name is less important in a bare clone, because we\ndon't create refs/remotes/foo/* there. But you might still care about\nthe name of the remote for doing \"git fetch foo\", etc. So I imagine we\ncould simply loosen the check like:\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex d269d6fec6..ed8d44bb6a 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -929,9 +929,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\toption_bare = 1;\n \n \tif (option_bare) {\n-\t\tif (option_origin)\n-\t\t\tdie(_(\"options '%s' and '%s %s' cannot be used together\"),\n-\t\t\t \"--bare\", \"--origin\", option_origin);\n \t\tif (real_git_dir)\n \t\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--bare\", \"--separate-git-dir\");\n \t\toption_no_checkout = 1;\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex f6bb02ab94..cf221e92c4 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -42,11 +42,12 @@ test_expect_success 'rejects invalid -o/--origin' '\n \n '\n \n-test_expect_success 'disallows --bare with --origin' '\n+test_expect_success 'clone --bare -o' '\n \n-\ttest_must_fail git clone -o foo --bare parent clone-bare-o 2>err &&\n-\ttest_debug \"cat err\" &&\n-\ttest_i18ngrep -e \"options .--bare. and .--origin foo. cannot be used together\" err\n+\tgit clone -o foo --bare parent clone-bare-o &&\n+\t(cd parent && pwd) >expect &&\n+\tgit -C clone-bare-o config remote.foo.url >actual &&\n+\ttest_cmp expect actual\n \n '\n \n\n> When I try to do `git remote rename origin foo` in the bare repository\n> (cloned without -o foo), I get a different error:\n> \n> >fatal: could not unset 'remote.foo.fetch'\n> \n> However, `git config remote.foo.fetch` indicates that this value is not\n> set.\n> \n> Why does this behavior differ from the equivalent commands on a non-bare\n> repository?\n\nThis second case is IMHO a minor bug in the git-remote code. As\ndocumented in git-clone(1):\n\n --bare::\n [...]\n Also the branch heads at the remote are copied directly\n to corresponding local branch heads, without mapping\n them to `refs/remotes/origin/`. When this option is\n used, neither remote-tracking branches nor the related\n configuration variables are created.\n\nSo it is expected that there is no configured refspec in a bare\nrepository. But it looks like the remote-renaming code is not prepared\nto handle this case. The backtrace for the error is:\n\n #0 die (err=0x5555558c349a \"could not unset '%s'\") at usage.c:175\n #1 0x00005555556bce85 in git_config_set_multivar_in_file (config_filename=0x5555559bc700 \"config\",\n key=0x5555559b5850 \"remote.foo.fetch\", value=0x0, value_pattern=0x0, flags=1) at config.c:3459\n #2 0x00005555556bcf77 in git_config_set_multivar (key=0x5555559b5850 \"remote.foo.fetch\", value=0x0,\n value_pattern=0x0, flags=1) at config.c:3485\n #3 0x000055555562bb59 in mv (argc=2, argv=0x7fffffffe550, prefix=0x0) at builtin/remote.c:738\n #4 0x000055555562f7b9 in cmd_remote (argc=3, argv=0x7fffffffe550, prefix=0x0) at builtin/remote.c:1772\n #5 0x0000555555574c53 in run_builtin (p=0x5555559760b0 , argc=4, argv=0x7fffffffe550) at git.c:466\n #6 0x000055555557506f in handle_builtin (argc=4, argv=0x7fffffffe550) at git.c:721\n #7 0x00005555555752dd in run_argv (argcp=0x7fffffffe3bc, argv=0x7fffffffe3b0) at git.c:788\n #8 0x0000555555575848 in cmd_main (argc=4, argv=0x7fffffffe550) at git.c:921\n #9 0x00005555556694c1 in main (argc=5, argv=0x7fffffffe548) at common-main.c:56\n\nSo probably it needs to use the \"gently\" version of git_config_set_multivar(),\nwhich avoids calling die(), and then check for a return code which\nindicates no value was found.\n\n-Peff\n"},{"id":"463418","messageId":"YyvzIQL9H9aTYNch@coredump.intra.peff.net","threadId":"58481","inReplyTo":"Yyvq01W/r73xfgqf@coredump.intra.peff.net","subject":"Re: fatal: options '--bare' and '--origin foo' cannot be used together","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-09-22T05:31:13Z","receivedAt":"2022-09-22T05:31:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 22, 2022 at 12:55:47AM -0400, Jeff King wrote:\n\n> This second case is IMHO a minor bug in the git-remote code. As\n> documented in git-clone(1):\n> [...]\n> So probably it needs to use the \"gently\" version of git_config_set_multivar(),\n> which avoids calling die(), and then check for a return code which\n> indicates no value was found.\n\nActually, there is an even simpler fix. So here are two cleaned-up\npatches which fix both issues you found.\n\n [1/2]: clone: allow \"--bare\" with \"-o\"\n [2/2]: remote: handle rename of remote without fetch refspec\n\n builtin/clone.c | 3 ---\n builtin/remote.c | 48 +++++++++++++++++++++-------------------\n t/t5505-remote.sh | 11 +++++++++\n t/t5606-clone-options.sh | 9 ++++----\n 4 files changed, 41 insertions(+), 30 deletions(-)\n\n-Peff\n"},{"id":"463419","messageId":"YyvzVdfQVdysvMp2@coredump.intra.peff.net","threadId":"58481","inReplyTo":"YyvzIQL9H9aTYNch@coredump.intra.peff.net","subject":"[PATCH 1/2] clone: allow \"--bare\" with \"-o\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-09-22T05:32:05Z","receivedAt":"2022-09-22T05:32:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We explicitly forbid the combination of \"--bare\" with \"-o\", but there\ndoesn't seem to be any good reason to do so. The original logic came as\npart of e6489a1bdf (clone: do not accept more than one -o option.,\n2006-01-22), but that commit does not give any reason.\n\nFurthermore, the equivalent combination via config is allowed:\n\n git -c clone.defaultRemoteName=foo clone ...\n\nand works as expected. It may be that this combination was considered\nuseless, because a bare clone does not set remote.origin.fetch (and\nhence there is no refs/remotes/origin hierarchy). But it does set\nremote.origin.url, and that name is visible to the user via \"git fetch\norigin\", etc.\n\nLet's allow the options to be used together, and switch the \"forbid\"\ntest in t5606 to check that we use the requested name. That test came\nmuch later in 349cff76de (clone: add tests for --template and some\ndisallowed option pairs, 2020-09-29), and does not offer any logic\nbeyond \"let's test what the code currently does\".\n\nReported-by: John A. Leuenhagen \nSigned-off-by: Jeff King \n---\n builtin/clone.c | 3 ---\n t/t5606-clone-options.sh | 9 +++++----\n 2 files changed, 5 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex d269d6fec6..ed8d44bb6a 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -929,9 +929,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\toption_bare = 1;\n \n \tif (option_bare) {\n-\t\tif (option_origin)\n-\t\t\tdie(_(\"options '%s' and '%s %s' cannot be used together\"),\n-\t\t\t \"--bare\", \"--origin\", option_origin);\n \t\tif (real_git_dir)\n \t\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--bare\", \"--separate-git-dir\");\n \t\toption_no_checkout = 1;\ndiff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\nindex f6bb02ab94..cf221e92c4 100755\n--- a/t/t5606-clone-options.sh\n+++ b/t/t5606-clone-options.sh\n@@ -42,11 +42,12 @@ test_expect_success 'rejects invalid -o/--origin' '\n \n '\n \n-test_expect_success 'disallows --bare with --origin' '\n+test_expect_success 'clone --bare -o' '\n \n-\ttest_must_fail git clone -o foo --bare parent clone-bare-o 2>err &&\n-\ttest_debug \"cat err\" &&\n-\ttest_i18ngrep -e \"options .--bare. and .--origin foo. cannot be used together\" err\n+\tgit clone -o foo --bare parent clone-bare-o &&\n+\t(cd parent && pwd) >expect &&\n+\tgit -C clone-bare-o config remote.foo.url >actual &&\n+\ttest_cmp expect actual\n \n '\n \n-- \n2.38.0.rc1.580.gfaa349ad1e\n\n"},{"id":"463420","messageId":"YyvzqZ5tmI0UdRAW@coredump.intra.peff.net","threadId":"58481","inReplyTo":"YyvzIQL9H9aTYNch@coredump.intra.peff.net","subject":"[PATCH 2/2] remote: handle rename of remote without fetch refspec","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-09-22T05:33:29Z","receivedAt":"2022-09-22T05:33:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We return an error when trying to rename a remote that has no fetch\nrefspec:\n\n $ git config --unset-all remote.origin.fetch\n $ git remote rename origin foo\n fatal: could not unset 'remote.foo.fetch'\n\nTo make things even more confusing, we actually _do_ complete the config\nmodification, via git_config_rename_section(). After that we try to\nrewrite the fetch refspec (to say refs/remotes/foo instead of origin).\nBut our call to git_config_set_multivar() to remove the existing entries\nfails, since there aren't any, and it calls die().\n\nWe could fix this by using the \"gently\" form of the config call, and\nchecking the error code. But there is an even simpler fix: if we know\nthat there are no refspecs to rewrite, then we can skip that part\nentirely.\n\nReported-by: John A. Leuenhagen \nSigned-off-by: Jeff King \n---\nThe diff is a bit noisy due to indentation, but with \"-w\" you can see\nthat it's just wrapping this part of the function in a conditional.\n\n builtin/remote.c | 48 ++++++++++++++++++++++++-----------------------\n t/t5505-remote.sh | 11 +++++++++++\n 2 files changed, 36 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 985b845a18..910f7b9316 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -733,29 +733,31 @@ static int mv(int argc, const char **argv, const char *prefix)\n \t\treturn error(_(\"Could not rename config section '%s' to '%s'\"),\n \t\t\t\tbuf.buf, buf2.buf);\n \n-\tstrbuf_reset(&buf);\n-\tstrbuf_addf(&buf, \"remote.%s.fetch\", rename.new_name);\n-\tgit_config_set_multivar(buf.buf, NULL, NULL, CONFIG_FLAGS_MULTI_REPLACE);\n-\tstrbuf_addf(&old_remote_context, \":refs/remotes/%s/\", rename.old_name);\n-\tfor (i = 0; i < oldremote->fetch.raw_nr; i++) {\n-\t\tchar *ptr;\n-\n-\t\tstrbuf_reset(&buf2);\n-\t\tstrbuf_addstr(&buf2, oldremote->fetch.raw[i]);\n-\t\tptr = strstr(buf2.buf, old_remote_context.buf);\n-\t\tif (ptr) {\n-\t\t\trefspec_updated = 1;\n-\t\t\tstrbuf_splice(&buf2,\n-\t\t\t\t ptr-buf2.buf + strlen(\":refs/remotes/\"),\n-\t\t\t\t strlen(rename.old_name), rename.new_name,\n-\t\t\t\t strlen(rename.new_name));\n-\t\t} else\n-\t\t\twarning(_(\"Not updating non-default fetch refspec\\n\"\n-\t\t\t\t \"\\t%s\\n\"\n-\t\t\t\t \"\\tPlease update the configuration manually if necessary.\"),\n-\t\t\t\tbuf2.buf);\n-\n-\t\tgit_config_set_multivar(buf.buf, buf2.buf, \"^$\", 0);\n+\tif (oldremote->fetch.raw_nr) {\n+\t\tstrbuf_reset(&buf);\n+\t\tstrbuf_addf(&buf, \"remote.%s.fetch\", rename.new_name);\n+\t\tgit_config_set_multivar(buf.buf, NULL, NULL, CONFIG_FLAGS_MULTI_REPLACE);\n+\t\tstrbuf_addf(&old_remote_context, \":refs/remotes/%s/\", rename.old_name);\n+\t\tfor (i = 0; i < oldremote->fetch.raw_nr; i++) {\n+\t\t\tchar *ptr;\n+\n+\t\t\tstrbuf_reset(&buf2);\n+\t\t\tstrbuf_addstr(&buf2, oldremote->fetch.raw[i]);\n+\t\t\tptr = strstr(buf2.buf, old_remote_context.buf);\n+\t\t\tif (ptr) {\n+\t\t\t\trefspec_updated = 1;\n+\t\t\t\tstrbuf_splice(&buf2,\n+\t\t\t\t\t ptr-buf2.buf + strlen(\":refs/remotes/\"),\n+\t\t\t\t\t strlen(rename.old_name), rename.new_name,\n+\t\t\t\t\t strlen(rename.new_name));\n+\t\t\t} else\n+\t\t\t\twarning(_(\"Not updating non-default fetch refspec\\n\"\n+\t\t\t\t\t \"\\t%s\\n\"\n+\t\t\t\t\t \"\\tPlease update the configuration manually if necessary.\"),\n+\t\t\t\t\tbuf2.buf);\n+\n+\t\t\tgit_config_set_multivar(buf.buf, buf2.buf, \"^$\", 0);\n+\t\t}\n \t}\n \n \tread_branches();\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 9006196ac6..43b7bcd715 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -902,6 +902,17 @@ test_expect_success 'rename a remote renames repo remote.pushDefault but keeps g\n \t)\n '\n \n+test_expect_success 'rename handles remote without fetch refspec' '\n+\tgit clone --bare one no-refspec.git &&\n+\t# confirm assumption that bare clone does not create refspec\n+\ttest_expect_code 5 \\\n+\t\tgit -C no-refspec.git config --unset-all remote.origin.fetch &&\n+\tgit -C no-refspec.git config remote.origin.url >expect &&\n+\tgit -C no-refspec.git remote rename origin foo &&\n+\tgit -C no-refspec.git config remote.foo.url >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'rename does not update a non-default fetch refspec' '\n \tgit clone one four.one &&\n \t(\n-- \n2.38.0.rc1.580.gfaa349ad1e\n"},{"id":"463421","messageId":"CAPig+cTEF=jBoW07_eLQ_BAWxULfWtehVa-JbJKpS+8rPenSiQ@mail.gmail.com","threadId":"58481","inReplyTo":"YyvzVdfQVdysvMp2@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] clone: allow \"--bare\" with \"-o\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-09-22T05:58:36Z","receivedAt":"2022-09-22T05:58:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Sep 22, 2022 at 1:33 AM Jeff King wrote:\n> [...]\n> Let's allow the options to be used together, and switch the \"forbid\"\n> test in t5606 to check that we use the requested name. That test came\n> much later in 349cff76de (clone: add tests for --template and some\n> disallowed option pairs, 2020-09-29), and does not offer any logic\n> beyond \"let's test what the code currently does\".\n>\n> Signed-off-by: Jeff King \n> ---\n> diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\n> @@ -42,11 +42,12 @@ test_expect_success 'rejects invalid -o/--origin' '\n> +test_expect_success 'clone --bare -o' '\n> + git clone -o foo --bare parent clone-bare-o &&\n> + (cd parent && pwd) >expect &&\n> + git -C clone-bare-o config remote.foo.url >actual &&\n> + test_cmp expect actual\n> '\n\nIs this safe on Microsoft Windows? My understanding from t/README:\n\n When a test checks for an absolute path that a git command\n generated, construct the expected value using $(pwd) rather than\n $PWD, $TEST_DIRECTORY, or $TRASH_DIRECTORY. It makes a difference\n on Windows, where the shell (MSYS bash) mangles absolute path\n names. For details, see the commit message of 4114156ae9.\n\nwas that you should use $(pwd) rather than raw `pwd` when comparing\nagainst a path generated by Git. Is there a gap in my understanding\nhere?\n"},{"id":"463422","messageId":"Yyv+GjjauvvcX3M1@coredump.intra.peff.net","threadId":"58481","inReplyTo":"CAPig+cTEF=jBoW07_eLQ_BAWxULfWtehVa-JbJKpS+8rPenSiQ@mail.gmail.com","subject":"Re: [PATCH 1/2] clone: allow \"--bare\" with \"-o\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-09-22T06:18:02Z","receivedAt":"2022-09-22T06:18:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 22, 2022 at 01:58:36AM -0400, Eric Sunshine wrote:\n\n> > diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh\n> > @@ -42,11 +42,12 @@ test_expect_success 'rejects invalid -o/--origin' '\n> > +test_expect_success 'clone --bare -o' '\n> > + git clone -o foo --bare parent clone-bare-o &&\n> > + (cd parent && pwd) >expect &&\n> > + git -C clone-bare-o config remote.foo.url >actual &&\n> > + test_cmp expect actual\n> > '\n> \n> Is this safe on Microsoft Windows? My understanding from t/README:\n> \n> When a test checks for an absolute path that a git command\n> generated, construct the expected value using $(pwd) rather than\n> $PWD, $TEST_DIRECTORY, or $TRASH_DIRECTORY. It makes a difference\n> on Windows, where the shell (MSYS bash) mangles absolute path\n> names. For details, see the commit message of 4114156ae9.\n> \n> was that you should use $(pwd) rather than raw `pwd` when comparing\n> against a path generated by Git. Is there a gap in my understanding\n> here?\n\nI think you might be mis-reading the advice here. It is saying to use\nthe \"pwd\" program, rather than relying on the shell's $PWD variable. So\n$(pwd) and `pwd` are the same thing (and are what I'm using). The $() I\nthink is just indicating that you'd do:\n\n foo=$(pwd)\n\nAnd yes, I think this is a case where using the right one is important\n(which is why I used the pwd program, and not $pwd in the test).\n\nOr am I missing something else?\n\n-Peff\n"},{"id":"463424","messageId":"CAPig+cQxSwz2cZFLNwg_rJ==M5=pxMVvwgbDFV1r=v+rzbCuNg@mail.gmail.com","threadId":"58481","inReplyTo":"Yyv+GjjauvvcX3M1@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] clone: allow \"--bare\" with \"-o\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-09-22T06:26:09Z","receivedAt":"2022-09-22T06:27:04Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Sep 22, 2022 at 2:18 AM Jeff King wrote:\n> On Thu, Sep 22, 2022 at 01:58:36AM -0400, Eric Sunshine wrote:\n> > > + (cd parent && pwd) >expect &&\n> > > + git -C clone-bare-o config remote.foo.url >actual &&\n> > > + test_cmp expect actual\n> >\n> > Is this safe on Microsoft Windows? My understanding from t/README:\n> >\n> > When a test checks for an absolute path that a git command\n> > generated, construct the expected value using $(pwd) rather than\n> > $PWD, $TEST_DIRECTORY, or $TRASH_DIRECTORY. It makes a difference\n> > on Windows, where the shell (MSYS bash) mangles absolute path\n> > names. For details, see the commit message of 4114156ae9.\n> >\n> > was that you should use $(pwd) rather than raw `pwd` when comparing\n> > against a path generated by Git. Is there a gap in my understanding\n> > here?\n>\n> I think you might be mis-reading the advice here. It is saying to use\n> the \"pwd\" program, rather than relying on the shell's $PWD variable. So\n> $(pwd) and `pwd` are the same thing (and are what I'm using). The $() I\n> think is just indicating that you'd do:\n>\n> foo=$(pwd)\n>\n> And yes, I think this is a case where using the right one is important\n> (which is why I used the pwd program, and not $pwd in the test).\n>\n> Or am I missing something else?\n\nI was thinking, in particular, about this snippet from t/test-lib.sh:\n\n # git sees Windows-style pwd\n pwd () {\n builtin pwd -W\n }\n\nIf that's inherited by the subshell used in the test, then I suppose\nall is okay, though I think it would not be inherited.\n"},{"id":"463425","messageId":"YywADT0aHRSoDeg3@coredump.intra.peff.net","threadId":"58481","inReplyTo":"Yyv+GjjauvvcX3M1@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] clone: allow \"--bare\" with \"-o\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-09-22T06:26:21Z","receivedAt":"2022-09-22T06:27:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 22, 2022 at 02:18:02AM -0400, Jeff King wrote:\n\n> And yes, I think this is a case where using the right one is important\n> (which is why I used the pwd program, and not $pwd in the test).\n\nEr, of course I meant all-caps $PWD in the final sentence. ;)\n\n-Peff\n"},{"id":"463426","messageId":"YywCImASth7I0Wx2@coredump.intra.peff.net","threadId":"58481","inReplyTo":"CAPig+cQxSwz2cZFLNwg_rJ==M5=pxMVvwgbDFV1r=v+rzbCuNg@mail.gmail.com","subject":"Re: [PATCH 1/2] clone: allow \"--bare\" with \"-o\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-09-22T06:35:14Z","receivedAt":"2022-09-22T06:35:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 22, 2022 at 02:26:09AM -0400, Eric Sunshine wrote:\n\n> > I think you might be mis-reading the advice here. It is saying to use\n> > the \"pwd\" program, rather than relying on the shell's $PWD variable. So\n> > $(pwd) and `pwd` are the same thing (and are what I'm using). The $() I\n> > think is just indicating that you'd do:\n> >\n> > foo=$(pwd)\n> >\n> > And yes, I think this is a case where using the right one is important\n> > (which is why I used the pwd program, and not $pwd in the test).\n> >\n> > Or am I missing something else?\n> \n> I was thinking, in particular, about this snippet from t/test-lib.sh:\n> \n> # git sees Windows-style pwd\n> pwd () {\n> builtin pwd -W\n> }\n> \n> If that's inherited by the subshell used in the test, then I suppose\n> all is okay, though I think it would not be inherited.\n\nI think it's OK. $() is itself a subshell, and you can find many calls\nto FOO=$(pwd) or similar. And in general, functions are inherited in\nsubshells:\n\n $ sh -c 'foo() { echo bar; }; (cd .. && foo)'\n bar\n\nThey'd have to be or many things in our test suite would break, since we\nuse test_must_fail, etc, inside subshells.\n\n-Peff\n"},{"id":"463427","messageId":"CAPig+cSwYYD6U0bgHL+8pp+j8wOPRhVc6o6LdkP5eR=vOOy6HQ@mail.gmail.com","threadId":"58481","inReplyTo":"YywCImASth7I0Wx2@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] clone: allow \"--bare\" with \"-o\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2022-09-22T06:40:28Z","receivedAt":"2022-09-22T06:40:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Sep 22, 2022 at 2:35 AM Jeff King wrote:\n> On Thu, Sep 22, 2022 at 02:26:09AM -0400, Eric Sunshine wrote:\n> > I was thinking, in particular, about this snippet from t/test-lib.sh:\n> >\n> > # git sees Windows-style pwd\n> > pwd () {\n> > builtin pwd -W\n> > }\n> >\n> > If that's inherited by the subshell used in the test, then I suppose\n> > all is okay, though I think it would not be inherited.\n>\n> I think it's OK. $() is itself a subshell, and you can find many calls\n> to FOO=$(pwd) or similar. And in general, functions are inherited in\n> subshells:\n>\n> $ sh -c 'foo() { echo bar; }; (cd .. && foo)'\n> bar\n>\n> They'd have to be or many things in our test suite would break, since we\n> use test_must_fail, etc, inside subshells.\n\nYep, thanks for the sanity dose. For some reason, my brain was\noff-kilter, thinking that it was running a shell afresh, not just\nusing a subshell.\n"}]}