workaround for issue 182 (git add -p or git add -i with a subdirectory)

[kkheller]

inspired by pull request 218 using code from @PhilipDavis

[kkheller]

The initial content of this pull request (commit 5fd4faa) has some changes compared to my original patch (original: #218 (comment)).

On the line that called print $fhargs "$_ \n"; there are now quotes wrapping the $_ variable. (This is needed if the $_ string variable—which is a file path and name—contains spaces.)

I used the elsif suggested by @dscho , although on my computer $^O eq 'MSWin32' was insufficient so i also had to check for 'msys' or 'MSWin32'. (Maybe my local installation of msysgit is old or weird?)

Lastly, after testing on 3 different computers, I found that 2 of them needed xargs -s 20000 instead of just plain old xargs

Disclaimer: I am woefully unfamiliar with the source code of git (both the perl code and the c code). This patch was arrived at only because I encountered the older pull request #218 and then desperately poked at it until I could use "git add -p SUBDIR" again (which is by far my most-used git command on any given day).

[kkheller]

Just in case, I will once again note the conditions of the large repo that led me into the "git add -p" failure in the first place:

git add -p SmallFolder/ # succeeds
git add -p FOLDER_WITH_HUGE_CONTENT/ # fails

the statistics for my FOLDER_WITH_HUGE_CONTENT:

find FOLDER_WITH_HUGE_CONTENT/ | wc -l produces a line count of 505.

find FOLDER_WITH_HUGE_CONTENT/ | wc -c produces a char count of 21734.

[kkheller]

To anyone interested: i am happy to add you as a "collaborator" (granted push access) to my forked repo that was forked for the sole purpose of making this pull request. This way others can continue editing this same request. You can add more commits by pushing to the msysgit_issues_182 branch on kkheller/git. Anyone interested: just ask! I will grant. (@dscho is already added as collaborator.)

[buildhive]

MSysGit - the development behind Git for Windows » git #282 SUCCESS
This pull request looks good
(what's this?)

[dscho]

On the line that called print $fhargs "$_ \n"; there are now quotes wrapping the $_ variable. (This is needed if the $_ string variable—which is a file path and name—contains spaces.)

Yep, I think quoting is a good thing, we probably need to handle file names containing quotes, too, though.

How about a completely different strategy, though? Most of Git tries to separate input to xargs via NUL bytes. This is the safest because NUL is not a legal part of any file name. The way you would do this is by calling print $fhargs $_ . "\0"; and passing the -0 parameter to xargs.

BTW I think you also need to pass the -r flag to xargs, i.e. "no run if empty".

And yes, I think you always need -s 20000 because xargs might not necessarily know that Windows' command line length limit is 32,000 (if I remember correctly).

[dscho]

I guess the main two concerns are:

1. do we correctly separate between options and file names?
2. do we need a temporary file, or could we use Open2 to pipe the list of files into, and the output from, xargs -s 20000 -0r git add?

[kkheller]

Thanks, @dscho. These are great notes. I will see if I can address them at some point in the next week or two.

Again, if anyone else wants to jump in before then, post a note here and i'll add collaborators that can push changes to this pull request (current list is just me and @dscho ).

[kkheller]

Three things I did today:

1. minor cleanup in commit bd23048
2. using NUL to separate file arguments, and using the -0 with xargs (commit 717b526)
3. some investigating in regards to the question of absence/presence of: --

@dscho was right: we need to verify that a -- (dash dash) is guaranteed to be present in the arguments to run_cmd_pipe.

Good news: the guarantee appears to be solid. I only see calls to run_cmd_pipe inside the single source file: git-add--interactive.perl , and I have examined each of those call sites in that file.

Wherever run_cmd_pipe is called, the -- is always explicitly part of what is passed in. Calls typically look like this:

run_cmd_pipe(qw(git ls-files --others --exclude-standard --), @ARGV);

There are two exceptions, but that is ok (explanation follows). The two exceptions are:

run_cmd_pipe(qw(git rev-parse --git-dir))
run_cmd_pipe(qw(git var GIT_EDITOR))

Those are ok, because for the code in this patch to be executed, the following has to be true: (scalar @_ > 200) ... and that will never be true for the two calls to run_cmd_pipe that I just listed.

[buildhive]

MSysGit - the development behind Git for Windows » git #283 SUCCESS
This pull request looks good
(what's this?)

[dscho]

Sorry to let this slip for so long!

[dscho]

Moving to git-for-windows#305, as Git for Windows 1.x was retired and enjoys playing with the grand children now.