On Fri, Sep 24, 2010 at 11:22, Tor Arntsen <tor@xxxxxxxxxxx> wrote: > On Fri, Sep 24, 2010 at 13:05, Ãvar ArnfjÃrà Bjarmason <avarab@xxxxxxxxx> wrote: >> On Fri, Sep 24, 2010 at 10:27, Tor Arntsen <tor@xxxxxxxxxxx> wrote: > [..] >>> I've found that for add -p you'll need 5.8.x or newer, due to stuff like >>> >>>        Âmy $fh = undef; >>>        Âopen($fh, '-|', @_) or die; >>> >>> which fails in e.g. perl 5.6. >>> There could be some other stuff (in addition to add -p) that also does >>> this kind of thing. >> >> If that's the case (I don't have a 5.6 here to do archeology on) then >> git add -p never worked in 5.6. That was added in 5cde71d6 when it was >> introduced in 2006: >> >>  Â+sub run_cmd_pipe { >>  Â+    my $fh = undef; >>  Â+    open($fh, '-|', @_) or die; >>  Â+    return <$fh>; >>  Â+} >> >> Can you show us the specific error you're getting, and the output of >> your `perl -V` ? > > I don't have that particular installation anymore (I installed perl > 5.8 on the machine I had trouble with), and the only other system I > have left with perl 5.6 only has an old Git 1.5 version. But it's easy > enough to reproduce, Perl 5.6 simply doesn't support that notation. > Put the code above in a perl script and execute it: > > Can't use an undefined value as filehandle reference at test-pl.pl line 5. > (that's the 'open' line) > > I can provide the output of -V if you wish, but I don't think it > matters really, except for the version: It matters because you keep saying "5.6", which is ambiguous since the syntax in question was introduced in 5.6.1. The error you may be having on 5.6.0 is very different from 5.6.1, and 5.6.2 has additional bugfixes: http://search.cpan.org/~jesse/perl-5.12.2/pod/perl561delta.pod#open()_with_more_than_two_arguments Anyway, I compiled maint-5.6 from perl.git and confirmed this: $ /home/avar/perl5/installed/bin/perl5.6.2 -Mdiagnostics -le 'print $]; sub run_cmd_pipe { my $fh = undef; open($fh, "-|", @_) or die; return <$fh>; } print run_cmd_pipe(qw(git --help));' 5.006002 Can't use an undefined value as filehandle reference at -e line 1 (#1) (F) A value used as either a hard reference or a symbolic reference must be a defined value. This helps to delurk some insidious errors. Uncaught exception from user code: Can't use an undefined value as filehandle reference at -e line 1. main::run_cmd_pipe('git', '--help') called at -e line 1 As compared to a more recent perl: $ perl -Mdiagnostics -le 'print $]; sub run_cmd_pipe { my $fh = undef; open($fh, "-|", @_) or die; return <$fh>; } print run_cmd_pipe(qw(git --help));' 5.013004 usage: git [--version] [--exec-path[=GIT_EXEC_PATH]] [--html-path] [...] And to reply to Alex Riesen I can't get this to work with `my $fh` (which would be undef too) or glob open either. Although it should be possible with other types of pipe open, or even using temporary files. $ /home/avar/perl5/installed/bin/perl5.6.2 -Mdiagnostics -le 'print $]; sub run_cmd_pipe { local *TMP; open(TMP, "-|", @_) or die; return <TMP>; } print run_cmd_pipe(qw(git --help));' 5.006002 Can't use an undefined value as filehandle reference at -e line 1 (#1) (F) A value used as either a hard reference or a symbolic reference must be a defined value. This helps to delurk some insidious errors. Uncaught exception from user code: Can't use an undefined value as filehandle reference at -e line 1. main::run_cmd_pipe('git', '--help') called at -e line 1 However, I'd like to shift the discussion a bit: Do we want to support the 5.6 line *at all* anymore? I don't think so. As you point out yourself you can just compile 5.8 or later on these machines. I just tried running our test suite with maint-5.6 after compiling with: NO_PERL_MAKEMAKER=1 PERL_PATH=/home/avar/perl5/installed/bin/perl5.6.2 prefix=/opt/git/perl-56 all install NO_PERL_MAKEMAKER=1 is needed because the ExtUtils::MakeMaker that comes with 5.6.2 doesn't know how to move perl/private-Error.pm to perl/blib/*/Error.pm. So if you don't provide it you'll get: $ /opt/git/perl-56/bin/git add -p Can't locate Error.pm That would also break e.g. git-send-email. Here are the test results *without* NO_PERL_MAKEMAKER=1, for reference: Test Summary Report ------------------- t2016-checkout-patch.sh (Wstat: 256 Tests: 14 Failed: 12) Failed tests: 2-13 Non-zero exit status: 1 t3701-add-interactive.sh (Wstat: 256 Tests: 33 Failed: 17) Failed tests: 2, 4-5, 7, 9-10, 13, 16, 18, 21-25, 29 31, 33 Non-zero exit status: 1 t3904-stash-patch.sh (Wstat: 256 Tests: 5 Failed: 2) Failed tests: 3-4 Non-zero exit status: 1 t7105-reset-patch.sh (Wstat: 256 Tests: 8 Failed: 6) Failed tests: 2-7 Non-zero exit status: 1 t7501-commit.sh (Wstat: 256 Tests: 42 Failed: 1) Failed test: 21 Non-zero exit status: 1 t7800-difftool.sh (Wstat: 256 Tests: 22 Failed: 21) Failed tests: 2-22 Non-zero exit status: 1 t9700-perl-git.sh (Wstat: 256 Tests: 2 Failed: 1) Failed test: 2 Non-zero exit status: 1 Parse errors: No plan found in TAP output t9001-send-email.sh (Wstat: 256 Tests: 85 Failed: 59) Failed tests: 4-7, 9-10, 12-13, 15, 17-20, 22, 24, 26 28-30, 32, 34, 36, 38, 40, 42, 44, 46, 48-55 58-77, 80-82, 85 Non-zero exit status: 1 And with NO_PERL_MAKEMAKER=1: Test Summary Report ------------------- t2016-checkout-patch.sh (Wstat: 256 Tests: 14 Failed: 12) Failed tests: 2-13 Non-zero exit status: 1 t3904-stash-patch.sh (Wstat: 256 Tests: 5 Failed: 2) Failed tests: 3-4 Non-zero exit status: 1 t3701-add-interactive.sh (Wstat: 256 Tests: 33 Failed: 17) Failed tests: 2, 4-5, 7, 9-10, 13, 16, 18, 21-25, 29 31, 33 Non-zero exit status: 1 t7105-reset-patch.sh (Wstat: 256 Tests: 8 Failed: 6) Failed tests: 2-7 Non-zero exit status: 1 t7501-commit.sh (Wstat: 256 Tests: 42 Failed: 1) Failed test: 21 Non-zero exit status: 1 t9700-perl-git.sh (Wstat: 256 Tests: 14 Failed: 0) Non-zero exit status: 1 Parse errors: No plan found in TAP output All but the last error are due to this bug in git-add--interactive.perl. I didn't track down what was wrong with t9700-perl-git.sh. git-svn would have failed too if I had SVN libraries: $ /opt/git/perl-56/bin/git svn Can't locate Digest/MD5.pm in @INC [...] Getting a working Digest::* (and anything else git-svn needs) for 5.6 at this point is a *lot* harder than just compiling 5.8 (or even better, 5.12). Since we're not getting patches for common things that have been broken on 5.6 for years and bumping the requirenment to an 8 year old perl (5.8) instead of a 10 year old one (5.6) would make things much easier, including: * Fixing the perl/ Makefile mess * Being able to use 5.8 features * Being able to honestly support the 5.8 release, 5.6 doesn't even compile on modern systems without undocumented monkeypatches, and few people use it so we don't get fixes for it. I'd like to propose dropping 5.6 support, and move to say 5.008. I can do the work required to add appropriate docs / use statements and fixes to bugs that we can't fix on 5.6. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html