[PATCH] stash: prevent warning about null bytes in input

[PATCH] stash: prevent warning about null bytes in input

[Kevin Daudt]

The no_changes function calls the untracked_files function through
command substitution. untracked_files will return null bytes because it
runs ls-files with the '-z' option.

Bash since version 4.4 warns about these null bytes. As they are not
required for the test that is being done, remove null bytes from the
input.

This warning is triggered when running git stash save -u resulting in
two warnings:

    git-stash: line 43: warning: command substitution: ignored null byte
    in input

Signed-off-by: Kevin Daudt <me@ikke.info>
---
 git-stash.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-stash.sh b/git-stash.sh
index 9b6c2da7b..0dcca3cd6 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -39,7 +39,7 @@ fi
 no_changes () {
 	git diff-index --quiet --cached HEAD --ignore-submodules -- "$@" &&
 	git diff-files --quiet --ignore-submodules -- "$@" &&
-	(test -z "$untracked" || test -z "$(untracked_files)")
+	(test -z "$untracked" || test -z "$(untracked_files | tr -d '\0')")
 }
 
 untracked_files () {
-- 
2.14.0.rc1.33.g384a8b271c

Re: [PATCH] stash: prevent warning about null bytes in input

[Junio C Hamano]

Kevin Daudt <me@ikke.info> writes:

> The no_changes function calls the untracked_files function through
> command substitution. untracked_files will return null bytes because it
> runs ls-files with the '-z' option.
>
> Bash since version 4.4 warns about these null bytes. As they are not
> required for the test that is being done, remove null bytes from the
> input.

That's an interesting one ;-)

I wonder if you considered giving an option to untracked_files
helper function, though.  After all, it has only two callers,
and it feels a bit suboptimal to ask the command to do a special
thing (i.e. "-z") only to clean it up with a pipe.

IOW, something along the lines of (totally untested)...

 git-stash.sh | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 9b6c2da7b4..5f09a47f0a 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -43,9 +43,16 @@ no_changes () {
 }
 
 untracked_files () {
+	if test "$1" = "-z"
+	then
+		shift
+		z=-z
+	else
+		z=
+	fi
 	excl_opt=--exclude-standard
 	test "$untracked" = "all" && excl_opt=
-	git ls-files -o -z $excl_opt -- "$@"
+	git ls-files -o $z $excl_opt -- "$@"
 }
 
 clear_stash () {
@@ -114,7 +121,7 @@ create_stash () {
 		# Untracked files are stored by themselves in a parentless commit, for
 		# ease of unpacking later.
 		u_commit=$(
-			untracked_files "$@" | (
+			untracked_files -z "$@" | (
 				GIT_INDEX_FILE="$TMPindex" &&
 				export GIT_INDEX_FILE &&
 				rm -f "$TMPindex" &&

Re: [PATCH] stash: prevent warning about null bytes in input

[Kevin Daudt]

On Mon, Aug 14, 2017 at 12:51:26PM -0700, Junio C Hamano wrote:
> Kevin Daudt <me@ikke.info> writes:
> 
> > The no_changes function calls the untracked_files function through
> > command substitution. untracked_files will return null bytes because it
> > runs ls-files with the '-z' option.
> >
> > Bash since version 4.4 warns about these null bytes. As they are not
> > required for the test that is being done, remove null bytes from the
> > input.
> 
> That's an interesting one ;-)
> 
> I wonder if you considered giving an option to untracked_files
> helper function, though.  After all, it has only two callers,
> and it feels a bit suboptimal to ask the command to do a special
> thing (i.e. "-z") only to clean it up with a pipe.

As a matter of fact, I did not consider that option. I do agree that's a
much better approach.

> 
> IOW, something along the lines of (totally untested)...
> 

How should I proceed with this? Resubmit it after testing with the
appropriate attribution?


>  git-stash.sh | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/git-stash.sh b/git-stash.sh
> index 9b6c2da7b4..5f09a47f0a 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -43,9 +43,16 @@ no_changes () {
>  }
>  
>  untracked_files () {
> +	if test "$1" = "-z"
> +	then
> +		shift
> +		z=-z
> +	else
> +		z=
> +	fi
>  	excl_opt=--exclude-standard
>  	test "$untracked" = "all" && excl_opt=
> -	git ls-files -o -z $excl_opt -- "$@"
> +	git ls-files -o $z $excl_opt -- "$@"
>  }
>  
>  clear_stash () {
> @@ -114,7 +121,7 @@ create_stash () {
>  		# Untracked files are stored by themselves in a parentless commit, for
>  		# ease of unpacking later.
>  		u_commit=$(
> -			untracked_files "$@" | (
> +			untracked_files -z "$@" | (
>  				GIT_INDEX_FILE="$TMPindex" &&
>  				export GIT_INDEX_FILE &&
>  				rm -f "$TMPindex" &&
> 
> 

Re: [PATCH] stash: prevent warning about null bytes in input

[Junio C Hamano]

Kevin Daudt <me@ikke.info> writes:

> On Mon, Aug 14, 2017 at 12:51:26PM -0700, Junio C Hamano wrote:
>> Kevin Daudt <me@ikke.info> writes:
>> 
>> > The no_changes function calls the untracked_files function through
>> > command substitution. untracked_files will return null bytes because it
>> > runs ls-files with the '-z' option.
>> >
>> > Bash since version 4.4 warns about these null bytes. As they are not
>> > required for the test that is being done, remove null bytes from the
>> > input.
>> 
>> That's an interesting one ;-)
>> 
>> I wonder if you considered giving an option to untracked_files
>> helper function, though.  After all, it has only two callers,
>> and it feels a bit suboptimal to ask the command to do a special
>> thing (i.e. "-z") only to clean it up with a pipe.
>
> As a matter of fact, I did not consider that option. I do agree that's a
> much better approach.
>
>> 
>> IOW, something along the lines of (totally untested)...
>> 
>
> How should I proceed with this? Resubmit it after testing with the
> appropriate attribution?

Sure.  

An appropriate attribution would be a "Helped-by: me" at most, but I
do not think for something this small it may not even bee needed.

>
>>  git-stash.sh | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>> 
>> diff --git a/git-stash.sh b/git-stash.sh
>> index 9b6c2da7b4..5f09a47f0a 100755
>> --- a/git-stash.sh
>> +++ b/git-stash.sh
>> @@ -43,9 +43,16 @@ no_changes () {
>>  }
>>  
>>  untracked_files () {
>> +	if test "$1" = "-z"
>> +	then
>> +		shift
>> +		z=-z
>> +	else
>> +		z=
>> +	fi
>>  	excl_opt=--exclude-standard
>>  	test "$untracked" = "all" && excl_opt=
>> -	git ls-files -o -z $excl_opt -- "$@"
>> +	git ls-files -o $z $excl_opt -- "$@"
>>  }
>>  
>>  clear_stash () {
>> @@ -114,7 +121,7 @@ create_stash () {
>>  		# Untracked files are stored by themselves in a parentless commit, for
>>  		# ease of unpacking later.
>>  		u_commit=$(
>> -			untracked_files "$@" | (
>> +			untracked_files -z "$@" | (
>>  				GIT_INDEX_FILE="$TMPindex" &&
>>  				export GIT_INDEX_FILE &&
>>  				rm -f "$TMPindex" &&
>> 
>> 

[PATCH v2] stash: prevent warning about null bytes in input

[Kevin Daudt]

The `no_changes` function calls the `untracked_files` function through
command substitution. `untracked_files` will return null bytes because it
runs ls-files with the '-z' option.

Bash since version 4.4 warns about these null bytes. As they are not
required for the test that is being done, make sure `untracked_files`
does not output null bytes when not required.

This is achieved by adding a parameter to the `untracked_files` function to
specify wither `-z` should be passed to ls-files or not.

This warning is triggered when running git stash save -u resulting in
two warnings:

    git-stash: line 43: warning: command substitution: ignored null byte
    in input

Signed-off-by: Kevin Daudt <me@ikke.info>
---
 git-stash.sh | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 9b6c2da7b..5f09a47f0 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -43,9 +43,16 @@ no_changes () {
 }
 
 untracked_files () {
+	if test "$1" = "-z"
+	then
+		shift
+		z=-z
+	else
+		z=
+	fi
 	excl_opt=--exclude-standard
 	test "$untracked" = "all" && excl_opt=
-	git ls-files -o -z $excl_opt -- "$@"
+	git ls-files -o $z $excl_opt -- "$@"
 }
 
 clear_stash () {
@@ -114,7 +121,7 @@ create_stash () {
 		# Untracked files are stored by themselves in a parentless commit, for
 		# ease of unpacking later.
 		u_commit=$(
-			untracked_files "$@" | (
+			untracked_files -z "$@" | (
 				GIT_INDEX_FILE="$TMPindex" &&
 				export GIT_INDEX_FILE &&
 				rm -f "$TMPindex" &&
-- 
2.14.1.145.gb3622a4ee9