fix for long command line on git-add-interactive

[albfan]

See #182 for more details
This implementation is not checked. It was created from @PhilipDavis
proposal https://gist.github.com/PhilipDavis/a7e8843dad39493a698c

[buildhive]

MSysGit - the development behind Git for Windows » git #241 FAILURE
Looks like there's a problem with this pull request
(what's this?)

[PhilipDavis]

I didn't create a pull request myself (and deleted my comment on the thread) because I found problems with my code. At first it appeared to be working... but on closer inspection, git was just showing all the files in the repo rather than just the files specified in the path.

e.g. modify a file in a directory, go into a subdirectory and git add -i . from there. The file from the parent directory should not show up (but does).

[PhilipDavis]

After more debugging, it looks like msysgit is translating the "." into an empty string when used in a subdirectory.

e.g.
C:\GitHub\project\src\foo> git add -i .
=> the script calls "git ls-files -- :(prefix:8)src/foo/

As a result, the args file I create ends up getting the names of all files in the repo

[dscho]

@albfan @PhilipDavis I hope you are still interested in resolving this issue? If so, I think the first order of business would be to create a test demonstrating the issue, say, in t3701-add-interactive.sh.

We could then try to fix the "." issue.

In the long run, we would want to avoid the temporary file, either by passing the command-line arguments via stdin or by turning the entire add-interactive script into a builtin (which would make it possible to avoid exec'ing the Git programs and run the relevant functions directly, circumventing the command-line limit)..

[PhilipDavis]

Hey, thanks for following up. I've since moved on to the SourceTree GUI and haven't had a need to go back to the command line.

From what I recall, reproducing the issue would involve creating a repo where the serialization of all the filenames (including directories) exceeds 32K. Then go into a subdirectory and invoke git add -i .

[albfan]

I will try to create the test. Even to fork with the suggested fix and
create a PR.

El vie, 13 de febrero de 2015 00:01, Philip Davis [email protected]
escribió:

[kkheller]

I am really, really grateful to have found this patch. Issue 182 (#182) has been driving me insane for months. I did some google searching, but somehow i never managed to find this until now.

I have taken PhilipDavis's patch and added use of "xargs" to replace the "less-than operator" ("< $filename") that wasn't having the desired outcome.

When I read @PhilipDavis's initial objections about grabbing other people's (unready) code and creating a pull request, i was 100% sympathetic with @PhilipDavis, and I could imagine myself having the exact same reaction.

It turns out, however, that the motto "Trust in git and collaboration" (spoken by @albfan) turns out to be true here! Knowing zero amount of perl means I never would have found the time to know how to compose the code that Philip wrote. But I was able to change one line of it to finally solve #182 on my windows computer!

Both @PhilipDavis and @albfan have my deepest thanks!

Here is my patch (which is mostly Philip's code still). I used the master branch as of dd8b3ba, and did my work on top of that.

From b49c9e3fb3a1e00d1d9ab16601fd89a802f158e1 Mon Sep 17 00:00:00 2001
From: Kelly Heller <[email protected]>
Date: Tue, 26 May 2015 16:52:32 -0700
Subject: [PATCH] edits made on dd8b3ba4f5, workaround for
 https://github.com/msysgit/git/issues/182

---
 git-add--interactive.perl | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 1fadd69..f4ae203 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -178,10 +178,55 @@ sub run_cmd_pipe {
        my @args = map { m/ /o ? "\"$_\"": $_ } @_;
        return qx{@args};
    } else {
+
+       # workaround for https://github.com/msysgit/git/issues/182
+       if ( scalar @_ > 200 ) {
+           # mostly the work of @PhilipDavis, circa 2014-08-07
+
+           my @myArgs = @_;
+           my $cmd = "@myArgs";
+           my $path = "$ENV{APPDATA}";
+           $path =~ s/\\/\//g;
+
+           use File::Temp qw(tempfile);
+           my ($fhargs, $filename) = tempfile("$path/git-args-XXXXXX", UNLINK => 1);
+
+           if (grep $_ eq "--", @myArgs) {
+
+               $cmd = "";
+               while ($myArgs[0] ne '--') {
+                   $cmd = $cmd . shift(@myArgs) . " ";
+               }
+
+               $cmd = $cmd . "-- ";
+               shift(@myArgs);
+
+               foreach (@myArgs) {
+                   print $fhargs "$_  \n";
+               }
+
+               $fhargs->flush;
+
+               # 2015 may 26: @kkheller using cat to xargs instead of "< $filename"
+               $cmd = "cat $filename | xargs " . $cmd;
+           }
+
+           print "$filename\n";
+           print "$cmd \n\n";
+
+           my $fh = undef;
+           open($fh, '-|', $cmd) or die;
+           return <$fh>;
+
+       } # if ( $argsizetest > 200 )
+       else {
+
+       # these three lines were left exactly as they were *prior* to any workaround for issue 182
        my $fh = undef;
        open($fh, '-|', @_) or die;
        return <$fh>;
-   }
+       }
+    }
 }

 my ($GIT_DIR) = run_cmd_pipe(qw(git rev-parse --git-dir));
-- 
1.9.5.msysgit.0

My version of the patch certainly still lacks the necessary quality and polish to become part of mainline. My perl skills are dismal. You can also see that i "improvised" by using a test of whether there are more than 200 args in the array. 200 is just a magic number that seems to work for me. (I didn't want to make every call to "git add -p" go through this new path of execution unless it really was necessary.) I also left print statements in, because I wanted to keep an eye on the output for now.

This works for me on a Windows 7 64 bit computer and git version 1.9.5.msysgit.0

[PhilipDavis]

Cool. Glad you figured it out! :)

[dscho]

@kkheller could you please open your own Pull Request? I would like to suggest inserting the new clause before the existing else and augment it with an additional condition (stolen from here):

   } elsif ($^O eq 'MSWin32' && scalar @_ > 200) {
      ...
   } else {
      ...

[kkheller]

No problem. New pull request made at #354

I used the elsif approach this time, thanks.

After testing my patch on other computers, I also discovered I needed to use xargs with the -s option, too. And add quotes when outputting each path string to the temp file. So the new pull request comes with all those new subtleties.

[dscho]

Moving this Pull Request to git-for-windows#305, as the Git for Windows 1.x project was superseded by Git for Windows 2.x.