# [PATCH] filter-branch: use printf instead of echo -e 10 messages from 2018-03-19 to 2018-03-23. Participants: Michele Locati, Junio C Hamano, CB Bailey, Jeff King, Ian Campbell, Johannes Schindelin. Thread: https://gitlist.dev/t/48084 ## Michele Locati, 2018-03-19 14:49 Subject: [PATCH] filter-branch: use printf instead of echo -e Message-ID: <20180319144905.11564-1-michele@locati.it> URL: https://gitlist.dev/e/20180319144905.11564-1-michele%40locati.it ``` In order to echo a tab character, it's better to use printf instead of "echo -e", because it's more portable (for instance, "echo -e" doesn't work as expected on a Mac). This solves the "fatal: Not a valid object name" error in git-filter-branch when using the --state-branch option. Signed-off-by: Michele Locati --- git-filter-branch.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-filter-branch.sh b/git-filter-branch.sh index 1b7e4b2cd..21d84eff3 100755 --- a/git-filter-branch.sh +++ b/git-filter-branch.sh @@ -627,7 +627,7 @@ then print H "$_:$f\n" or die; } close(H) or die;' || die "Unable to save state") - state_tree=$(/bin/echo -e "100644 blob $state_blob\tfilter.map" | git mktree) + state_tree=$(printf '100644 blob %s\tfilter.map\n' "$state_blob" | git mktree) if test -n "$state_commit" then state_commit=$(/bin/echo "Sync" | git commit-tree "$state_tree" -p "$state_commit") -- 2.16.2.windows.1 ``` ## Michele Locati, 2018-03-19 15:52 Subject: [PATCH v2] filter-branch: use printf instead of echo -e Message-ID: <20180319155259.13200-1-michele@locati.it> URL: https://gitlist.dev/e/20180319155259.13200-1-michele%40locati.it In-Reply-To: <20180319144905.11564-1-michele@locati.it> ``` In order to echo a tab character, it's better to use printf instead of "echo -e", because it's more portable (for instance, "echo -e" doesn't work as expected on a Mac). This solves the "fatal: Not a valid object name" error in git-filter-branch when using the --state-branch option. Furthermore, let's switch from "/bin/echo" to just "echo", so that the built-in echo command is used where available. Signed-off-by: Michele Locati --- git-filter-branch.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/git-filter-branch.sh b/git-filter-branch.sh index 1b7e4b2cd..98c76ec58 100755 --- a/git-filter-branch.sh +++ b/git-filter-branch.sh @@ -627,12 +627,12 @@ then print H "$_:$f\n" or die; } close(H) or die;' || die "Unable to save state") - state_tree=$(/bin/echo -e "100644 blob $state_blob\tfilter.map" | git mktree) + state_tree=$(printf '100644 blob %s\tfilter.map\n' "$state_blob" | git mktree) if test -n "$state_commit" then - state_commit=$(/bin/echo "Sync" | git commit-tree "$state_tree" -p "$state_commit") + state_commit=$(echo "Sync" | git commit-tree "$state_tree" -p "$state_commit") else - state_commit=$(/bin/echo "Sync" | git commit-tree "$state_tree" ) + state_commit=$(echo "Sync" | git commit-tree "$state_tree" ) fi git update-ref "$state_branch" "$state_commit" fi -- 2.16.2.windows.1 ``` ## Junio C Hamano, 2018-03-19 18:00 Subject: Re: [PATCH v2] filter-branch: use printf instead of echo -e Message-ID: URL: https://gitlist.dev/e/xmqqtvtcuer6.fsf%40gitster-ct.c.googlers.com In-Reply-To: <20180319155259.13200-1-michele@locati.it> ``` Michele Locati writes: > In order to echo a tab character, it's better to use printf instead of > "echo -e", because it's more portable (for instance, "echo -e" doesn't work > as expected on a Mac). > > This solves the "fatal: Not a valid object name" error in git-filter-branch > when using the --state-branch option. > > Furthermore, let's switch from "/bin/echo" to just "echo", so that the > built-in echo command is used where available. > > Signed-off-by: Michele Locati > --- Thanks; will queue. > git-filter-branch.sh | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/git-filter-branch.sh b/git-filter-branch.sh > index 1b7e4b2cd..98c76ec58 100755 > --- a/git-filter-branch.sh > +++ b/git-filter-branch.sh > @@ -627,12 +627,12 @@ then > print H "$_:$f\n" or die; > } > close(H) or die;' || die "Unable to save state") > - state_tree=$(/bin/echo -e "100644 blob $state_blob\tfilter.map" | git mktree) > + state_tree=$(printf '100644 blob %s\tfilter.map\n' "$state_blob" | git mktree) > if test -n "$state_commit" > then > - state_commit=$(/bin/echo "Sync" | git commit-tree "$state_tree" -p "$state_commit") > + state_commit=$(echo "Sync" | git commit-tree "$state_tree" -p "$state_commit") > else > - state_commit=$(/bin/echo "Sync" | git commit-tree "$state_tree" ) > + state_commit=$(echo "Sync" | git commit-tree "$state_tree" ) > fi > git update-ref "$state_branch" "$state_commit" > fi ``` ## CB Bailey, 2018-03-19 15:39 Subject: Re: [PATCH] filter-branch: use printf instead of echo -e Message-ID: <20180319153945.kchupu43cpcbg25n@hashpling.org> URL: https://gitlist.dev/e/20180319153945.kchupu43cpcbg25n%40hashpling.org In-Reply-To: <20180319144905.11564-1-michele@locati.it> ``` On Mon, Mar 19, 2018 at 03:49:05PM +0100, Michele Locati wrote: > In order to echo a tab character, it's better to use printf instead of > "echo -e", because it's more portable (for instance, "echo -e" doesn't work > as expected on a Mac). > > This solves the "fatal: Not a valid object name" error in git-filter-branch > when using the --state-branch option. > > Signed-off-by: Michele Locati > --- > git-filter-branch.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/git-filter-branch.sh b/git-filter-branch.sh > index 1b7e4b2cd..21d84eff3 100755 > --- a/git-filter-branch.sh > +++ b/git-filter-branch.sh > @@ -627,7 +627,7 @@ then > print H "$_:$f\n" or die; > } > close(H) or die;' || die "Unable to save state") > - state_tree=$(/bin/echo -e "100644 blob $state_blob\tfilter.map" | git mktree) > + state_tree=$(printf '100644 blob %s\tfilter.map\n' "$state_blob" | git mktree) > if test -n "$state_commit" > then > state_commit=$(/bin/echo "Sync" | git commit-tree "$state_tree" -p "$state_commit") I think the change from 'echo -e' to printf is good because of the better portability reason that you cite. Looking at the change, I am now curious as to why '/bin/echo' is used. Testing on a Mac, bash's built in 'echo' recognizes '-e' whereas '/bin/echo' does not. This is just an observation, I still prefer the move to 'printf' that you suggest. There are two further uses of '/bin/echo' in git-filter-branch.sh which are portable (no "-e", just printing a word that cannot be confused for an option). One is visible in your diff context and the other is just below it. For consistency with other echos in git-filter-branch.sh, I think that these should probably use simple 'echo' rather than '/bin/echo' to use a builtin where available. CB ``` ## Jeff King, 2018-03-20 04:22 Subject: Re: [PATCH] filter-branch: use printf instead of echo -e Message-ID: <20180320042245.GA13302@sigill.intra.peff.net> URL: https://gitlist.dev/e/20180320042245.GA13302%40sigill.intra.peff.net In-Reply-To: <20180319153945.kchupu43cpcbg25n@hashpling.org> ``` On Mon, Mar 19, 2018 at 03:39:46PM +0000, CB Bailey wrote: > > diff --git a/git-filter-branch.sh b/git-filter-branch.sh > > index 1b7e4b2cd..21d84eff3 100755 > > --- a/git-filter-branch.sh > > +++ b/git-filter-branch.sh > > @@ -627,7 +627,7 @@ then > > print H "$_:$f\n" or die; > > } > > close(H) or die;' || die "Unable to save state") > > - state_tree=$(/bin/echo -e "100644 blob $state_blob\tfilter.map" | git mktree) > > + state_tree=$(printf '100644 blob %s\tfilter.map\n' "$state_blob" | git mktree) > > if test -n "$state_commit" > > then > > state_commit=$(/bin/echo "Sync" | git commit-tree "$state_tree" -p "$state_commit") > > I think the change from 'echo -e' to printf is good because of the > better portability reason that you cite. > > Looking at the change, I am now curious as to why '/bin/echo' is used. > Testing on a Mac, bash's built in 'echo' recognizes '-e' whereas > '/bin/echo' does not. This is just an observation, I still prefer the > move to 'printf' that you suggest. Right. Moving them to just "echo -e" would work on systems where /bin/sh is bash, but not elsewhere (e.g., Debian systems with "dash" whose built-in echo doesn't understand "-e"). So my guess as to why /bin/echo was used is that on Linux systems it's _more_ predictable and portable, because you know you're always going to get the GNU coreutils version, which knows "-e". Even if you're using a non-bash shell. But on non-Linux systems, who knows what system "echo" you'll get. :) Author cc'd in case there's something more interesting going on. -Peff ``` ## Ian Campbell, 2018-03-20 09:53 Subject: Re: [PATCH] filter-branch: use printf instead of echo -e Message-ID: <1521539621.2474.60.camel@hellion.org.uk> URL: https://gitlist.dev/e/1521539621.2474.60.camel%40hellion.org.uk In-Reply-To: <20180320042245.GA13302@sigill.intra.peff.net> ``` On Tue, 2018-03-20 at 00:22 -0400, Jeff King wrote: > Author cc'd in case there's something more interesting going on. That code was written years ago, if I had a good reason at the time I've forgotten what it was and I can't think of a fresh one now. Switching to printf seems like a reasonable thing to do. Perhaps switch the remaining `/bin/echo` (there are two without `-e`) uses to just `echo` for consistency with the rest of the file? Ian. ``` ## Michele Locati, 2018-03-20 10:21 Subject: Re: [PATCH] filter-branch: use printf instead of echo -e Message-ID: URL: https://gitlist.dev/e/d2bf28a9-ae79-9046-22f4-ceac502882f6%40locati.it In-Reply-To: <1521539621.2474.60.camel@hellion.org.uk> ``` Il 20/03/2018 10:53, Ian Campbell ha scritto: > On Tue, 2018-03-20 at 00:22 -0400, Jeff King wrote: >> Author cc'd in case there's something more interesting going on. > > That code was written years ago, if I had a good reason at the time > I've forgotten what it was and I can't think of a fresh one now. > Switching to printf seems like a reasonable thing to do. > > Perhaps switch the remaining `/bin/echo` (there are two without `-e`) > uses to just `echo` for consistency with the rest of the file? > > Ian. > Thanks for reply, Ian. Charles already suggested to replace /bin/echo with echo, and I already updated the patch accordingly: see https://marc.info/?l=git&m=152147479517707&w=2 Junio already queued that PATCH-v2. -- Michele ``` ## Johannes Schindelin, 2018-03-21 21:50 Subject: Re: [PATCH v2] filter-branch: use printf instead of echo -e Message-ID: URL: https://gitlist.dev/e/nycvar.QRO.7.76.6.1803212249170.77%40ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz In-Reply-To: <20180319155259.13200-1-michele@locati.it> ``` Hi Michele, On Mon, 19 Mar 2018, Michele Locati wrote: > [...] > -- > 2.16.2.windows.1 Yaaaaay! Out of curiosity: did the CONTRIBUTING.md file help that was recently turned into a guide how to contribute to Git (for Windows) by Derrick Stolee? Ciao, Johannes ``` ## Michele Locati, 2018-03-22 13:40 Subject: Re: [PATCH v2] filter-branch: use printf instead of echo -e Message-ID: <081ccfce-7a89-503e-465e-3befe6f33f17@locati.it> URL: https://gitlist.dev/e/081ccfce-7a89-503e-465e-3befe6f33f17%40locati.it In-Reply-To: ``` Il 21/03/2018 22:50, Johannes Schindelin ha scritto: > Hi Michele, > > On Mon, 19 Mar 2018, Michele Locati wrote: > >> [...] >> -- >> 2.16.2.windows.1 > > Yaaaaay! > > Out of curiosity: did the CONTRIBUTING.md file help that was recently > turned into a guide how to contribute to Git (for Windows) by Derrick > Stolee? Well, no. Here's what led me here... The people behind the concrete5 CMS asked support for improving the following procedure: concrete5 is stored in https://github.com/concrete5/concrete5 In order to make it installable with Composer (a PHP package manager), we need to extract its "/concrete" directory, and push the results to https://github.com/concrete5/concrete5-core We previously used "git filter-branch --all" with this script: https://github.com/concrete5/core_splitter/blob/70879e676b95160f7fc5d0ffc22b8f7420b0580b/bin/splitcore That script is executed every time someone pushes concrete5/concrete5, and it took very long time (3~5 minutes). I noticed on the git-filter-branch manual page that there's a "--state-branch", and it seemed quite interesting. So, I asked in git@vger.kernel.org how to use it, and the author of that feature (Ian Campbell) told me to look at some code he wrote. So, I wrote https://github.com/concrete5/incremental-filter-branch which wraps "git filter-branch --filter-state", and it's used in https://github.com/concrete5/core_splitter/blob/99bbbcea1ab90572a04864ccb92327532ab6a42c/bin/splitcore (it not yet in production: Korvin wants to be sure everything works well before adopting it) The time required for the operation dropped from 3~5 minutes to ~10 seconds ;) While writing that script, I noticed a couple of things that could be improved in "git-filter-branch", so I submitted https://marc.info/?l=git&m=152111905428554&w=2 (so that the script could run without problems if there's nothing to do) and https://marc.info/?l=git&m=152147095416175&w=2 (so that --filter-state could be used on Mac and other non-Linux systems). How did I learn how to submit to git? I searched for "git src", landed on https://github.com/git/git, read Documentation/SubmittingPatches and used git send-email. And yes, all that on Windows (and WSL). > > Ciao, > Johannes Ciao! -- Michele ``` ## Johannes Schindelin, 2018-03-23 09:35 Subject: Re: [PATCH v2] filter-branch: use printf instead of echo -e Message-ID: URL: https://gitlist.dev/e/nycvar.QRO.7.76.6.1803231034400.77%40ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz In-Reply-To: <081ccfce-7a89-503e-465e-3befe6f33f17@locati.it> ``` Hi Michele, On Thu, 22 Mar 2018, Michele Locati wrote: > Il 21/03/2018 22:50, Johannes Schindelin ha scritto: > > > > On Mon, 19 Mar 2018, Michele Locati wrote: > > > > > [...] > > > -- > > > 2.16.2.windows.1 > > > > Yaaaaay! > > > > Out of curiosity: did the CONTRIBUTING.md file help that was recently > > turned into a guide how to contribute to Git (for Windows) by Derrick > > Stolee? > > Well, no. Here's what led me here... > > [...] Thank you for taking the time to tell the story! Ciao, Johannes ```