Re: [PATCH] filter-branch: use printf instead of echo -e

From: CB Bailey <charles@hashpling.org>
Date: Mar 19, 2018, 15:39 UTC
Message-ID: <20180319153945.kchupu43cpcbg25n@hashpling.org>
In-Reply-To: <20180319144905.11564-1-michele@locati.it>

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

Show 25 quoted lines

> 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(-)
> 
> 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