{"thread":{"id":"44656","subject":"[BUG] regarding `git add -p` and files containing wildcard characters","startedAt":"2016-12-09T01:47:16Z","lastAt":"2016-12-09T05:37:59Z","messageCount":2,"participants":["unixway.drive@gmail.com","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"307323","messageId":"c9876671-6252-5dfa-18df-a6719dc6834c@gmail.com","threadId":"44656","inReplyTo":null,"subject":"[BUG] regarding `git add -p` and files containing wildcard characters","fromName":"","fromEmail":"unixway.drive@gmail.com","sentAt":"2016-12-09T01:46:49Z","receivedAt":"2016-12-09T01:47:16Z","isPatch":false,"sender":{"key":"unixway.drive@gmail.com","avatar":null},"body":"`git add -p` behaves incorrectly if modified file contains any wildcard \ncharacter. Consider 'random.diff' (attached). For this, interactive \n'add' would first ask to add hunk with two diff headers (or with some \n\"random\" header at the end):\n\n     $ git add -p\n     diff --git a/Random * b/Random *\n     index 01e79c3..01f03ff 100644\n     --- a/Random *\n     +++ b/Random *\n     @@ -1,3 +1,5 @@\n      1\n     +  a\n      2\n     +  b\n      3\n     diff --git a/Random Crap b/Random Crap\n     index 01e79c3..b4373d5 100644\n     --- a/Random Crap\n     +++ b/Random Crap\n     Stage this hunk [y,n,q,a,d,/,j,J,g,s,e,?]? y\n\nThen it will ask for some irrelevant hunk:\n\n     @@ -1,3 +1,5 @@\n      1\n     +  whoa\n      2\n     +  dude\n      3\n     Stage this hunk [y,n,q,a,d,/,K,g,s,e,?]? y\n\n…and, if confirmed, apply it to the wildcard-named file instead of what \nwas confirmed for staging before:\n\n     $ git diff --cached\n     diff --git a/Random * b/Random *\n     index 01e79c3..283ac91 100644\n     --- a/Random *\n     +++ b/Random *\n     @@ -1,3 +1,5 @@\n      1\n     +  whoa\n      2\n     +  dude\n      3\n\nThen one can just repeat this over and over again (unless safely named \nfiles are resolved first):\n\n     $ git add -p\n     diff --git a/Random * b/Random *\n     index 283ac91..04c92f1 100644\n     --- a/Random *\t\n     +++ b/Random *\t\n     @@ -1,5 +1,5 @@\n      1\n     -  whoa\n     +  a\n      2\n     -  dude\n     +  b\n      3\n     diff --git a/Random Crap b/Random Crap\n     index 01e79c3..283ac91 100644\n     --- a/Random Crap\t\n     +++ b/Random Crap\t\n     Stage this hunk [y,n,q,a,d,/,j,J,g,s,e,?]?\n\nThe problem is that `git-diff-files` does some globbing on the 'path' \narguments on its own and has no option to disable that (and \n`git-add--interactive`'s `run_cmd_pipe` already handles all other sorts \nof unsafe characters like spaces and backticks well).\n\n     $ strace -f -e trace=execve git add -p\n     …\n     [pid  1713] execve(\"/usr/lib/git-core/git\",\n         [\"git\", \"diff-files\", \"-p\", \"--\", \"Random *\"],\n     [/* 36 vars */]) = 0\n     …\n     $ git diff-files -- 'Random *'\n     :100644 100644 … … M\tRandom *\n     :100644 100644 … … M\tRandom Crap\n\nFor all the eager people (although I doubt if anybody else is lazy \nenough to not bother with file names like these), this could be easily \nworked around like this:\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex ee3d812..358d877 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -2,3 +2,3 @@\n\n-use 5.008;\n+use 5.014;\n  use strict;\n@@ -761,3 +761,5 @@ sub parse_diff {\n         }\n-       my @diff = run_cmd_pipe(\"git\", @diff_cmd, \"--\", $path);\n+       my @diff = run_cmd_pipe(\"git\", @diff_cmd, \"--\", (\n+               $path =~ s#[\\[*?]#\\\\$&#gr\n+       ));\n         my @colored = ();\n\n(Although a `git-diff-files` option is clearly a better solution that \nwould cover tools other than `git-add--interactive` as well.)\n\n\ndiff --git a/Random * b/Random *\nindex 01e79c3..04c92f1 100644\n--- a/Random *\t\n+++ b/Random *\t\n@@ -1,3 +1,5 @@\n 1\n+  a\n 2\n+  b\n 3\ndiff --git a/Random Crap b/Random Crap\nindex 01e79c3..283ac91 100644\n--- a/Random Crap\t\n+++ b/Random Crap\t\n@@ -1,3 +1,5 @@\n 1\n+  whoa\n 2\n+  dude\n 3\n"},{"id":"307327","messageId":"20161209053751.dpq3brm6pih3rc3n@sigill.intra.peff.net","threadId":"44656","inReplyTo":"c9876671-6252-5dfa-18df-a6719dc6834c@gmail.com","subject":"Re: [BUG] regarding `git add -p` and files containing wildcard characters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-09T05:37:51Z","receivedAt":"2016-12-09T05:37:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 09, 2016 at 04:46:49AM +0300, unixway.drive@gmail.com wrote:\n\n> The problem is that `git-diff-files` does some globbing on the 'path'\n> arguments on its own and has no option to disable that (and\n> `git-add--interactive`'s `run_cmd_pipe` already handles all other sorts of\n> unsafe characters like spaces and backticks well).\n\nI think the option you're looking for is:\n\n  git --literal-pathspecs diff-files ... -- 'Random *'\n\nI don't know if there are other commands run by add--interactive that\nwould want the same treatment. It might actually make sense to set\nGIT_LITERAL_PATHSPECS=1 in the environment right after it expands the\nlist via ls-files.\n\n> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n> index ee3d812..358d877 100755\n> --- a/git-add--interactive.perl\n> +++ b/git-add--interactive.perl\n> @@ -2,3 +2,3 @@\n> \n> -use 5.008;\n> +use 5.014;\n>  use strict;\n> @@ -761,3 +761,5 @@ sub parse_diff {\n>         }\n> -       my @diff = run_cmd_pipe(\"git\", @diff_cmd, \"--\", $path);\n> +       my @diff = run_cmd_pipe(\"git\", @diff_cmd, \"--\", (\n> +               $path =~ s#[\\[*?]#\\\\$&#gr\n> +       ));\n\nThis callsite covers \"-p\". It looks like list_modified() probably needs\nsimilar treatment. Maybe others; I didn't look exhaustively.\n\n-Peff\n"}]}