Opened 2 years ago

Last modified 3 months ago

#29499 new enhancement

Rename ./configure --with-system-SPKG option values to 'no'/'check'/'yes' and add new value 'force'

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.7
Component: build: configure Keywords:
Cc: mjo, gh-kliem, dimpase, vdelecroix Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

Right now we support ./configure --with-system-SPKG={no,yes,force}, where force means: terminate with an error if no suitable system package exists.

We rename these option values to {no,check,yes}, which makes the value force available for a new behavior: experienced/daring users can force to use the system package even if our spkg-configure has a concern about the system package. See #29494 (Reject old system pari with bug in hyperellcharpoly).

Change History (26)

comment:1 Changed 2 years ago by mjo

As a time-saving mechanism, this doesn't really solve the problem, because it doesn't distinguish between important checks (API and dependent-package compatibility) in spkg-configure.m4 and the ones that I consider unimportant (bug checks to enforce bleeding-edge patched versions).

While I'm working on the sage build system every day, I know that the latest pari-2.11.2 in Gentoo is absolutely fine, but if I take a month off and try to pass --with-system-pari=yeptrustme, it could compile stuff for three hours before crashing because another package needs a symbol in libpari that wasn't there in 2.11.2. Then I've wasted not just the time needed to build pari, but everything before it.

comment:2 Changed 2 years ago by mkoeppe

Of course this is a valid point. I see it more as a development tool - easier to pass that option to check if things work than to edit out some checks in the spkg-configure to achieve the same

comment:3 Changed 2 years ago by mjo

You can't skip the check entirely because e.g. PKG_CHECK_MODULES puts things into global variables that we need later on.

And after some further thought, it wouldn't do me much good to skip the checks anyway. Ultimately, for using sage to do computations, I would prefer to use the system copy. But that only works until I want to review a ticket; then I have to build a git checkout and run the test suite.

The spkg-configure work is useful for me because it cuts down that compile time from 20+ hours, to (now) about 5. But that's only helpful if the resulting test suite still passes. If we modify the spkg-configures to only accept specific versions, then the test suite will evolve to support only those specific versions. Skipping the version checks at that point is only going to build something that's useless for running a ptestlong.

comment:4 Changed 2 years ago by dimpase

we can have a special macro that makes some tests conditional, or a special section of SAGE_SPKG_CONFIGURE, which contains tests skipped if whatever is set.

E.g. the Pari #29494 problem won't affect anyone who does not work with hyperelliptic curves...

comment:5 Changed 2 years ago by gh-kliem

(In an ideal world) When I install sage I want all doctests to pass by default. This would also be great for automated testing, because then we don't have to keep a huge lists of things that fail here and there.

In #29493 I suggested a similar flag that skips some tests, when we don't use the package installed by sage. Of course this shouldn't be serious errors, but maybe the representation slightly differs.

In the case of #29494 it would be nice if I am informed at configuration time that pari is rejected because of a bug in hyperelliptic curves. Then, if I decide I don't care about hyperelliptic curves, I can insist on using pari anyway. In a perfect world it would even tell me at doctesting that there is 1 expected failure (because of my configuration).

Maybe we could have a flag as # expected failure trac:12345, such that at configuration time it detects that those doctests aren't expected to work.

This might be hard to maintain but I believe it gives great flexibility to installation (and at installation time you know pretty good which parts of sage you are breaking). And it only exposes information that we know by automated testing anyway.

comment:6 Changed 2 years ago by gh-kliem

Anyway, I don't have strong opinions on which road to take. I just think we should find a rather consistent way of how to deal with those things.

comment:7 Changed 2 years ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

comment:8 Changed 2 years ago by mkoeppe

  • Cc vdelecroix added

comment:9 Changed 2 years ago by slabbe

I think force is a better name for the new whatever option. (When I do rm -f file it does not return an error if file does not exist, and that's the point, no?)

What is the difference between yes and the current force?

comment:10 Changed 2 years ago by dimpase

  • yes: it will try the system lib, and fallback to Sage one if it does not pass the tests.
  • force: it will try the system lib and fail with an error if it does not pass the tests.
Last edited 2 years ago by dimpase (previous) (diff)

comment:11 Changed 2 years ago by gh-kliem

I agree that this new whatever option really sounds like force.

How about a message that requires user confirmation? Similar to the message when you want to install polymake (or any experimental message). But this might be awful for automated testing.

Maybe we instead find a reasonable new name for the force option. How about require.

I think it might even be reasonable to move the current force option to yes. If I tell configure to use my system package, but it can't, IMO it should terminate and not just go ahead and not use the system package.

comment:12 follow-up: Changed 2 years ago by mkoeppe

Changing no/yes/force/whatever to no/yes/require/force is fine with me, but I think I might prefer no/check/yes/force (new default: check)

comment:13 in reply to: ↑ 12 Changed 2 years ago by gh-kliem

Replying to mkoeppe:

but I think I might prefer no/check/yes/force (new default: check)

Sounds good to me.

comment:14 Changed 2 years ago by slabbe

I agree too.

comment:15 Changed 2 years ago by mkoeppe

  • Description modified (diff)
  • Summary changed from ./configure --with-system-SPKG=whatever to Rename ./configure --with-system-SPKG option values to 'no'/'check'/'yes' and add new value 'force'

comment:16 Changed 2 years ago by mkoeppe

Updated the ticket description accordingly.

comment:17 Changed 2 years ago by gh-EnderWannabe

  • Branch set to u/gh-EnderWannabe/berkovich_dynamical

comment:18 follow-up: Changed 2 years ago by dimpase

  • Commit set to 2261e04c76d35c43f82e7c005370e319f7faff8d

hmm, what is this branch doing here?!


Last 10 new commits:

b3ea2e829949: reformated classcalls, docs
71c3fd329949: added image of type III points for number fields
af1b33929949: better check for poles in type III disk in padic case
1327b8a29844 doc clean-up
eaa636c29844: deleted duplicate functions. added element file to docs
b72d17829844: fixed parent error in involution map
c1becdb29844: minor doc updates
d54619e29949: Merge branch 'berkovich_revisions' into berkovich_dynamical
67feb5d29949: better check for poles in type III call
2261e0429499: added as_scheme_dynamical_system and fixed radius for image of type III

comment:19 in reply to: ↑ 18 Changed 2 years ago by gh-EnderWannabe

Replying to dimpase:

hmm, what is this branch doing here?!

Yikes! I'm so sorry, I meant to push to 29949. How do you want to handle this? I can try and fix it.

  • edit: did this ticket have a branch?
Last edited 2 years ago by gh-EnderWannabe (previous) (diff)

comment:20 Changed 2 years ago by dimpase

  • Branch u/gh-EnderWannabe/berkovich_dynamical deleted
  • Commit 2261e04c76d35c43f82e7c005370e319f7faff8d deleted

there was no branch. Just manually enter u/gh-EnderWannabe/berkovich_dynamical into the Branch: field of #29949

comment:21 Changed 2 years ago by dimpase

Actually #29949 already has it set, so no need to do anything (I've cleared the Branch: field here).

comment:22 Changed 20 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:23 Changed 16 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

comment:24 Changed 12 months ago by mkoeppe

  • Milestone changed from sage-9.4 to sage-9.5

comment:25 Changed 7 months ago by mkoeppe

  • Milestone changed from sage-9.5 to sage-9.6

comment:26 Changed 3 months ago by mkoeppe

  • Milestone changed from sage-9.6 to sage-9.7
Note: See TracTickets for help on using tickets.