{"thread":{"id":"46583","subject":"[PATCH] stash: prevent warning about null bytes in input","startedAt":"2017-08-14T05:09:16Z","lastAt":"2017-08-14T21:44:06Z","messageCount":5,"participants":["Kevin Daudt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"326260","messageId":"20170814050801.7158-1-me@ikke.info","threadId":"46583","inReplyTo":null,"subject":"[PATCH] stash: prevent warning about null bytes in input","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2017-08-14T05:08:01Z","receivedAt":"2017-08-14T05:09:16Z","isPatch":true,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"The no_changes function calls the untracked_files function through\ncommand substitution. untracked_files will return null bytes because it\nruns ls-files with the '-z' option.\n\nBash since version 4.4 warns about these null bytes. As they are not\nrequired for the test that is being done, remove null bytes from the\ninput.\n\nThis warning is triggered when running git stash save -u resulting in\ntwo warnings:\n\n    git-stash: line 43: warning: command substitution: ignored null byte\n    in input\n\nSigned-off-by: Kevin Daudt <me@ikke.info>\n---\n git-stash.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 9b6c2da7b..0dcca3cd6 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -39,7 +39,7 @@ fi\n no_changes () {\n \tgit diff-index --quiet --cached HEAD --ignore-submodules -- \"$@\" &&\n \tgit diff-files --quiet --ignore-submodules -- \"$@\" &&\n-\t(test -z \"$untracked\" || test -z \"$(untracked_files)\")\n+\t(test -z \"$untracked\" || test -z \"$(untracked_files | tr -d '\\0')\")\n }\n \n untracked_files () {\n-- \n2.14.0.rc1.33.g384a8b271c\n\n"},{"id":"326301","messageId":"xmqq7ey6udvl.fsf@gitster.mtv.corp.google.com","threadId":"46583","inReplyTo":"20170814050801.7158-1-me@ikke.info","subject":"Re: [PATCH] stash: prevent warning about null bytes in input","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-14T19:51:26Z","receivedAt":"2017-08-14T19:51:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Daudt <me@ikke.info> writes:\n\n> The no_changes function calls the untracked_files function through\n> command substitution. untracked_files will return null bytes because it\n> runs ls-files with the '-z' option.\n>\n> Bash since version 4.4 warns about these null bytes. As they are not\n> required for the test that is being done, remove null bytes from the\n> input.\n\nThat's an interesting one ;-)\n\nI wonder if you considered giving an option to untracked_files\nhelper function, though.  After all, it has only two callers,\nand it feels a bit suboptimal to ask the command to do a special\nthing (i.e. \"-z\") only to clean it up with a pipe.\n\nIOW, something along the lines of (totally untested)...\n\n git-stash.sh | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 9b6c2da7b4..5f09a47f0a 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -43,9 +43,16 @@ no_changes () {\n }\n \n untracked_files () {\n+\tif test \"$1\" = \"-z\"\n+\tthen\n+\t\tshift\n+\t\tz=-z\n+\telse\n+\t\tz=\n+\tfi\n \texcl_opt=--exclude-standard\n \ttest \"$untracked\" = \"all\" && excl_opt=\n-\tgit ls-files -o -z $excl_opt -- \"$@\"\n+\tgit ls-files -o $z $excl_opt -- \"$@\"\n }\n \n clear_stash () {\n@@ -114,7 +121,7 @@ create_stash () {\n \t\t# Untracked files are stored by themselves in a parentless commit, for\n \t\t# ease of unpacking later.\n \t\tu_commit=$(\n-\t\t\tuntracked_files \"$@\" | (\n+\t\t\tuntracked_files -z \"$@\" | (\n \t\t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n \t\t\t\texport GIT_INDEX_FILE &&\n \t\t\t\trm -f \"$TMPindex\" &&\n\n\n"},{"id":"326307","messageId":"20170814203246.GA3839@alpha.vpn.ikke.info","threadId":"46583","inReplyTo":"xmqq7ey6udvl.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] stash: prevent warning about null bytes in input","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2017-08-14T20:32:46Z","receivedAt":"2017-08-14T20:32:52Z","isPatch":true,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"On Mon, Aug 14, 2017 at 12:51:26PM -0700, Junio C Hamano wrote:\n> Kevin Daudt <me@ikke.info> writes:\n> \n> > The no_changes function calls the untracked_files function through\n> > command substitution. untracked_files will return null bytes because it\n> > runs ls-files with the '-z' option.\n> >\n> > Bash since version 4.4 warns about these null bytes. As they are not\n> > required for the test that is being done, remove null bytes from the\n> > input.\n> \n> That's an interesting one ;-)\n> \n> I wonder if you considered giving an option to untracked_files\n> helper function, though.  After all, it has only two callers,\n> and it feels a bit suboptimal to ask the command to do a special\n> thing (i.e. \"-z\") only to clean it up with a pipe.\n\nAs a matter of fact, I did not consider that option. I do agree that's a\nmuch better approach.\n\n> \n> IOW, something along the lines of (totally untested)...\n> \n\nHow should I proceed with this? Resubmit it after testing with the\nappropriate attribution?\n\n\n>  git-stash.sh | 11 +++++++++--\n>  1 file changed, 9 insertions(+), 2 deletions(-)\n> \n> diff --git a/git-stash.sh b/git-stash.sh\n> index 9b6c2da7b4..5f09a47f0a 100755\n> --- a/git-stash.sh\n> +++ b/git-stash.sh\n> @@ -43,9 +43,16 @@ no_changes () {\n>  }\n>  \n>  untracked_files () {\n> +\tif test \"$1\" = \"-z\"\n> +\tthen\n> +\t\tshift\n> +\t\tz=-z\n> +\telse\n> +\t\tz=\n> +\tfi\n>  \texcl_opt=--exclude-standard\n>  \ttest \"$untracked\" = \"all\" && excl_opt=\n> -\tgit ls-files -o -z $excl_opt -- \"$@\"\n> +\tgit ls-files -o $z $excl_opt -- \"$@\"\n>  }\n>  \n>  clear_stash () {\n> @@ -114,7 +121,7 @@ create_stash () {\n>  \t\t# Untracked files are stored by themselves in a parentless commit, for\n>  \t\t# ease of unpacking later.\n>  \t\tu_commit=$(\n> -\t\t\tuntracked_files \"$@\" | (\n> +\t\t\tuntracked_files -z \"$@\" | (\n>  \t\t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n>  \t\t\t\texport GIT_INDEX_FILE &&\n>  \t\t\t\trm -f \"$TMPindex\" &&\n> \n> \n"},{"id":"326310","messageId":"xmqqpobxubrh.fsf@gitster.mtv.corp.google.com","threadId":"46583","inReplyTo":"20170814203246.GA3839@alpha.vpn.ikke.info","subject":"Re: [PATCH] stash: prevent warning about null bytes in input","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-14T20:37:06Z","receivedAt":"2017-08-14T20:37:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kevin Daudt <me@ikke.info> writes:\n\n> On Mon, Aug 14, 2017 at 12:51:26PM -0700, Junio C Hamano wrote:\n>> Kevin Daudt <me@ikke.info> writes:\n>> \n>> > The no_changes function calls the untracked_files function through\n>> > command substitution. untracked_files will return null bytes because it\n>> > runs ls-files with the '-z' option.\n>> >\n>> > Bash since version 4.4 warns about these null bytes. As they are not\n>> > required for the test that is being done, remove null bytes from the\n>> > input.\n>> \n>> That's an interesting one ;-)\n>> \n>> I wonder if you considered giving an option to untracked_files\n>> helper function, though.  After all, it has only two callers,\n>> and it feels a bit suboptimal to ask the command to do a special\n>> thing (i.e. \"-z\") only to clean it up with a pipe.\n>\n> As a matter of fact, I did not consider that option. I do agree that's a\n> much better approach.\n>\n>> \n>> IOW, something along the lines of (totally untested)...\n>> \n>\n> How should I proceed with this? Resubmit it after testing with the\n> appropriate attribution?\n\nSure.  \n\nAn appropriate attribution would be a \"Helped-by: me\" at most, but I\ndo not think for something this small it may not even bee needed.\n\n>\n>>  git-stash.sh | 11 +++++++++--\n>>  1 file changed, 9 insertions(+), 2 deletions(-)\n>> \n>> diff --git a/git-stash.sh b/git-stash.sh\n>> index 9b6c2da7b4..5f09a47f0a 100755\n>> --- a/git-stash.sh\n>> +++ b/git-stash.sh\n>> @@ -43,9 +43,16 @@ no_changes () {\n>>  }\n>>  \n>>  untracked_files () {\n>> +\tif test \"$1\" = \"-z\"\n>> +\tthen\n>> +\t\tshift\n>> +\t\tz=-z\n>> +\telse\n>> +\t\tz=\n>> +\tfi\n>>  \texcl_opt=--exclude-standard\n>>  \ttest \"$untracked\" = \"all\" && excl_opt=\n>> -\tgit ls-files -o -z $excl_opt -- \"$@\"\n>> +\tgit ls-files -o $z $excl_opt -- \"$@\"\n>>  }\n>>  \n>>  clear_stash () {\n>> @@ -114,7 +121,7 @@ create_stash () {\n>>  \t\t# Untracked files are stored by themselves in a parentless commit, for\n>>  \t\t# ease of unpacking later.\n>>  \t\tu_commit=$(\n>> -\t\t\tuntracked_files \"$@\" | (\n>> +\t\t\tuntracked_files -z \"$@\" | (\n>>  \t\t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n>>  \t\t\t\texport GIT_INDEX_FILE &&\n>>  \t\t\t\trm -f \"$TMPindex\" &&\n>> \n>> \n"},{"id":"326320","messageId":"20170814214333.12789-1-me@ikke.info","threadId":"46583","inReplyTo":"20170814050801.7158-1-me@ikke.info","subject":"[PATCH v2] stash: prevent warning about null bytes in input","fromName":"Kevin Daudt","fromEmail":"me@ikke.info","sentAt":"2017-08-14T21:43:33Z","receivedAt":"2017-08-14T21:44:06Z","isPatch":true,"sender":{"key":"me@ikke.info","avatar":"https://avatars.githubusercontent.com/u/135698?v=4"},"body":"The `no_changes` function calls the `untracked_files` function through\ncommand substitution. `untracked_files` will return null bytes because it\nruns ls-files with the '-z' option.\n\nBash since version 4.4 warns about these null bytes. As they are not\nrequired for the test that is being done, make sure `untracked_files`\ndoes not output null bytes when not required.\n\nThis is achieved by adding a parameter to the `untracked_files` function to\nspecify wither `-z` should be passed to ls-files or not.\n\nThis warning is triggered when running git stash save -u resulting in\ntwo warnings:\n\n    git-stash: line 43: warning: command substitution: ignored null byte\n    in input\n\nSigned-off-by: Kevin Daudt <me@ikke.info>\n---\n git-stash.sh | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 9b6c2da7b..5f09a47f0 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -43,9 +43,16 @@ no_changes () {\n }\n \n untracked_files () {\n+\tif test \"$1\" = \"-z\"\n+\tthen\n+\t\tshift\n+\t\tz=-z\n+\telse\n+\t\tz=\n+\tfi\n \texcl_opt=--exclude-standard\n \ttest \"$untracked\" = \"all\" && excl_opt=\n-\tgit ls-files -o -z $excl_opt -- \"$@\"\n+\tgit ls-files -o $z $excl_opt -- \"$@\"\n }\n \n clear_stash () {\n@@ -114,7 +121,7 @@ create_stash () {\n \t\t# Untracked files are stored by themselves in a parentless commit, for\n \t\t# ease of unpacking later.\n \t\tu_commit=$(\n-\t\t\tuntracked_files \"$@\" | (\n+\t\t\tuntracked_files -z \"$@\" | (\n \t\t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n \t\t\t\texport GIT_INDEX_FILE &&\n \t\t\t\trm -f \"$TMPindex\" &&\n-- \n2.14.1.145.gb3622a4ee9\n\n"}]}