Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#21770 closed defect (fixed)

Package libpcre as a standard package

Reported by: charpent Owned by:
Priority: major Milestone: sage-7.6
Component: packages: standard Keywords: r-project, days85
Cc: mkoeppe, dimpase Merged in:
Authors: Emmanuel Charpentier Reviewers: Jean-Pierre Flori
Report Upstream: N/A Work issues:
Branch: ef48f71 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by charpent)

Rationale : R (standard package) requires libpcre (in a non-default configuration), but stopped to package it with version 3.3.0.

See this discussion on sage-devel, but look also here and contribute here.

Tarball : https://ftp.pcre.org/pub/pcre/pcre-8.40.tar.gz

Change History (36)

comment:1 Changed 6 years ago by charpent

  • Branch set to u/charpent/package_libpcre_as_a_standard_package

comment:2 Changed 6 years ago by charpent

  • Commit set to a8d77b5c25a0f20c209f04c3a5377b673683cd6e
  • Description modified (diff)

New commits:

a8d77b5Initial packaging of pcre : unconditionnal installation.

comment:3 Changed 6 years ago by charpent

  • Authors set to Emmanuel Charpentier
  • Status changed from new to needs_review

The initial packaging builds without problem and passes its own testsuite ; the resulting Sage passes ptestlong with only the usual transient error on simplicial_complex.py (which passes standalone).

==> needs_review.

comment:4 Changed 6 years ago by jdemeyer

  • Status changed from needs_review to needs_work

In the light of #20692, please remove this patch block (especially given that there are no patches):

# Patch sources.
for patch in ../patches/*.patch; do
    [ -r "$patch" ] || continue  # Skip non-existing or non-readable patches
    patch -p1 <"$patch"
    if [ $? -ne 0 ]; then
        echo >&2 "Error applying '$patch'"
        exit 1
    fi
done

comment:5 follow-up: Changed 6 years ago by git

  • Commit changed from a8d77b5c25a0f20c209f04c3a5377b673683cd6e to 21ce3e75574d3a08266324b7fac41e044e0ef280

Branch pushed to git repo; I updated commit sha1. New commits:

21ce3e7Removed patches handling.

comment:6 in reply to: ↑ 5 Changed 6 years ago by charpent

Replying to git:

Branch pushed to git repo; I updated commit sha1. New commits:

21ce3e7Removed patches handling.

Builds OK, passes its own test suite.

comment:7 Changed 6 years ago by charpent

  • Status changed from needs_work to needs_review

comment:8 Changed 5 years ago by charpent

Sage-7.6.beta4 + #20523 + #21767 + #21770 (this patch) passes ptestlong witht two transient errors :

----------------------------------------------------------------------
sage -t --long src/sage/homology/simplicial_set_morphism.py  # Bad exit: 2
sage -t --long src/sage/homology/simplicial_complex.py  # 1 doctest failed
----------------------------------------------------------------------

Both tests pass when run standalone.

This should close the R saga (for now) by allowing the current interfaces to work with R >= 3.3.x.

Still needs_review

comment:9 Changed 5 years ago by charpent

Merged with #20523

This ticket should be marked as invalid by someeone with the right rights...

comment:10 Changed 5 years ago by charpent

After discussion of (potential) #20523 reviewers, I now think that this ticket should be kept, reviewed and validated. Pleas DON'T invalidate it.

comment:11 follow-up: Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_work
[pcre-8.39] WARNING: spkg-install is not executable, making it executable

comment:12 Changed 5 years ago by git

  • Commit changed from 21ce3e75574d3a08266324b7fac41e044e0ef280 to bfc26ef6badfd02ca88dcd69bd5e86684d1dce41

Branch pushed to git repo; I updated commit sha1. New commits:

bfc26efWups. chmod +x spkg-install...

comment:13 in reply to: ↑ 11 Changed 5 years ago by charpent

  • Status changed from needs_work to needs_review

Replying to jdemeyer:

[pcre-8.39] WARNING: spkg-install is not executable, making it executable

Done. needs_review

comment:14 Changed 5 years ago by dimpase

  • Cc mkoeppe added
  • Milestone changed from sage-7.5 to sage-7.6

I wonder if pcre also pops up in polymake-related tickets (which do need a lot of perl)...

comment:15 Changed 5 years ago by charpent

  • Description modified (diff)

Updated the address of the original tarball (patchbots seem to be unable to use an FTP address...).

comment:16 follow-up: Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_work

I would suggest to make the package optional for now and only make it standard when really needed.

comment:17 in reply to: ↑ 16 ; follow-up: Changed 5 years ago by charpent

Replying to jdemeyer:

I would suggest to make the package optional for now and only make it standard when really needed.

We've already been there (see previous saga...) :

  • R is a standard package
  • R depends on PCRE starting 3.3.x
  • A standard package cannot depend on an optional package

Therefore pcre has to be standard. QED.

Last edited 5 years ago by charpent (previous) (diff)

comment:18 Changed 5 years ago by charpent

  • Status changed from needs_work to needs_review

And, yes, it needs_review.

comment:19 in reply to: ↑ 17 ; follow-up: Changed 5 years ago by jdemeyer

Replying to charpent:

  • R version 3.2.4-revised is a standard package
  • R version 3.3.x depends on PCRE
  • A standard package cannot depend on an optional package

I stand by my proposal: make it standard when really needed.

comment:20 in reply to: ↑ 19 ; follow-up: Changed 5 years ago by charpent

Replying to jdemeyer:

Replying to charpent:

  • R version 3.2.4-revised is a standard package

And is about 1 year late on current R. Having an up-to-date R version is a sine qua non to get help on R-help, and to be able to install recent versions of the >10 000 R packages. Which are a not inconsiderable resource to those lowly life-forms called applied statisticians...

  • R version 3.3.x depends on PCRE
  • A standard package cannot depend on an optional package

I stand by my proposal: make it standard when really needed.

IMHO it *is* really needed. But it happens that I need R for my everyday work. Do you ?

Last edited 5 years ago by charpent (previous) (diff)

comment:21 in reply to: ↑ 20 ; follow-up: Changed 5 years ago by jdemeyer

Replying to charpent:

Replying to jdemeyer:

Replying to charpent:

  • R version 3.2.4-revised is a standard package

And is about 1 year late on current R. Having an up-to-date R version is a sine qua non to get help on R-help, and to be able to install recent versions of the >10 000 R packages. Which are a not inconsiderable resource to those lowly life-forms called applied statisticians...

That may be all true, but I don't see how that matters here.

  • R version 3.3.x depends on PCRE
  • A standard package cannot depend on an optional package

I stand by my proposal: make it standard when really needed.

IMHO it *is* really needed.

...when we upgrade to R-3.3.0, which is not the case yet.

comment:22 in reply to: ↑ 21 ; follow-up: Changed 5 years ago by charpent

Replying to jdemeyer:

Replying to charpent:

Replying to jdemeyer:

Replying to charpent:

  • R version 3.2.4-revised is a standard package

And is about 1 year late on current R. Having an up-to-date R version is a sine qua non to get help on R-help, and to be able to install recent versions of the >10 000 R packages. Which are a not inconsiderable resource to those lowly life-forms called applied statisticians...

That may be all true, but I don't see how that matters here.

  • R version 3.3.x depends on PCRE
  • A standard package cannot depend on an optional package

I stand by my proposal: make it standard when really needed.

IMHO it *is* really needed.

...when we upgrade to R-3.3.0, which is not the case yet.

#20523 is waiting on this ticket for review (and has been for a looong time : it has been delayed by the openssl brouhaha).

Last edited 5 years ago by charpent (previous) (diff)

comment:23 in reply to: ↑ 22 ; follow-up: Changed 5 years ago by jdemeyer

Replying to charpent:

#20523 is waiting on this ticket for review (and has been for a looong time : it has been delayed by the openssl brouhaha).

Then make it optional here and standard in #20523.

comment:24 in reply to: ↑ 23 ; follow-up: Changed 5 years ago by charpent

Replying to jdemeyer:

Replying to charpent:

#20523 is waiting on this ticket for review (and has been for a looong time : it has been delayed by the openssl brouhaha).

Then make it optional here and standard in #20523.

I can't see the point... Care to explain ?

comment:25 in reply to: ↑ 24 ; follow-up: Changed 5 years ago by jdemeyer

Replying to charpent:

I can't see the point... Care to explain ?

I don't see the point of having a standard package which is not used by anything in Sage.

comment:26 in reply to: ↑ 25 Changed 5 years ago by charpent

Replying to jdemeyer:

Replying to charpent:

I can't see the point... Care to explain ?

I don't see the point of having a standard package which is not used by anything in Sage.

OK. If I follow you :

  • PCRE is not used by anything in Sage
  • PCRE is used by R

Therefore, by modus tollens:

  • R is not in Sage.

We should submit this interesting conclusion to sage-devel : the discussion would probably be lively and entertaining (for questionable values of "entertaining"...).

comment:27 Changed 5 years ago by jpflori

  • Cc dimpase added

comment:28 follow-up: Changed 5 years ago by jpflori

@emmanuel: can you update to 8.40 pls?

comment:29 in reply to: ↑ 28 Changed 5 years ago by charpent

Replying to jpflori:

@emmanuel: can you update to 8.40 pls?

I'm on it. Count about 15 minutes to post an update patch, about 2-3 hours to compile and ptestlong it.

BTW : since #21767 and the present patch have been merged in #20523, I'll have to update it also before review.

comment:30 Changed 5 years ago by git

  • Commit changed from bfc26ef6badfd02ca88dcd69bd5e86684d1dce41 to ef48f715189d6629f776b7a87334fc4da2e6219f

Branch pushed to git repo; I updated commit sha1. New commits:

ef48f71Upgraded to pcre 8.40

comment:31 Changed 5 years ago by charpent

  • Description modified (diff)

Upgrade as requested. Therefore, tarball modified accordingly.


New commits:

ef48f71Upgraded to pcre 8.40

comment:32 Changed 5 years ago by charpent

  • Passes its own testsuite.
  • (#21567 + #21770 + #20523 remerged with the two previous ones) passes ptestlong with two failures :
    ----------------------------------------------------------------------
    sage -t --long src/sage/homology/simplicial_complex.py  # 1 doctest failed
    sage -t --long src/sage/rings/polynomial/multi_polynomial_ideal.py  # 1 doctest failed
    ----------------------------------------------------------------------
    

The first one is transient (passes when ran standalone) ; the second one is a well-documented, ticketed and not yet fixed difference on the expression of the expected result.

==> needs_review for good.

comment:33 Changed 5 years ago by jpflori

  • Reviewers set to Jean-Pierre Flori
  • Status changed from needs_review to positive_review

Looks ok.

comment:34 Changed 5 years ago by jpflori

  • Keywords days85 added

comment:35 Changed 5 years ago by vbraun

  • Branch changed from u/charpent/package_libpcre_as_a_standard_package to ef48f715189d6629f776b7a87334fc4da2e6219f
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:36 Changed 4 years ago by jdemeyer

  • Commit ef48f715189d6629f776b7a87334fc4da2e6219f deleted

Is the --enable-jit flag really required? It already caused problems on Cygwin and it is breaking on Solaris too (#24628).

Note: See TracTickets for help on using tickets.