xslate / p5-Mouse

Lightweight class builder for Perl, as a subset of Moose
https://metacpan.org/release/Mouse
Other
46 stars 32 forks source link

XS_VERSION error while building on Perl 5.8.8 #18

Closed wyoung closed 10 years ago

wyoung commented 10 years ago

I get this error while attempting to build Mouse on RHEL 5, which ships with Perl 5.8.8:

gcc -I. -Ixs-src -I/usr/lib64/perl5/5.8.8/x86_64-linux-thread-multi/CORE -fPIC -Wall -Wextra -Wdeclaration-after-statement -Wc++-compat -c -D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -Wdeclaration-after-statement -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -o xs-src/MouseTypeConstraints.o xs-src/MouseTypeConstraints.c xs-src/MouseTypeConstraints.xs: In function ‘mouse_tc_check’: xs-src/MouseTypeConstraints.xs:47: error: expected ‘)’ before ‘XS_VERSION’ xs-src/MouseTypeConstraints.xs:47: error: too few arguments to function ‘Perl_hv_fetch’

The complaint about XS_VERSION repeats on lines 459, 500, 608, 639, and 831.

I realize Perl 5.8.8 is getting a bit old by this point, but Red Hat will be supporting RHEL 5 for another 3 years. This will drive a certain amount of demand for Perl 5.8 compatibility.

The same problem does not occur on RHEL 6 (Perl 5.10) but I cannot upgrade hundreds of machines to RHEL 6 just to work around this.

syohex commented 10 years ago

This is caused by that XS_VERSION is not defined in such files(MouseTypeConstraints.xs, MouseUtil.xs). Usually definition of XS_VERSION is passed by Module::Build as compiler option(-DXS_VERSION=distribution_version) when xs file is compiled, But that compile option is passed only files which are specified xs_files parameter in Build.PL or own builder file(builder/MyBuilder.pm in Mouse) .

In Mouse, only xs-src/Mouse.xs is specified as xs_files, so I suppose this issue is raised.

tokuhirom commented 10 years ago

@gfx here is a patch for this issue.

diff --git a/.gitignore b/.gitignore
index ab7527e..bb8aa94 100644
--- a/.gitignore
+++ b/.gitignore
@@ -37,3 +37,4 @@ Moose-t/
 !LICENSE
 /lib/Mouse.xs
 /MANIFEST
+/xs-src/xs_version.h
diff --git a/builder/MyBuilder.pm b/builder/MyBuilder.pm
index 26968b2..96a632e 100644
--- a/builder/MyBuilder.pm
+++ b/builder/MyBuilder.pm
@@ -28,12 +28,18 @@ sub new {
 }

 sub ACTION_code {
-    my ($class) = @_;
+    my ($self) = @_;

     system($^X, 'tool/generate-mouse-tiny.pl', 'lib/Mouse/Tiny.pm') == 0
         or warn "Cannot generate Mouse::Tiny: $!";

-    unless ($class->pureperl_only) {
+    open my $fh, '>', 'xs-src/xs_version.h';
+    print {$fh} "#ifndef XS_VERSION\n";
+    printf {$fh} "#define XS_VERSION \"%s\"\n", $self->dist_version;
+    print {$fh} "#endif\n";
+    close($fh);
+
+    unless ($self->pureperl_only) {
         require ExtUtils::ParseXS;
         for my $xs (qw(
             xs-src/MouseAccessor.xs
@@ -50,7 +56,7 @@ sub ACTION_code {
         }
     }

-    $class->SUPER::ACTION_code();
+    $self->SUPER::ACTION_code();
 }

 sub ACTION_test {
diff --git a/xs-src/MouseTypeConstraints.xs b/xs-src/MouseTypeConstraints.xs
index 62858c5..48a984e 100644
--- a/xs-src/MouseTypeConstraints.xs
+++ b/xs-src/MouseTypeConstraints.xs
@@ -5,6 +5,7 @@
  */

 #include "mouse.h"
+#include "xs_version.h"

 #ifndef SvRXOK
 #define SvRXOK(sv) (SvROK(sv) && SvMAGICAL(SvRV(sv)) && mg_find(SvRV(sv), PERL_MAGIC_qr))
diff --git a/xs-src/MouseUtil.xs b/xs-src/MouseUtil.xs
index 8b6970c..506b275 100644
--- a/xs-src/MouseUtil.xs
+++ b/xs-src/MouseUtil.xs
@@ -1,4 +1,5 @@
 #include "mouse.h"
+#include "xs_version.h"

 #define MY_CXT_KEY "Mouse::Util::_guts" XS_VERSION
 typedef struct {
tokuhirom commented 10 years ago

This patch is bit hacky, but works.

tokuhirom commented 10 years ago

Note. Perl 5.10+ does not use MY_CXT. It's the reason of this regression.

wyoung commented 10 years ago

tokuhirom's patch does indeed allow Mouse to build on RHEL 5. Thank you!

That in turn allowed Xslate to build. On testing my Xslate-based app on RHEL 5, it seems to run correctly, so it seems Mouse is also running correctly on RHEL 5.

I assume this patch will appear in the next version of Mouse?

gfx commented 10 years ago

It must be fixed and I have a will to do XD

gfx commented 10 years ago

@tokuhirom it's sounds good. can you make a p-r?

gfx commented 10 years ago

Fixed by #20. Thanks.