Re: [PATCH] contrib/subtree: fix linefeeds trimming for cmd_split()
From: Danny Lin <hidden>
Date: 2016-06-15 23:04:40
Replace all echo using printf for better portability. Also re-wrap previous 'say -n "$str<CR>"' using a new function state() so to prevent CR chars included in the source code, which could be mal-processed on some shells (e.g. MsysGit trims CR before executing a shell script file in order to make it work right on Windows even if it uses CRLF as linefeeds. Signed-off-by: Danny Lin <redacted> --- contrib/subtree/git-subtree.sh | 56 +++++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 22 deletions(-)
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index fa1a583..2da1433 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh@@ -29,7 +29,7 @@ rejoin merge the new branch back into HEAD options for 'add', 'merge', 'pull' and 'push' squash merge subtree changes as a single commit " -eval "$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)" +eval "$(printf %s "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" ||
printf %s "exit $?")" PATH=$PATH:$(git --exec-path) . git-sh-setup
@@ -51,17 +51,29 @@ prefix= debug() { if [ -n "$debug" ]; then - echo "$@" >&2 + printf "%s\n" "$*" >&2 fi } say() { if [ -z "$quiet" ]; then - echo "$@" >&2 + printf "%s\n" "$*" >&2 fi } +state() +{ + if [ -z "$quiet" ]; then + printf "%s\r" "$*" >&2 + fi +} + +log() +{ + printf "%s\n" "$*" +} + assert() { if "$@"; then
@@ -72,7 +84,7 @@ assert() } -#echo "Options: $*" +#log "Options: $*" while [ $# -gt 0 ]; do opt="$1"
@@ -149,7 +161,7 @@ cache_get() for oldrev in $*; do if [ -r "$cachedir/$oldrev" ]; then read newrev <"$cachedir/$oldrev" - echo $newrev + log $newrev fi done }
@@ -158,7 +170,7 @@ cache_miss() { for oldrev in $*; do if [ ! -r "$cachedir/$oldrev" ]; then - echo $oldrev + log $oldrev fi done }
@@ -175,7 +187,7 @@ check_parents() set_notree() { - echo "1" > "$cachedir/notree/$1" + log "1" > "$cachedir/notree/$1" } cache_set()
@@ -187,7 +199,7 @@ cache_set() -a -e "$cachedir/$oldrev" ]; then die "cache for $oldrev already exists!" fi - echo "$newrev" >"$cachedir/$oldrev" + log "$newrev" >"$cachedir/$oldrev" } rev_exists()
@@ -219,7 +231,7 @@ rev_is_descendant_of_branch() try_remove_previous() { if rev_exists "$1^"; then - echo "^$1^" + log "^$1^" fi }
@@ -247,7 +259,7 @@ find_latest_squash() sq="$sub" fi debug "Squash found: $sq $sub" - echo "$sq" "$sub" + log "$sq" "$sub" break fi sq=
@@ -339,9 +351,9 @@ add_msg() add_squashed_msg() { if [ -n "$message" ]; then - echo "$message" + log "$message" else - echo "Merge commit '$1' as '$2'" + log "Merge commit '$1' as '$2'" fi }
@@ -373,17 +385,17 @@ squash_msg() if [ -n "$oldsub" ]; then oldsub_short=$(git rev-parse --short "$oldsub") - echo "Squashed '$dir/' changes from $oldsub_short..$newsub_short" + log "Squashed '$dir/' changes from $oldsub_short..$newsub_short" echo git log --pretty=tformat:'%h %s' "$oldsub..$newsub" git log --pretty=tformat:'REVERT: %h %s' "$newsub..$oldsub" else - echo "Squashed '$dir/' content from commit $newsub_short" + log "Squashed '$dir/' content from commit $newsub_short" fi echo - echo "git-subtree-dir: $dir" - echo "git-subtree-split: $newsub" + log "git-subtree-dir: $dir" + log "git-subtree-split: $newsub" } toptree_for_commit()
@@ -401,7 +413,7 @@ subtree_for_commit() assert [ "$name" = "$dir" ] assert [ "$type" = "tree" -o "$type" = "commit" ] [ "$type" = "commit" ] && continue # ignore submodules - echo $tree + log $tree break done }
@@ -474,7 +486,7 @@ copy_or_skip() done if [ -n "$identical" ]; then - echo $identical + log $identical else copy_commit $rev $tree "$p" || exit $? fi
@@ -526,7 +538,7 @@ cmd_add() cmd_add_repository() { - echo "git fetch" "$@" + log "git fetch" "$@" repository=$1 refspec=$2 git fetch "$@" || exit $?
@@ -599,7 +611,7 @@ cmd_split() eval "$grl" | while read rev parents; do revcount=$(($revcount + 1)) - say -n "$revcount/$revmax ($createcount)
"
+ state "$revcount/$revmax ($createcount)"
debug "Processing commit: $rev"
exists=$(cache_get $rev)
if [ -n "$exists" ]; then@@ -656,7 +668,7 @@ cmd_split() git update-ref -m 'subtree split' "refs/heads/$branch"
$latest_new || exit $?
say "$action branch '$branch'"
fi
- echo $latest_new
+ log $latest_new
exit 0
}
@@ -726,7 +738,7 @@ cmd_push() if [ -e "$dir" ]; then repository=$1 refspec=$2 - echo "git push using: " $repository $refspec + log "git push using: " $repository $refspec localrev=$(git subtree split --prefix="$prefix") || die git push $repository $localrev:refs/heads/$refspec else
--
2.3.7.windows.1
2015-05-07 3:58 GMT+08:00 Eric Sunshine <sunshine@sunshineco.com>:
> On Wed, May 6, 2015 at 3:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Danny Lin <danny0838@gmail.com> writes:
>>
>>> cmd_split() prints a CR char by assigning a variable
>>> with a literal CR in the source code, which could be
>>> trimmed or mis-processed in some terminals. Replace
>>> with $(printf '\r') to fix it.
>
> For future readers of the patch who haven't followed the email
> discussion, it might be a good idea to explain the problem in more
> detail. Saying merely "could be trimmed or mis-processed in some
> terminals" doesn't give much for people to latch onto if they want to
> understand the specific problem. Concrete information would help.
>
Added related information.
>>> Signed-off-by: Danny Lin <danny0838@gmail.com>
>>> ---
>>> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
>>> index fa1a583..3a581fc 100755
>>> --- a/contrib/subtree/git-subtree.sh
>>> +++ b/contrib/subtree/git-subtree.sh
>>> @@ -596,10 +596,11 @@ cmd_split()
>>> revmax=$(eval "$grl" | wc -l)
>>> revcount=0
>>> createcount=0
>>> + CR=$(printf '\r')
>>> eval "$grl" |
>>> while read rev parents; do
>>> revcount=$(($revcount + 1))
>>> - say -n "$revcount/$revmax ($createcount)
>>> "
>>> + say -n "$revcount/$revmax ($createcount)$CR"
>>
>> Interesting. I would have expected, especially this is a portability-fix
>> change, that the change would be a single liner
>>
>> - say -n ...
>> + printf "%s\r" "$revcount/$revmax ($createcount)"
>>
>> that does not touch any other line.
>
> Unfortunately, that solution does not respect the $quiet flag like
> say() does. I had envisioned the patch as reimplementing say() using
> printf rather than echo, and having say() itself either recognizing
> the -n flag or just update callers to specify \n when they want it
> (which is probably the cleaner of the two approaches).
>
If a more thorough portability fix is desired, I'd prefer a work like this
(see the patch above).