Ticket #2024 (new bug)

Opened 3 years ago

Last modified 3 years ago

t/library/pcre.t: Not honoring '--without-pcre' configuration flag

Reported by: jkeenan Owned by:
Priority: normal Milestone:
Component: testing Version: 3.1.0
Severity: medium Keywords:
Cc: fperrad, ronaldws, Util, Coke Language:
Patch status: Platform:

Description (last modified by jkeenan) (diff)

Poking around in Smolder reports, I noticed one from Coke where failures in multiple files included t/library/pcre.t. This was surprising, because in this run Parrot was configured without PCRE.

Configure args
  "--cc=ccache cc" "--ccflags=-g" "--without-gettext" 
  "--without-gmp" "--without-libffi" "--without-extra-nci-thunks" 
  "--without-opengl" "--without-readline" "--without-pcre" 
  "--without-zlib" "--without-threads" "--without-icu" "--cc=g++" 
  "--link=g++" "--ld=g++" 
  "--prefix=/Users/coke/sandbox/parrot-smoker/installed_parrot"

Looking at t/library/pcre.t, I see that it never bothers to check $PConfig{HAS_PCRE} and so proceeds to run the tests. There's a lot of code that appears to predate config/auto/pcre.pm which attempts to determine whether PCRE is present.

# if we keep pcre, we need a config test
my $cmd = ( $^O =~ /MSWin32/ )
  ? "pcregrep --version" : "pcre-config --version";
my $has_pcre = !Parrot::Test::run_command( 
  $cmd, STDOUT => File::Spec->devnull ,
  STDERR => File::Spec->devnull, );
my $pcre_libpath = '';

# It's possible that libpcre is installed in 
# some non-standard path...
if ($has_pcre && ($^O !~ /MSWin32/)) {
    # Extract the library path for non-windows platforms 
    # (in case it isn't in
    # the normal lookup locations)
    my $outfile = 'pcre-config.out';
    Parrot::Test::run_command('pcre-config --prefix', 
       STDOUT => $outfile);
    my $out = Parrot::Test::slurp_file($outfile);
    unlink $outfile;
    chomp $out;
    $pcre_libpath = "$out/lib";
}

The value of $pcre_libpath is set in this block and used subsequently in tests. Not surprisingly, since no value is ever assigned when on Windows, the tests have to be skipped for Windows.

This is clearly not a satisfactory situation. By the time we get this far into testing, we should unambiguously know whether we have PCRE or not. If we need to know a library path, we should also determine that during configuration as well.

Questions:

1. Is there anything of value in the way t/library/pcre.t tests for the presence of PCRE that ought to be incorporated into config/auto/pcre.pm?

2. The tests in this file presume we know the non-empty-string value of $pcre_libpath. Should we be determining that during configuration time as well.

Am cc-ing some people who helped with TT #1401.

Thank you very much.

kid51

Change History

follow-up: ↓ 3   Changed 3 years ago by NotFound

I think that pcre should be out of parrot core: put it on its own repo, and put library searching and any other config step needed in its installation (or at runtime in its PIR part).

  Changed 3 years ago by jkeenan

  • description modified (diff)

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

Replying to NotFound:

I think that pcre should be out of parrot core: put it on its own repo, and put library searching and any other config step needed in its installation (or at runtime in its PIR part).

I don't necessarily object to having that discussion -- but that would constitute scope creep for this Trac ticket. It's the sort of discussion that ought to begin on parrot-dev. Could you post to that list?

Thank you very much.

kid51

follow-up: ↓ 5   Changed 3 years ago by whiteknight

To play devil's advocate here, why do we include PCRE in the parrot build at all? Do we use PCRE internally to Parrot for any reason, or do we simply expose it's use through NCI?

If it's not a necessary core component of Parrot, I suggest we stop including it in the Parrot build, move it's use into an external library wrapper project, and close this ticket.

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

Replying to whiteknight:

To play devil's advocate here,

The place for playing devil's advocate is on the parrot-dev mailing list. Please take this discussion there.

Thank you very much.

kid51

  Changed 3 years ago by NotFound

The pcre removing has been discussed at #ps and is now tracked in #2028

Note: See TracTickets for help on using tickets.