Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build PCRE without JIT if needed #24628

Closed
dimpase opened this issue Feb 1, 2018 · 18 comments
Closed

Build PCRE without JIT if needed #24628

dimpase opened this issue Feb 1, 2018 · 18 comments

Comments

@dimpase
Copy link
Member

dimpase commented Feb 1, 2018

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

CC: @mkoeppe @dimpase @EmmanuelCharpentier

Component: packages: standard

Author: Jeroen Demeyer

Branch: db16bc9

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/24628

@dimpase dimpase added this to the sage-8.2 milestone Feb 1, 2018
@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

jdemeyer commented Feb 5, 2018

Author: Jeroen Demeyer

@jdemeyer jdemeyer changed the title fix pcre build on Solaris 11 Build PCRE without JIT if needed Feb 5, 2018
@jdemeyer

This comment has been minimized.

@dimpase
Copy link
Member Author

dimpase commented Feb 5, 2018

comment:3

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...

@jdemeyer
Copy link

jdemeyer commented Feb 5, 2018

@jdemeyer
Copy link

jdemeyer commented Feb 5, 2018

Commit: db16bc9

@jdemeyer
Copy link

jdemeyer commented Feb 5, 2018

comment:5

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

@jdemeyer
Copy link

jdemeyer commented Feb 5, 2018

comment:7
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

@dimpase
Copy link
Member Author

dimpase commented Feb 5, 2018

Reviewer: Dima Pasechnik

@dimpase
Copy link
Member Author

dimpase commented Feb 5, 2018

comment:8

looks good to me

@vbraun
Copy link
Member

vbraun commented Feb 9, 2018

Changed branch from u/jdemeyer/build_pcre_without_jit_if_needed to db16bc9

@embray
Copy link
Contributor

embray commented Feb 16, 2018

Changed commit from db16bc9 to none

@embray
Copy link
Contributor

embray commented Feb 16, 2018

comment:10

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.

@dimpase
Copy link
Member Author

dimpase commented Feb 16, 2018

comment:11

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

@embray
Copy link
Contributor

embray commented Feb 16, 2018

comment:12

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

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Feb 16, 2018

comment:13

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...

@embray
Copy link
Contributor

embray commented Feb 16, 2018

comment:14

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.

@embray

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants