patchfilter-branch: use printf instead of echo -e

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 <michele@locati.it>
---
 git-filter-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 <michele@locati.it>
---
 git-filter-branch.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Michele Locati <michele@locati.it> writes:

Thanks; will queue.

Hi Michele,

On Mon, 19 Mar 2018, Michele Locati wrote:

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

Il 21/03/2018 22:50, Johannes Schindelin ha scritto:

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!

-- Michele

Hi Michele,

On Thu, 22 Mar 2018, Michele Locati wrote:

Thank you for taking the time to tell the story!

Ciao, Johannes

On Mon, Mar 19, 2018 at 03:49:05PM +0100, Michele Locati wrote:

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

On Mon, Mar 19, 2018 at 03:39:46PM +0000, CB Bailey wrote:

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

On Tue, 2018-03-20 at 00:22 -0400, Jeff King wrote:

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.

Il 20/03/2018 10:53, Ian Campbell ha scritto:

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