Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#24628 closed defect (fixed)

Build PCRE without JIT if needed

Reported by: dimpase Owned by:
Priority: major Milestone: sage-8.2
Component: packages: standard Keywords:
Cc: mkoeppe, dimpase, charpent Merged in:
Authors: Jeroen Demeyer Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: db16bc9 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by embray)

The JIT feature of PCRE is buggy on some systems: it already required a fix for Cygwin and it gives a "bus error" on Solaris.

To work around this, I propose to always run the PCRE testsuite. This takes only a few seconds anyway. If the testsuite fails, recompile PCRE without --enable-jit.

(Skip this, at the very least, on Cygwin since the testsuite fails there anyways, albeit for unrelated reasons; see #24756).

Change History (15)

comment:1 Changed 4 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Cc mkoeppe dimpase charpent added
  • Description modified (diff)
  • Summary changed from fix pcre build on Solaris 11 to Build PCRE without JIT if needed

comment:2 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:3 follow-up: Changed 4 years ago by dimpase

I believe PCRE is used in Polymake, and in Sage's polymake interface. I guess without JIT they (as well as R, as R is using it too) might get slower...

comment:4 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/build_pcre_without_jit_if_needed

comment:5 in reply to: ↑ 3 Changed 4 years ago by jdemeyer

  • Commit set to db16bc954c742ea82878da606e8737e57230865b

Replying to dimpase:

I believe PCRE is used in Polymake, and in Sage's polymake interface. I guess without JIT they (as well as R, as R is using it too) might get slower...

Slower but functional should be acceptable. Note that my patch still uses JIT on systems where it is supported.


New commits:

db16bc9Build PCRE without JIT if it does not work

comment:6 Changed 4 years ago by jdemeyer

  • Status changed from new to needs_review

comment:7 Changed 4 years ago by jdemeyer

real    9m46.453s
user    7m32.365s
sys     2m2.237s
Copying package files from temporary location /datapool/jeroen/sage/local/var/tmp/sage/build/pcre-8.40.p1/inst to /datapool/jeroen/sage/local
Successfully installed pcre-8.40.p1
Deleting temporary build directory
/datapool/jeroen/sage/local/var/tmp/sage/build/pcre-8.40.p1
Finished installing pcre-8.40.p1.spkg

comment:8 Changed 4 years ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

looks good to me

comment:9 Changed 4 years ago by vbraun

  • Branch changed from u/jdemeyer/build_pcre_without_jit_if_needed to db16bc954c742ea82878da606e8737e57230865b
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:10 Changed 4 years ago by embray

  • Commit db16bc954c742ea82878da606e8737e57230865b deleted

Wish someone had CC'd me on this (I don't read every ticket that gets opened). The test suite for PCRE has always failed on Cygwin (with or without JIT) so this broke the build on Cygwin.

comment:11 follow-up: Changed 4 years ago by dimpase

IMHO on Cygwin, and, in fact on any system that provides PCRE, we should just use the system's one.

comment:12 in reply to: ↑ 11 ; follow-up: Changed 4 years ago by embray

Replying to dimpase:

IMHO on Cygwin, and, in fact on any system that provides PCRE, we should just use the system's one.

Agreed. Working on that :)

comment:13 in reply to: ↑ 12 Changed 4 years ago by charpent

Replying to embray:

Replying to dimpase:

IMHO on Cygwin, and, in fact on any system that provides PCRE, we should just use the system's one.

Agreed. Working on that :)

This is a general need : unless we have specific needs not solvable in an interface, we should use whatever is installed systemwide.

A mechanism to do that in the toplevel configuration file would be useful. IIRC, Erik has undertaken to refactor parts of this cofiguraion system (that's why I haven't yet progresed much on OpenSSL, BTW : I'm waiting for the dust to settle a bit...), this might be one of his objectives.

However, I can think of difficulties to extende such a mechanism to library interfaces...

comment:14 Changed 4 years ago by embray

I already have a prototype for this that I'm pretty happy with, but I've been waiting, as you say, for the dust to settle on #21524 before moving forward on it. That ticket is ready for review BTW.

comment:15 Changed 4 years ago by embray

  • Description modified (diff)
Note: See TracTickets for help on using tickets.