I18N.pm is incompatible with perl < 5.8.3

I18N.pm is incompatible with perl < 5.8.3

[Tom G. Christensen]

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

Re: I18N.pm is incompatible with perl < 5.8.3

[Ævar Arnfjörð Bjarmason]

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

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

Re: I18N.pm is incompatible with perl < 5.8.3

[Tom G. Christensen]

Ævar Arnfjörð Bjarmason wrote:
> On Thu, Feb 2, 2012 at 14:11, Tom G. Christensen
> <tgc@statsbiblioteket.dk> wrote:
>> Hello,
>>
> 
> Thanks Tom, I'll submit a patch for that. Does this work, i.e. does
> 5.8.3 need *{import} = instead of *import = ?
> 
<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

Re: I18N.pm is incompatible with perl < 5.8.3

[Tom G. Christensen]

Ævar Arnfjörð Bjarmason wrote:
> On Thu, Feb 2, 2012 at 14:11, Tom G. Christensen
> <tgc@statsbiblioteket.dk> wrote:
>> Hello,
>>
> 
> Thanks Tom, I'll submit a patch for that. 

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

Re: I18N.pm is incompatible with perl < 5.8.3

[Ævar Arnfjörð Bjarmason]

On Wed, Mar 7, 2012 at 13:56, Tom G. Christensen
<tgc@statsbiblioteket.dk> wrote:
> Ævar Arnfjörð Bjarmason wrote:
>>
>> On Thu, Feb 2, 2012 at 14:11, Tom G. Christensen
>> <tgc@statsbiblioteket.dk> wrote:
>>>
>>> Hello,
>>>
>>
>> Thanks Tom, I'll submit a patch for that.
>
>
> 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.

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.

[PATCH 0/2] Minor fixes for Perl + Git::I18N

[Ævar Arnfjörð Bjarmason]

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(-)

-- 
1.7.9

[PATCH 1/2] Git::I18N: compatibility with perl <5.8.3

[Ævar Arnfjörð Bjarmason]

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(-)

diff --git a/perl/Git/I18N.pm b/perl/Git/I18N.pm
index 07597dc..40dd897 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 which supports this invocation was
+		# released with perl 5.8.3
+		Exporter->import('import');
+	}
+}
 
 our @EXPORT = qw(__);
 our @EXPORT_OK = @EXPORT;
-- 
1.7.9

[PATCH 2/2] perl/Makefile: install Git::I18N under NO_PERL_MAKEMAKER

[Ævar Arnfjörð Bjarmason]

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(-)

diff --git a/perl/Makefile b/perl/Makefile
index b2977cd..9af9025 100644
--- a/perl/Makefile
+++ b/perl/Makefile
@@ -23,20 +23,25 @@ clean:
 ifdef NO_PERL_MAKEMAKER
 instdir_SQ = $(subst ','\'',$(prefix)/lib)
 $(makfile): ../GIT-CFLAGS Makefile
-	echo all: private-Error.pm Git.pm > $@
+	echo all: private-Error.pm Git.pm Git/I18N.pm > $@
 	echo '	mkdir -p blib/lib' >> $@
+	echo '	mkdir -p blib/lib/Git' >> $@
 	echo '	$(RM) blib/lib/Git.pm; cp Git.pm blib/lib/' >> $@
+	echo '	$(RM) blib/lib/Git/I18N.pm; cp Git/I18N.pm blib/lib/Git/' >> $@
 	echo '	$(RM) blib/lib/Error.pm' >> $@
 	'$(PERL_PATH_SQ)' -MError -e 'exit($$Error::VERSION < 0.15009)' || \
 	echo '	cp private-Error.pm blib/lib/Error.pm' >> $@
 	echo install: >> $@
 	echo '	mkdir -p "$$(DESTDIR)$(instdir_SQ)"' >> $@
+	echo '	mkdir -p "$$(DESTDIR)$(instdir_SQ)/Git"' >> $@
 	echo '	$(RM) "$$(DESTDIR)$(instdir_SQ)/Git.pm"; cp Git.pm "$$(DESTDIR)$(instdir_SQ)"' >> $@
+	echo '	$(RM) "$$(DESTDIR)$(instdir_SQ)/Git/I18N.pm"; cp Git/I18N.pm "$$(DESTDIR)$(instdir_SQ)/Git"' >> $@
 	echo '	$(RM) "$$(DESTDIR)$(instdir_SQ)/Error.pm"' >> $@
 	'$(PERL_PATH_SQ)' -MError -e 'exit($$Error::VERSION < 0.15009)' || \
 	echo '	cp private-Error.pm "$$(DESTDIR)$(instdir_SQ)/Error.pm"' >> $@
 	echo instlibdir: >> $@
 	echo '	echo $(instdir_SQ)' >> $@
+	echo '	echo $(instdir_SQ)/Git' >> $@
 else
 $(makfile): Makefile.PL ../GIT-CFLAGS
 	$(PERL_PATH) $< PREFIX='$(prefix_SQ)' INSTALL_BASE='' --localedir='$(localedir_SQ)'
-- 
1.7.9

Re: [PATCH 2/2] perl/Makefile: install Git::I18N under NO_PERL_MAKEMAKER

[Junio C Hamano]

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

> 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(-)
>
> diff --git a/perl/Makefile b/perl/Makefile
> index b2977cd..9af9025 100644
> --- a/perl/Makefile
> +++ b/perl/Makefile
> @@ -23,20 +23,25 @@ clean:
>  ifdef NO_PERL_MAKEMAKER
>  instdir_SQ = $(subst ','\'',$(prefix)/lib)
>  $(makfile): ../GIT-CFLAGS Makefile
> -	echo all: private-Error.pm Git.pm > $@
> +	echo all: private-Error.pm Git.pm Git/I18N.pm > $@
>  	echo '	mkdir -p blib/lib' >> $@
> +	echo '	mkdir -p blib/lib/Git' >> $@

micronit; the previous line can be removed.

>  	echo '	$(RM) blib/lib/Git.pm; cp Git.pm blib/lib/' >> $@
> +	echo '	$(RM) blib/lib/Git/I18N.pm; cp Git/I18N.pm blib/lib/Git/' >> $@
>  	echo '	$(RM) blib/lib/Error.pm' >> $@
>  	'$(PERL_PATH_SQ)' -MError -e 'exit($$Error::VERSION < 0.15009)' || \
>  	echo '	cp private-Error.pm blib/lib/Error.pm' >> $@
>  	echo install: >> $@
>  	echo '	mkdir -p "$$(DESTDIR)$(instdir_SQ)"' >> $@
> +	echo '	mkdir -p "$$(DESTDIR)$(instdir_SQ)/Git"' >> $@
>  	echo '	$(RM) "$$(DESTDIR)$(instdir_SQ)/Git.pm"; cp Git.pm "$$(DESTDIR)$(instdir_SQ)"' >> $@
> +	echo '	$(RM) "$$(DESTDIR)$(instdir_SQ)/Git/I18N.pm"; cp Git/I18N.pm "$$(DESTDIR)$(instdir_SQ)/Git"' >> $@
>  	echo '	$(RM) "$$(DESTDIR)$(instdir_SQ)/Error.pm"' >> $@
>  	'$(PERL_PATH_SQ)' -MError -e 'exit($$Error::VERSION < 0.15009)' || \
>  	echo '	cp private-Error.pm "$$(DESTDIR)$(instdir_SQ)/Error.pm"' >> $@
>  	echo instlibdir: >> $@
>  	echo '	echo $(instdir_SQ)' >> $@
> +	echo '	echo $(instdir_SQ)/Git' >> $@

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 >$@+ && \

Re: [PATCH 2/2] perl/Makefile: install Git::I18N under NO_PERL_MAKEMAKER

[Ævar Arnfjörð Bjarmason]

On Sat, Mar 10, 2012 at 22:38, Junio C Hamano <gitster@pobox.com> wrote:
>>       echo '  $(RM) blib/lib/Git.pm; cp Git.pm blib/lib/' >> $@
>> +     echo '  $(RM) blib/lib/Git/I18N.pm; cp Git/I18N.pm blib/lib/Git/' >> $@
>>       echo '  $(RM) blib/lib/Error.pm' >> $@
>>       '$(PERL_PATH_SQ)' -MError -e 'exit($$Error::VERSION < 0.15009)' || \
>>       echo '  cp private-Error.pm blib/lib/Error.pm' >> $@
>>       echo install: >> $@
>>       echo '  mkdir -p "$$(DESTDIR)$(instdir_SQ)"' >> $@
>> +     echo '  mkdir -p "$$(DESTDIR)$(instdir_SQ)/Git"' >> $@
>>       echo '  $(RM) "$$(DESTDIR)$(instdir_SQ)/Git.pm"; cp Git.pm "$$(DESTDIR)$(instdir_SQ)"' >> $@
>> +     echo '  $(RM) "$$(DESTDIR)$(instdir_SQ)/Git/I18N.pm"; cp Git/I18N.pm "$$(DESTDIR)$(instdir_SQ)/Git"' >> $@
>>       echo '  $(RM) "$$(DESTDIR)$(instdir_SQ)/Error.pm"' >> $@
>>       '$(PERL_PATH_SQ)' -MError -e 'exit($$Error::VERSION < 0.15009)' || \
>>       echo '  cp private-Error.pm "$$(DESTDIR)$(instdir_SQ)/Error.pm"' >> $@
>>       echo instlibdir: >> $@
>>       echo '  echo $(instdir_SQ)' >> $@
>> +     echo '  echo $(instdir_SQ)/Git' >> $@
>
> 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

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

Re: [PATCH 2/2] perl/Makefile: install Git::I18N under NO_PERL_MAKEMAKER

[Junio C Hamano]

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.