I18N.pm is incompatible with perl < 5.8.3

Hello,

While running the git 1.7.9 testsuite on RHEL 3 with perl 5.8.0 and gettext 0.11.4 I got this error in t0202-gettext-perl.sh: # test_external test Perl Git::I18N API failed: /usr/bin/perl /builddir/build/BUILD/git-1.7.9/t/t0202/test.pl # test_external_without_stderr test no stderr: Perl Git::I18N API failed: /usr/bin/perl /builddir/build/BUILD/git-1.7.9/t/t0202/test.pl:

A verbose run gave me this: # test_external test Perl Git::I18N API failed: /usr/bin/perl /builddir/build/BUILD/git-1.7.9/t/t0202/test.pl # expecting no stderr from previous command # test_external_without_stderr test no stderr: Perl Git::I18N API failed: /usr/bin/perl /builddir/build/BUILD/git-1.7.9/t/t0202/test.pl: # Stderr is: "import" is not exported by the Exporter module Can't continue after import errors at /builddir/build/BUILD/git-1.7.9/t/../perl/blib/lib/Git/I18N.pm line 5 BEGIN failed--compilation aborted at /builddir/build/BUILD/git-1.7.9/t/../perl/blib/lib/Git/I18N.pm line 5. Compilation failed in require at /builddir/build/BUILD/git-1.7.9/t/t0202/test.pl line 8. BEGIN failed--compilation aborted at /builddir/build/BUILD/git-1.7.9/t/t0202/test.pl line 8. # Looks like your test died before it could output anything.

I found the cause and the solution here: http://www.nntp.perl.org/group/perl.module.build/2008/02/msg1214.html

I've changed
  use Exporter 'import'
to
  BEGIN {
   require Exporter;
   *{import} = \&Exporter::import;
   }
in I18N.pm.

The test now passes (GETTEXT_LOCALE=1): # lib-gettext: No is_IS UTF-8 locale available # lib-gettext: No is_IS ISO-8859-1 locale available # run 1: Perl Git::I18N API (/usr/bin/perl /builddir/build/BUILD/git-1.7.9/t/t0202/test.pl) 1..8 ok 1 - Testing Git::I18N with NO Perl gettext library ok 2 - Git::I18N is located at /builddir/build/BUILD/git-1.7.9/t/../perl/blib/lib/Git/I18N.pm ok 3 - sanity: Git::I18N has 1 export(s) ok 4 - sanity: Git::I18N exports everything by default ok 5 - sanity: __ has a $ prototype ok 6 - Passing a string through __() in the C locale works ok 7 - Without a gettext library + <C> locale <TEST: A Perl test string> turns into <TEST: A Perl test string> ok 8 - Without a gettext library + <is> locale <TEST: A Perl test string> turns into <TEST: A Perl test string> # test_external test Perl Git::I18N API was ok # expecting no stderr from previous command # test_external_without_stderr test no stderr: Perl Git::I18N API was ok

-tgc

On Thu, Feb 2, 2012 at 14:11, Tom G. Christensen <tgc@statsbiblioteket.dk> wrote:

Thanks Tom, I'll submit a patch for that. Does this work, i.e. does 5.8.3 need *{import} = instead of *import = ?

    diff --git a/perl/Git/I18N.pm b/perl/Git/I18N.pm
    index 07597dc..5bcfed5 100644
    --- a/perl/Git/I18N.pm
    +++ b/perl/Git/I18N.pm
    @@ -2,7 +2,16 @@ package Git::I18N;
     use 5.008;
     use strict;
     use warnings;
    -use Exporter 'import';
    +BEGIN {
    +       require Exporter;
    +       if ($] < 5.008003) {
    +               *import = \&Exporter::import;
    +       } else {
    +               # Exporter 5.57 supporting this invocation was released with
    +               # 5.8.3
    +               Exporter->import('import');
    +       }
    +}

     our @EXPORT = qw(__);
     our @EXPORT_OK = @EXPORT;

But actually it might be better to check $Exporter::VERSION

Ævar Arnfjörð Bjarmason wrote:

<snip> I applied the patch and have run the test with perl 5.8.0 (el3) and 5.8.5 (el4) and it passes the test on both versions.

I noticed an additional problem which is that Git/I18N.pm is not installed when NO_PERL_MAKEMAKER is used. It looks like perl/Makefile did not get updated when Git/I18N.pm was added to perl/Makefile.PL.

In perl 5.8.0 ExtUtils::MakeMaker is too old (6.03) to be used with perl/Makefile.PL.

-tgc

Ævar Arnfjörð Bjarmason wrote:

Nothing seems to have been submitted and both this issue and the issue with missing NO_PERL_MAKEMAKER support still exists in 1.7.9.3.

I think I've answered your question but please just ask if you need more information from me.

-tgc

On Wed, Mar 7, 2012 at 13:56, Tom G. Christensen <tgc@statsbiblioteket.dk> wrote:

Sorry about that. I'll submit a patch this week or this weekend. That NO_PERL_MAKEMAKER issue is also either fixed by now or I'll fix it.

This minor series fixes the Perl issues raised by Tom Christensen. Since it shouldn't break anything I hope this can make it into 1.7.10, but that's up to Junio.

Ævar Arnfjörð Bjarmason (2):
  Git::I18N: compatibility with perl <5.8.3
  perl/Makefile: install Git::I18N under NO_PERL_MAKEMAKER

 perl/Git/I18N.pm |   11 ++++++++++-
 perl/Makefile    |    7 ++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

Change the Exporter invocation in Git::I18N to be compatible with 5.8.0 to 5.8.2 inclusive. Before Exporter 5.57 (released with 5.8.3) Exporter didn't export the 'import' subroutine.

Reported-by: Tom G. Christensen <tgc@statsbiblioteket.dk>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 perl/Git/I18N.pm |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

When I added the i18n infrastructure in v1.7.8-rc2-1-g5e9637c I forgot to install Git::I18N also when NO_PERL_MAKEMAKER=YesPlease was set. Change the generation of the fallback perl.mak file to do that.

Now Git/I18N.pm is installed alongside Git.pm in such a way that anything that uses GITPERLLIB will find it.

Reported-by: Tom G. Christensen <tgc@statsbiblioteket.dk>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 perl/Makefile |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

micronit; the previous line can be removed.

This break "make NO_PERL_MAKEMAKER=Nah git-add--interactive", or anything that is a perl script, it appears.

The resulting perl/perl.mak ends with

	instlibdir:
                echo foo
                echo foo/Git

but the top-level Makefile wants to see output from

	make instlibdir

in this directory as a single line in this rule:

$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl
	$(QUIET_GEN)$(RM) $@ $@+ && \
	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory instlibdir` && \
	echo "<$$INSTLIBDIR>" && \
	sed -e '1{' \
	    -e '	s|#!.*perl|#!$(PERL_PATH_SQ)|' \
	    -e '	h' \
	    -e '	s=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || "'"$$INSTLIBDIR"'"));=' \
	    -e '	H' \
	    -e '	x' \
	    -e '}' \
	    -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
	    $@.perl >$@+ && \

On Sat, Mar 10, 2012 at 22:38, Junio C Hamano <gitster@pobox.com> wrote:

Fail, I just tested installing it to /tmp. Sorry about that.

This topic has been stuck for almost a week.

    * ab/perl-i18n (2012-03-10) 3 commits
     - fixup! de1e4ae
     - perl/Makefile: install Git::I18N under NO_PERL_MAKEMAKER
     - Git::I18N: compatibility with perl <5.8.3

I just ran (including the fixup patch queued at the tip) this:

    $ PATH=/usr/bin:/bin ;# I do not have any "git" in these two places
    $ export PATH
    $ make NO_PERL_MAKEMAKER=NoThanks prefix=/tmp/no-perl-makemaker
    $ make NO_PERL_MAKEMAKER=NoThanks prefix=/tmp/no-perl-makemaker install

    $ head -n 2 /tmp/no-perl-makemaker/libexec/git-core/git-difftool
    #!/usr/bin/perl
    use lib (split(/:/, $ENV{GITPERLLIB} || "/tmp/no-perl-makemaker/lib"));
    $ ls -l /tmp/no-perl-makemaker/lib
    total 44
    drwxrwxr-x 2 junio junio  4096 Mar 16 09:06 Git
    -rw-rw-r-- 1 junio junio 36222 Mar 16 09:06 Git.pm
    drwxrwxr-x 3 junio junio  4096 Mar 16 09:06 python2.6

And the resulting perl-ish Porcelain seems to work OK.

    $ PATH=$PATH:/tmp/no-perl-makemaker/bin
    $ git add -i

So I'll be merging this (after squashing the fix-up) to master.