{"thread":{"id":"44855","subject":"[PATCH] lib-submodule-update.sh: drop unneeded shell","startedAt":"2017-01-11T18:47:46Z","lastAt":"2017-01-13T20:19:45Z","messageCount":6,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"309225","messageId":"20170111184732.26416-1-sbeller@google.com","threadId":"44855","inReplyTo":null,"subject":"[PATCH] lib-submodule-update.sh: drop unneeded shell","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-11T18:47:32Z","receivedAt":"2017-01-11T18:47:46Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In modern Git we prefer \"git -C <cmd\" over \"(cd <somewhere && git <cmd>)\"\nas it doesn't need an extra shell.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\nAnd because it is in a setup function, we actually save the invocation\nof 22 shells for a single run of the whole test suite.\n\nNoticed while adding a lot more in near vincinity, though not as near\nto cause merge conflicts, so sending it extra.\n\nThanks,\nStefan\n\n t/lib-submodule-update.sh | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh\nindex 79cdd34a54..915eb4a7c6 100755\n--- a/t/lib-submodule-update.sh\n+++ b/t/lib-submodule-update.sh\n@@ -69,10 +69,7 @@ create_lib_submodule_repo () {\n \n \t\tgit checkout -b \"replace_sub1_with_directory\" \"add_sub1\" &&\n \t\tgit submodule update &&\n-\t\t(\n-\t\t\tcd sub1 &&\n-\t\t\tgit checkout modifications\n-\t\t) &&\n+\t\tgit -C sub1 checkout modifications &&\n \t\tgit rm --cached sub1 &&\n \t\trm sub1/.git* &&\n \t\tgit config -f .gitmodules --remove-section \"submodule.sub1\" &&\n-- \n2.11.0.259.g7b30ecf4f0\n\n"},{"id":"309382","messageId":"xmqqtw92hkgc.fsf@gitster.mtv.corp.google.com","threadId":"44855","inReplyTo":"20170111184732.26416-1-sbeller@google.com","subject":"Re: [PATCH] lib-submodule-update.sh: drop unneeded shell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-13T18:40:51Z","receivedAt":"2017-01-13T18:41:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> In modern Git we prefer \"git -C <cmd\" over \"(cd <somewhere && git <cmd>)\"\n> as it doesn't need an extra shell.\n\nThere is a matching '>' missing.  The description is correct (I am\nnot sure if there actually is \"preference\", though), but I found the\ntitle a bit misleading.  With 'drop unneeded subshell', I would\nimagine a patch like\n\n\t\t(\n\t-\t\t( do something )\n\t+\t\tdo something;\n\t\t\tdo something else;\n\t\t)\n\nIt looks more like \"reduce use of subshell by using 'git -C <dir>'\"\nto me.\n\n> And because it is in a setup function, we actually save the invocation\n> of 22 shells for a single run of the whole test suite.\n\nNice.\n\n>\n> Noticed while adding a lot more in near vincinity, though not as near\n> to cause merge conflicts, so sending it extra.\n>\n> Thanks,\n> Stefan\n>\n>  t/lib-submodule-update.sh | 5 +----\n>  1 file changed, 1 insertion(+), 4 deletions(-)\n>\n> diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh\n> index 79cdd34a54..915eb4a7c6 100755\n> --- a/t/lib-submodule-update.sh\n> +++ b/t/lib-submodule-update.sh\n> @@ -69,10 +69,7 @@ create_lib_submodule_repo () {\n>  \n>  \t\tgit checkout -b \"replace_sub1_with_directory\" \"add_sub1\" &&\n>  \t\tgit submodule update &&\n> -\t\t(\n> -\t\t\tcd sub1 &&\n> -\t\t\tgit checkout modifications\n> -\t\t) &&\n> +\t\tgit -C sub1 checkout modifications &&\n>  \t\tgit rm --cached sub1 &&\n>  \t\trm sub1/.git* &&\n>  \t\tgit config -f .gitmodules --remove-section \"submodule.sub1\" &&\n"},{"id":"309386","messageId":"20170113190318.18412-1-sbeller@google.com","threadId":"44855","inReplyTo":"xmqqtw92hkgc.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] lib-submodule-update.sh: reduce use of subshell by using git -C <dir>","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-13T19:03:18Z","receivedAt":"2017-01-13T19:03:28Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In modern Git we prefer\n    \"git -C <cmd>\"\nover\n    \"(cd <somewhere && git <cmd>)\"\nas it doesn't need an extra shell.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n t/lib-submodule-update.sh | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh\nindex 79cdd34a54..915eb4a7c6 100755\n--- a/t/lib-submodule-update.sh\n+++ b/t/lib-submodule-update.sh\n@@ -69,10 +69,7 @@ create_lib_submodule_repo () {\n \n \t\tgit checkout -b \"replace_sub1_with_directory\" \"add_sub1\" &&\n \t\tgit submodule update &&\n-\t\t(\n-\t\t\tcd sub1 &&\n-\t\t\tgit checkout modifications\n-\t\t) &&\n+\t\tgit -C sub1 checkout modifications &&\n \t\tgit rm --cached sub1 &&\n \t\trm sub1/.git* &&\n \t\tgit config -f .gitmodules --remove-section \"submodule.sub1\" &&\n-- \n2.11.0.300.g08194d1431\n\n"},{"id":"309390","messageId":"xmqqa8auhgzt.fsf@gitster.mtv.corp.google.com","threadId":"44855","inReplyTo":"xmqqtw92hkgc.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] lib-submodule-update.sh: drop unneeded shell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-13T19:55:34Z","receivedAt":"2017-01-13T19:55:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> In modern Git we prefer \"git -C <cmd\" over \"(cd <somewhere && git <cmd>)\"\n>> as it doesn't need an extra shell.\n>\n> There is a matching '>' missing.  The description is correct (I am\n> not sure if there actually is \"preference\", though), but I found the\n> title a bit misleading....\n\nIt turns out that there were two missing '>' ;-)  It tentatively has\nbecome like this in my tree.\n\n-- >8 --\nFrom: Stefan Beller <sbeller@google.com>\nDate: Wed, 11 Jan 2017 10:47:32 -0800\nSubject: [PATCH] lib-submodule-update.sh: reduce use of subshell by using \"git -C\"\n\nWe write\n\n    (cd <dir> && git <cmd>)\n\nto avoid\n\n    cd <dir> && git <cmd> && cd ..\n\nthat allows a breakage in one part of the test script to leave the\nentire test process in an unexpected place.  We can do this more\nconcisely with \"git -C <dir> <cmd>\" with modern Git.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n\ndiff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh\nindex 79cdd34a54..915eb4a7c6 100755\n--- a/t/lib-submodule-update.sh\n+++ b/t/lib-submodule-update.sh\n@@ -69,10 +69,7 @@ create_lib_submodule_repo () {\n \n \t\tgit checkout -b \"replace_sub1_with_directory\" \"add_sub1\" &&\n \t\tgit submodule update &&\n-\t\t(\n-\t\t\tcd sub1 &&\n-\t\t\tgit checkout modifications\n-\t\t) &&\n+\t\tgit -C sub1 checkout modifications &&\n \t\tgit rm --cached sub1 &&\n \t\trm sub1/.git* &&\n \t\tgit config -f .gitmodules --remove-section \"submodule.sub1\" &&\n"},{"id":"309393","messageId":"CAGZ79kayLoH7EURQ9aKGh+FzDz_BegJRjB2175qo53beLZDYog@mail.gmail.com","threadId":"44855","inReplyTo":"xmqqa8auhgzt.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] lib-submodule-update.sh: drop unneeded shell","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-13T20:01:18Z","receivedAt":"2017-01-13T20:10:45Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Jan 13, 2017 at 11:55 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Stefan Beller <sbeller@google.com> writes:\n>>\n>>> In modern Git we prefer \"git -C <cmd\" over \"(cd <somewhere && git <cmd>)\"\n>>> as it doesn't need an extra shell.\n>>\n>> There is a matching '>' missing.  The description is correct (I am\n>> not sure if there actually is \"preference\", though), but I found the\n>> title a bit misleading....\n>\n> It turns out that there were two missing '>' ;-)  It tentatively has\n> become like this in my tree.\n\nThanks for fixing up locally. I had resent as\n\"[PATCH] lib-submodule-update.sh: reduce use of subshell by using git -C <dir>\"\nbut you can ignore that now.\n\nThank,\nStefan\n"},{"id":"309394","messageId":"xmqqk29yg1bb.fsf@gitster.mtv.corp.google.com","threadId":"44855","inReplyTo":"CAGZ79kayLoH7EURQ9aKGh+FzDz_BegJRjB2175qo53beLZDYog@mail.gmail.com","subject":"Re: [PATCH] lib-submodule-update.sh: drop unneeded shell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-13T20:19:36Z","receivedAt":"2017-01-13T20:19:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> On Fri, Jan 13, 2017 at 11:55 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> Stefan Beller <sbeller@google.com> writes:\n>>>\n>>>> In modern Git we prefer \"git -C <cmd\" over \"(cd <somewhere && git <cmd>)\"\n>>>> as it doesn't need an extra shell.\n>>>\n>>> There is a matching '>' missing.  The description is correct (I am\n>>> not sure if there actually is \"preference\", though), but I found the\n>>> title a bit misleading....\n>>\n>> It turns out that there were two missing '>' ;-)  It tentatively has\n>> become like this in my tree.\n>\n> Thanks for fixing up locally. I had resent as\n> \"[PATCH] lib-submodule-update.sh: reduce use of subshell by using git -C <dir>\"\n> but you can ignore that now.\n\nYeah, apparently our mails crossed.  Yours still have \"git -C <cmd>\"\nthat should have been \"git -C <dir> <cmd>\", so I'll keep the locally\nmunged one.\n\nThanks.\n\n\n"}]}