Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#29673 closed enhancement (fixed)

spkg-configure.m4 for sympow

Reported by: Dima Pasechnik Owned by:
Priority: major Milestone: sage-9.2
Component: build: configure Keywords:
Cc: Michael Orlitzky, Matthias Köppe, Isuru Fernando Merged in:
Authors: Dima Pasechnik Reviewers: Michael Orlitzky
Report Upstream: N/A Work issues:
Branch: 99ebe04 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

this should be easy, just binaries (and possibly data, tbd)

in Debian it's sympow and (maybe needed) sympow-data

Change History (14)

comment:1 Changed 3 years ago by Dima Pasechnik

Authors: Dima Pasechnik
Branch: u/dimpase/packages/sympowconfig
Cc: Michael Orlitzky Matthias Köppe added
Commit: 2113187d27646192083d29925b2873bdee0e6096
Status: newneeds_review

New commits:

2113187spkg-configure for sympow

comment:2 Changed 3 years ago by Dima Pasechnik

Cc: Isuru Fernando added

comment:3 Changed 3 years ago by Michael Orlitzky

Works for me with the sympow fork. As of five minutes ago, this is sci-mathematics/sympow in Gentoo. Can you add that to gentoo.txt?

My only other question is why you settled on testing $ac_cv_path_SYMPOW instead of just $SYMPOW? The name of that cache variable is documented, so it should work just as well, but it was surprising to me.

comment:4 Changed 3 years ago by git

Commit: 2113187d27646192083d29925b2873bdee0e609699ebe047c653cccc930d667876f8c7782138d6eb

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f36ccbaspkg-configure for sympow
99ebe04add gentoo package name

comment:5 Changed 3 years ago by Dima Pasechnik

Added gentoo info.

Regarding SYMPOW vs ac_cv_path_SYMPOW, the latter seems to be what we normally use in many other spkg-configure.m4 files.

It appears that using SYMPOW is more hacky, e.g. if it's set then the test is not carried out, etc. E.g. if it's set to something not in PATH, it will probably not be found later on by sagelib.

comment:6 in reply to:  5 ; Changed 3 years ago by Michael Orlitzky

Replying to dimpase:

It appears that using SYMPOW is more hacky, e.g. if it's set then the test is not carried out, etc. E.g. if it's set to something not in PATH, it will probably not be found later on by sagelib.

You mean if the user exports SYMPOW="junk or some non-PATH location"? I think the only reason ac_cv_path_SYMPOW avoids that same problem is because of the ugly variable name. Instead of SYMPOW we could use e.g. SAGE_SYMPOW_PATH or something like that to the same end.

In another life, we could make overrides work with e.g. --with-sympow=/home/mjo/bin/sympow but I think we all have better things to do right now.

comment:7 Changed 3 years ago by Michael Orlitzky

Reviewers: Michael Orlitzky
Status: needs_reviewpositive_review

comment:8 in reply to:  6 ; Changed 3 years ago by Dima Pasechnik

Replying to mjo:

Replying to dimpase:

It appears that using SYMPOW is more hacky, e.g. if it's set then the test is not carried out, etc. E.g. if it's set to something not in PATH, it will probably not be found later on by sagelib.

You mean if the user exports SYMPOW="junk or some non-PATH location"? I think the only reason ac_cv_path_SYMPOW avoids that same problem is because of the ugly variable name. Instead of SYMPOW we could use e.g. SAGE_SYMPOW_PATH or something like that to the same end.

Autoconf docs are silent on what happens if ac_cv_path_SYMPOW is pre-set, so it appears to be more robust.

Thanks for review!

comment:9 in reply to:  8 Changed 3 years ago by Michael Orlitzky

Replying to dimpase:

Autoconf docs are silent on what happens if ac_cv_path_SYMPOW is pre-set, so it appears to be more robust.

I can't find any explicit docs, but as a cache variable, it does what you'd expect: the configure script first tests the value of ac_cv_path_SYMPOW, and if it's set, its value gets used in lieu of the check:

if ${ac_cv_path_SYMPOW+:} false; then :
  $as_echo_n "(cached) " >&6
else
  # the check gets run and ac_cv_path_SYMPOW is populated
fi
SYMPOW=$ac_cv_path_SYMPOW

comment:10 Changed 3 years ago by Volker Braun

Branch: u/dimpase/packages/sympowconfig99ebe047c653cccc930d667876f8c7782138d6eb
Resolution: fixed
Status: positive_reviewclosed

comment:11 Changed 2 years ago by Matthias Köppe

Commit: 99ebe047c653cccc930d667876f8c7782138d6eb

Follow-up: It looks like this accepts a broken sympow on fedora-32 - #3360

comment:12 Changed 2 years ago by Matthias Köppe

Also with sympow on ubuntu-bionic-standard (https://github.com/mkoeppe/sage/runs/867730944)

comment:13 Changed 2 years ago by Matthias Köppe

comment:14 Changed 2 years ago by Matthias Köppe

I've opened #30147 (Fix spkg-configure.m4 for sympow) for this. Please fix!

Note: See TracTickets for help on using tickets.