{"thread":{"id":"48084","subject":"[PATCH] filter-branch: use printf instead of echo -e","startedAt":"2018-03-19T14:49:15Z","lastAt":"2018-03-23T09:35:30Z","messageCount":10,"participants":["Michele Locati","Junio C Hamano","CB Bailey","Jeff King","Ian Campbell","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"342191","messageId":"20180319144905.11564-1-michele@locati.it","threadId":"48084","inReplyTo":null,"subject":"[PATCH] filter-branch: use printf instead of echo -e","fromName":"Michele Locati","fromEmail":"michele@locati.it","sentAt":"2018-03-19T14:49:05Z","receivedAt":"2018-03-19T14:49:15Z","isPatch":true,"sender":{"key":"michele@locati.it","avatar":"https://avatars.githubusercontent.com/u/928116?v=4"},"body":"In order to echo a tab character, it's better to use printf instead of\n\"echo -e\", because it's more portable (for instance, \"echo -e\" doesn't work\nas expected on a Mac).\n\nThis solves the \"fatal: Not a valid object name\" error in git-filter-branch\nwhen using the --state-branch option.\n\nSigned-off-by: Michele Locati <michele@locati.it>\n---\n git-filter-branch.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 1b7e4b2cd..21d84eff3 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -627,7 +627,7 @@ then\n \t\t\t\tprint H \"$_:$f\\n\" or die;\n \t\t\t}\n \t\t\tclose(H) or die;' || die \"Unable to save state\")\n-\tstate_tree=$(/bin/echo -e \"100644 blob $state_blob\\tfilter.map\" | git mktree)\n+\tstate_tree=$(printf '100644 blob %s\\tfilter.map\\n' \"$state_blob\" | git mktree)\n \tif test -n \"$state_commit\"\n \tthen\n \t\tstate_commit=$(/bin/echo \"Sync\" | git commit-tree \"$state_tree\" -p \"$state_commit\")\n-- \n2.16.2.windows.1\n\n"},{"id":"342195","messageId":"20180319155259.13200-1-michele@locati.it","threadId":"48084","inReplyTo":"20180319144905.11564-1-michele@locati.it","subject":"[PATCH v2] filter-branch: use printf instead of echo -e","fromName":"Michele Locati","fromEmail":"michele@locati.it","sentAt":"2018-03-19T15:52:59Z","receivedAt":"2018-03-19T15:53:17Z","isPatch":true,"sender":{"key":"michele@locati.it","avatar":"https://avatars.githubusercontent.com/u/928116?v=4"},"body":"In order to echo a tab character, it's better to use printf instead of\n\"echo -e\", because it's more portable (for instance, \"echo -e\" doesn't work\nas expected on a Mac).\n\nThis solves the \"fatal: Not a valid object name\" error in git-filter-branch\nwhen using the --state-branch option.\n\nFurthermore, let's switch from \"/bin/echo\" to just \"echo\", so that the\nbuilt-in echo command is used where available.\n\nSigned-off-by: Michele Locati <michele@locati.it>\n---\n git-filter-branch.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 1b7e4b2cd..98c76ec58 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -627,12 +627,12 @@ then\n \t\t\t\tprint H \"$_:$f\\n\" or die;\n \t\t\t}\n \t\t\tclose(H) or die;' || die \"Unable to save state\")\n-\tstate_tree=$(/bin/echo -e \"100644 blob $state_blob\\tfilter.map\" | git mktree)\n+\tstate_tree=$(printf '100644 blob %s\\tfilter.map\\n' \"$state_blob\" | git mktree)\n \tif test -n \"$state_commit\"\n \tthen\n-\t\tstate_commit=$(/bin/echo \"Sync\" | git commit-tree \"$state_tree\" -p \"$state_commit\")\n+\t\tstate_commit=$(echo \"Sync\" | git commit-tree \"$state_tree\" -p \"$state_commit\")\n \telse\n-\t\tstate_commit=$(/bin/echo \"Sync\" | git commit-tree \"$state_tree\" )\n+\t\tstate_commit=$(echo \"Sync\" | git commit-tree \"$state_tree\" )\n \tfi\n \tgit update-ref \"$state_branch\" \"$state_commit\"\n fi\n-- \n2.16.2.windows.1\n\n"},{"id":"342220","messageId":"xmqqtvtcuer6.fsf@gitster-ct.c.googlers.com","threadId":"48084","inReplyTo":"20180319155259.13200-1-michele@locati.it","subject":"Re: [PATCH v2] filter-branch: use printf instead of echo -e","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-19T18:00:45Z","receivedAt":"2018-03-19T18:00:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michele Locati <michele@locati.it> writes:\n\n> In order to echo a tab character, it's better to use printf instead of\n> \"echo -e\", because it's more portable (for instance, \"echo -e\" doesn't work\n> as expected on a Mac).\n>\n> This solves the \"fatal: Not a valid object name\" error in git-filter-branch\n> when using the --state-branch option.\n>\n> Furthermore, let's switch from \"/bin/echo\" to just \"echo\", so that the\n> built-in echo command is used where available.\n>\n> Signed-off-by: Michele Locati <michele@locati.it>\n> ---\n\nThanks; will queue.\n\n>  git-filter-branch.sh | 6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index 1b7e4b2cd..98c76ec58 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -627,12 +627,12 @@ then\n>  \t\t\t\tprint H \"$_:$f\\n\" or die;\n>  \t\t\t}\n>  \t\t\tclose(H) or die;' || die \"Unable to save state\")\n> -\tstate_tree=$(/bin/echo -e \"100644 blob $state_blob\\tfilter.map\" | git mktree)\n> +\tstate_tree=$(printf '100644 blob %s\\tfilter.map\\n' \"$state_blob\" | git mktree)\n>  \tif test -n \"$state_commit\"\n>  \tthen\n> -\t\tstate_commit=$(/bin/echo \"Sync\" | git commit-tree \"$state_tree\" -p \"$state_commit\")\n> +\t\tstate_commit=$(echo \"Sync\" | git commit-tree \"$state_tree\" -p \"$state_commit\")\n>  \telse\n> -\t\tstate_commit=$(/bin/echo \"Sync\" | git commit-tree \"$state_tree\" )\n> +\t\tstate_commit=$(echo \"Sync\" | git commit-tree \"$state_tree\" )\n>  \tfi\n>  \tgit update-ref \"$state_branch\" \"$state_commit\"\n>  fi\n"},{"id":"342261","messageId":"20180319153945.kchupu43cpcbg25n@hashpling.org","threadId":"48084","inReplyTo":"20180319144905.11564-1-michele@locati.it","subject":"Re: [PATCH] filter-branch: use printf instead of echo -e","fromName":"CB Bailey","fromEmail":"charles@hashpling.org","sentAt":"2018-03-19T15:39:46Z","receivedAt":"2018-03-19T22:41:11Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Mon, Mar 19, 2018 at 03:49:05PM +0100, Michele Locati wrote:\n> In order to echo a tab character, it's better to use printf instead of\n> \"echo -e\", because it's more portable (for instance, \"echo -e\" doesn't work\n> as expected on a Mac).\n> \n> This solves the \"fatal: Not a valid object name\" error in git-filter-branch\n> when using the --state-branch option.\n> \n> Signed-off-by: Michele Locati <michele@locati.it>\n> ---\n>  git-filter-branch.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index 1b7e4b2cd..21d84eff3 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -627,7 +627,7 @@ then\n>  \t\t\t\tprint H \"$_:$f\\n\" or die;\n>  \t\t\t}\n>  \t\t\tclose(H) or die;' || die \"Unable to save state\")\n> -\tstate_tree=$(/bin/echo -e \"100644 blob $state_blob\\tfilter.map\" | git mktree)\n> +\tstate_tree=$(printf '100644 blob %s\\tfilter.map\\n' \"$state_blob\" | git mktree)\n>  \tif test -n \"$state_commit\"\n>  \tthen\n>  \t\tstate_commit=$(/bin/echo \"Sync\" | git commit-tree \"$state_tree\" -p \"$state_commit\")\n\nI think the change from 'echo -e' to printf is good because of the\nbetter portability reason that you cite.\n\nLooking at the change, I am now curious as to why '/bin/echo' is used.\nTesting on a Mac, bash's built in 'echo' recognizes '-e' whereas\n'/bin/echo' does not. This is just an observation, I still prefer the\nmove to 'printf' that you suggest.\n\nThere are two further uses of '/bin/echo' in git-filter-branch.sh which\nare portable (no \"-e\", just printing a word that cannot be confused for\nan option). One is visible in your diff context and the other is just\nbelow it. For consistency with other echos in git-filter-branch.sh, I\nthink that these should probably use simple 'echo' rather than\n'/bin/echo' to use a builtin where available.\n\nCB\n"},{"id":"342282","messageId":"20180320042245.GA13302@sigill.intra.peff.net","threadId":"48084","inReplyTo":"20180319153945.kchupu43cpcbg25n@hashpling.org","subject":"Re: [PATCH] filter-branch: use printf instead of echo -e","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-20T04:22:45Z","receivedAt":"2018-03-20T04:22:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 19, 2018 at 03:39:46PM +0000, CB Bailey wrote:\n\n> > diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> > index 1b7e4b2cd..21d84eff3 100755\n> > --- a/git-filter-branch.sh\n> > +++ b/git-filter-branch.sh\n> > @@ -627,7 +627,7 @@ then\n> >  \t\t\t\tprint H \"$_:$f\\n\" or die;\n> >  \t\t\t}\n> >  \t\t\tclose(H) or die;' || die \"Unable to save state\")\n> > -\tstate_tree=$(/bin/echo -e \"100644 blob $state_blob\\tfilter.map\" | git mktree)\n> > +\tstate_tree=$(printf '100644 blob %s\\tfilter.map\\n' \"$state_blob\" | git mktree)\n> >  \tif test -n \"$state_commit\"\n> >  \tthen\n> >  \t\tstate_commit=$(/bin/echo \"Sync\" | git commit-tree \"$state_tree\" -p \"$state_commit\")\n> \n> I think the change from 'echo -e' to printf is good because of the\n> better portability reason that you cite.\n> \n> Looking at the change, I am now curious as to why '/bin/echo' is used.\n> Testing on a Mac, bash's built in 'echo' recognizes '-e' whereas\n> '/bin/echo' does not. This is just an observation, I still prefer the\n> move to 'printf' that you suggest.\n\nRight. Moving them to just \"echo -e\" would work on systems where /bin/sh\nis bash, but not elsewhere (e.g., Debian systems with \"dash\" whose\nbuilt-in echo doesn't understand \"-e\").\n\nSo my guess as to why /bin/echo was used is that on Linux systems it's\n_more_ predictable and portable, because you know you're always going to\nget the GNU coreutils version, which knows \"-e\". Even if you're using a\nnon-bash shell.\n\nBut on non-Linux systems, who knows what system \"echo\" you'll get. :)\n\nAuthor cc'd in case there's something more interesting going on.\n\n-Peff\n"},{"id":"342302","messageId":"1521539621.2474.60.camel@hellion.org.uk","threadId":"48084","inReplyTo":"20180320042245.GA13302@sigill.intra.peff.net","subject":"Re: [PATCH] filter-branch: use printf instead of echo -e","fromName":"Ian Campbell","fromEmail":"ijc@hellion.org.uk","sentAt":"2018-03-20T09:53:41Z","receivedAt":"2018-03-20T09:53:53Z","isPatch":true,"sender":{"key":"ijc@hellion.org.uk","avatar":"https://avatars.githubusercontent.com/u/12985729?v=4"},"body":"On Tue, 2018-03-20 at 00:22 -0400, Jeff King wrote:\n> Author cc'd in case there's something more interesting going on.\n\nThat code was written years ago, if I had a good reason at the time\nI've forgotten what it was and I can't think of a fresh one now.\nSwitching to printf seems like a reasonable thing to do.\n\nPerhaps switch the remaining `/bin/echo` (there are two without `-e`)\nuses to just `echo` for consistency with the rest of the file?\n\nIan.\n"},{"id":"342312","messageId":"d2bf28a9-ae79-9046-22f4-ceac502882f6@locati.it","threadId":"48084","inReplyTo":"1521539621.2474.60.camel@hellion.org.uk","subject":"Re: [PATCH] filter-branch: use printf instead of echo -e","fromName":"Michele Locati","fromEmail":"michele@locati.it","sentAt":"2018-03-20T10:21:54Z","receivedAt":"2018-03-20T10:28:42Z","isPatch":true,"sender":{"key":"michele@locati.it","avatar":"https://avatars.githubusercontent.com/u/928116?v=4"},"body":"Il 20/03/2018 10:53, Ian Campbell ha scritto:\n> On Tue, 2018-03-20 at 00:22 -0400, Jeff King wrote:\n>> Author cc'd in case there's something more interesting going on.\n> \n> That code was written years ago, if I had a good reason at the time\n> I've forgotten what it was and I can't think of a fresh one now.\n> Switching to printf seems like a reasonable thing to do.\n> \n> Perhaps switch the remaining `/bin/echo` (there are two without `-e`)\n> uses to just `echo` for consistency with the rest of the file?\n> \n> Ian.\n> \n\nThanks for reply, Ian.\nCharles already suggested to replace /bin/echo with echo, and I already \nupdated the patch accordingly: see\nhttps://marc.info/?l=git&m=152147479517707&w=2\nJunio already queued that PATCH-v2.\n\n--\nMichele\n"},{"id":"342463","messageId":"nycvar.QRO.7.76.6.1803212249170.77@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48084","inReplyTo":"20180319155259.13200-1-michele@locati.it","subject":"Re: [PATCH v2] filter-branch: use printf instead of echo -e","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-21T21:50:21Z","receivedAt":"2018-03-21T21:50:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Michele,\n\nOn Mon, 19 Mar 2018, Michele Locati wrote:\n\n> [...]\n> -- \n> 2.16.2.windows.1\n\nYaaaaay!\n\nOut of curiosity: did the CONTRIBUTING.md file help that was recently\nturned into a guide how to contribute to Git (for Windows) by Derrick\nStolee?\n\nCiao,\nJohannes\n"},{"id":"342502","messageId":"081ccfce-7a89-503e-465e-3befe6f33f17@locati.it","threadId":"48084","inReplyTo":"nycvar.QRO.7.76.6.1803212249170.77@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [PATCH v2] filter-branch: use printf instead of echo -e","fromName":"Michele Locati","fromEmail":"michele@locati.it","sentAt":"2018-03-22T13:40:21Z","receivedAt":"2018-03-22T13:40:41Z","isPatch":true,"sender":{"key":"michele@locati.it","avatar":"https://avatars.githubusercontent.com/u/928116?v=4"},"body":"Il 21/03/2018 22:50, Johannes Schindelin ha scritto:\n> Hi Michele,\n> \n> On Mon, 19 Mar 2018, Michele Locati wrote:\n> \n>> [...]\n>> -- \n>> 2.16.2.windows.1\n> \n> Yaaaaay!\n> \n> Out of curiosity: did the CONTRIBUTING.md file help that was recently\n> turned into a guide how to contribute to Git (for Windows) by Derrick\n> Stolee?\n\nWell, no. Here's what led me here...\n\nThe people behind the concrete5 CMS asked support for improving the \nfollowing procedure:\n\nconcrete5 is stored in\nhttps://github.com/concrete5/concrete5\nIn order to make it installable with Composer (a PHP package manager), \nwe need to extract its \"/concrete\" directory, and push the results to \nhttps://github.com/concrete5/concrete5-core\nWe previously used \"git filter-branch --all\" with this script:\nhttps://github.com/concrete5/core_splitter/blob/70879e676b95160f7fc5d0ffc22b8f7420b0580b/bin/splitcore\n\nThat script is executed every time someone pushes concrete5/concrete5, \nand it took very long time (3~5 minutes).\n\nI noticed on the git-filter-branch manual page that there's a \n\"--state-branch\", and it seemed quite interesting.\nSo, I asked in git@vger.kernel.org how to use it, and the author of that \nfeature (Ian Campbell) told me to look at some code he wrote.\nSo, I wrote\nhttps://github.com/concrete5/incremental-filter-branch\nwhich wraps \"git filter-branch --filter-state\", and it's used in\nhttps://github.com/concrete5/core_splitter/blob/99bbbcea1ab90572a04864ccb92327532ab6a42c/bin/splitcore\n(it not yet in production: Korvin wants to be sure everything works well \nbefore adopting it)\nThe time required for the operation dropped from 3~5 minutes to ~10 \nseconds ;)\n\nWhile writing that script, I noticed a couple of things that could be \nimproved in \"git-filter-branch\", so I submitted\nhttps://marc.info/?l=git&m=152111905428554&w=2\n(so that the script could run without problems if there's nothing to do)\nand\nhttps://marc.info/?l=git&m=152147095416175&w=2\n(so that  --filter-state could be used on Mac and other non-Linux systems).\nHow did I learn how to submit to git? I searched for \"git src\", landed \non https://github.com/git/git, read Documentation/SubmittingPatches and \nused git send-email.\n\nAnd yes, all that on Windows (and WSL).\n\n\n> \n> Ciao,\n> Johannes\n\nCiao!\n\n--\nMichele\n\n"},{"id":"342571","messageId":"nycvar.QRO.7.76.6.1803231034400.77@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48084","inReplyTo":"081ccfce-7a89-503e-465e-3befe6f33f17@locati.it","subject":"Re: [PATCH v2] filter-branch: use printf instead of echo -e","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-23T09:35:21Z","receivedAt":"2018-03-23T09:35:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Michele,\n\nOn Thu, 22 Mar 2018, Michele Locati wrote:\n\n> Il 21/03/2018 22:50, Johannes Schindelin ha scritto:\n> > \n> > On Mon, 19 Mar 2018, Michele Locati wrote:\n> > \n> > > [...]\n> > > -- \n> > > 2.16.2.windows.1\n> > \n> > Yaaaaay!\n> > \n> > Out of curiosity: did the CONTRIBUTING.md file help that was recently\n> > turned into a guide how to contribute to Git (for Windows) by Derrick\n> > Stolee?\n> \n> Well, no. Here's what led me here...\n> \n> [...]\n\nThank you for taking the time to tell the story!\n\nCiao,\nJohannes\n"}]}