Ticket #2118 (closed bug: fixed)

Opened 3 years ago

Last modified 3 years ago

t/src/checkdepend.t fails when extra nci tthunks are disabled

Reported by: dukeleto Owned by: jkeenan
Priority: normal Milestone:
Component: testing Version: 3.4.0
Severity: medium Keywords:
Cc: Language:
Patch status: applied Platform: all

Description

not ok 82 - src/nci/extra_thunks.c (Makefile: line 1743).

#   Failed test 'src/nci/extra_thunks.c (Makefile: line 1743).'
#   at t/src/checkdepend.t line 309.
#          got: 'src/nci/extra_thunks.str(1)
# '
#     expected: ''

Attachments

t_src_checkdepend_t.diff Download (5.3 KB) - added by jkeenan 3 years ago.
patch will simply recognize when we don't need to check dependencies for a particular file

Change History

  Changed 3 years ago by jkeenan

  • patch set to new

I've been trying to understand the problem in this ticket, particularly since the question of what happens when you configure --without-extra-nci-thunks shows up in TT #1979, TT #2116, and TT #2024.

I created this branch in the git repository: tt2118/check_depend.

In that branch, I did some modest refactoring of t/src/checkdepend.t to make it easier for me to study. (This mostly entailed grouping all the subroutines together at the end of the file.) I then ran perl Configure.pl --without-extra-nci-thunks && make. This configuration causes the one test failure in t/src/checkdepend.t:

not ok 84 - src/nci/extra_thunks.c (Makefile: line 1753).

#   Failed test 'src/nci/extra_thunks.c (Makefile: line 1753).'
#   at t/src/checkdepend.t line 303.
#          got: 'src/nci/extra_thunks.str(1)
# '
#     expected: ''

After playing with this for several hours, I realized that we could get the test to pass with some rather kludgey code. See attachment for complete diff; here's the essence:

+use lib qw( lib );
+use Parrot::Config;
 
 =head1 NAME
 
@@ -48,7 +49,7 @@ if (! -e 'Makefile') {
     exit;
 }
 
-my @incfiles = [];
+my @incfiles = ();
 find( { wanted => \&wanted, no_chdir => 1 },
       qw/src compilers include frontend/ );
 
@@ -57,6 +58,8 @@ our %deps;
 foreach my $file (sort grep /\.[hc]$/, @incfiles) {
     # skip pmcs - we don't handle inheritance correctly
     next if $file =~ m{^src/(?:dyn)?pmc/};
+    next if ($file eq 'src/nci/extra_thunks.c' and
+        ! defined $Parrot::Config::PConfig_Temp{PARROT_HAS_EXTRA_NCI_THUNKS});

Would this suffice for this ticket?

Thank you very much.

kid51

Changed 3 years ago by jkeenan

patch will simply recognize when we don't need to check dependencies for a particular file

  Changed 3 years ago by jkeenan

dukeleto, plobsing:

Is the patch attached a reasonable approach?

Thank you very much.

kid51

follow-up: ↓ 4   Changed 3 years ago by ligne

i just saw (almost) the same thing happen when built --without-core-nci-thunks:

make realclean && perl Configure.pl --without-core-nci-thunks && nice make
prove t/src/checkdepend.t
# [snip]
#   Failed test 'src/nci/core_thunks.c (Makefile: line 1733).'
#   at t/src/checkdepend.t line 309.
#          got: 'src/nci/core_thunks.str(1)
# '
#     expected: ''
# Looks like you failed 1 test of 159.

in reply to: ↑ 3   Changed 3 years ago by jkeenan

  • status changed from new to assigned
  • owner changed from plobsing to jkeenan
  • patch changed from new to applied

Replying to ligne:

i just saw (almost) the same thing happen when built --without-core-nci-thunks:

Taking into account this report from ligne++ and his suggestion in TT #2116, I have applied this patch in commit 9f6220f:

diff --git a/t/library/pcre.t b/t/library/pcre.t
index 928bd0b..388ec55 100644
--- a/t/library/pcre.t
+++ b/t/library/pcre.t
@@ -7,6 +7,7 @@ use lib qw( t . lib ../lib ../../lib );
 
 use Test::More;
 use Parrot::Test tests => 2;
+use Parrot::Config qw( %PConfig );
 
 =head1 NAME
 
@@ -41,8 +42,12 @@ if ($has_pcre && ($^O !~ /MSWin32/)) {
 }
 
 SKIP: {
-    skip( 'no pcre-config', Test::Builder->new()->expected_tests() )
-        unless $has_pcre;
+    skip( 'no pcre-config',
+        Test::Builder->new()->expected_tests()
+    ) unless $has_pcre;
+    skip( 'Parrot built without libffi or extra NCI thunks',
+        Test::Builder->new()->expected_tests()
+    ) unless ($PConfig{HAS_EXTRA_NCI_THUNKS} || $PConfig{HAS_LIBFFI});
 
 ## 1
 ## Check that the library can be loaded and initialized,
diff --git a/t/src/checkdepend.t b/t/src/checkdepend.t
index d7b2f10..d0b9798 100644
--- a/t/src/checkdepend.t
+++ b/t/src/checkdepend.t
@@ -10,6 +10,8 @@ use Fatal qw(open);
 use File::Find;
 use File::Spec;
 use Test::More;
+use lib qw( lib );
+use Parrot::Config;
 
 =head1 NAME
 
@@ -57,6 +59,10 @@ our %deps;
 foreach my $file (sort grep /\.[hc]$/, @incfiles) {
     # skip pmcs - we don't handle inheritance correctly
     next if $file =~ m{^src/(?:dyn)?pmc/};
+    next if ($file eq 'src/nci/core_thunks.c' and
+        ! defined $Parrot::Config::PConfig_Temp{PARROT_HAS_CORE_NCI_THUNKS});
+    next if ($file eq 'src/nci/extra_thunks.c' and
+        ! defined $Parrot::Config::PConfig_Temp{PARROT_HAS_EXTRA_NCI_THUNKS});
 
     open my $fh, '<', $file;
     my $guts;

I have achieved PASS on make test on linux/i386 with both:

perl Configure.pl

# and

perl Configure.pl --without-gettext  \
  --without-gmp \
  --without-libffi \
  --without-core-nci-thunks \
  --without-extra-nci-thunks \
  --without-opengl \
  --without-readline \
  --without-pcre \
  --without-zlib \
  --without-threads \
  --without-icu \
  $@

Will report results from darwin/ppc tomorrow; other tests would be appreciated.

Thank you very much.

kid51

  Changed 3 years ago by ligne

all the relevant tests come up clean on a range of linux/x86_64 configurations.

follow-up: ↓ 7   Changed 3 years ago by jkeenan

I will close this ticket on or before Sun Jun 12 if there is no further objection.

in reply to: ↑ 6   Changed 3 years ago by jkeenan

Replying to jkeenan:

I will close this ticket on or before Sun Jun 12 if there is no further objection.

No complaints received; closing as per plan.

  Changed 3 years ago by jkeenan

  • status changed from assigned to closed
  • resolution set to fixed
Note: See TracTickets for help on using tickets.