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

spkg-configure.m4 for sympow #29673

Closed
dimpase opened this issue May 10, 2020 · 18 comments
Closed

spkg-configure.m4 for sympow #29673

dimpase opened this issue May 10, 2020 · 18 comments

Comments

@dimpase
Copy link
Member

dimpase commented May 10, 2020

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

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

CC: @orlitzky @mkoeppe @isuruf

Component: build: configure

Author: Dima Pasechnik

Branch: 99ebe04

Reviewer: Michael Orlitzky

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

@dimpase dimpase added this to the sage-9.2 milestone May 10, 2020
@dimpase
Copy link
Member Author

dimpase commented May 10, 2020

Commit: 2113187

@dimpase
Copy link
Member Author

dimpase commented May 10, 2020

New commits:

2113187spkg-configure for sympow

@dimpase
Copy link
Member Author

dimpase commented May 10, 2020

Author: Dima Pasechnik

@dimpase
Copy link
Member Author

dimpase commented May 10, 2020

Branch: u/dimpase/packages/sympowconfig

@orlitzky
Copy link
Contributor

comment:3

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 18, 2020

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

f36ccbaspkg-configure for sympow
99ebe04add gentoo package name

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 18, 2020

Changed commit from 2113187 to 99ebe04

@dimpase
Copy link
Member Author

dimpase commented May 18, 2020

comment:5

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.

@orlitzky
Copy link
Contributor

comment:6

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.

@orlitzky
Copy link
Contributor

Reviewer: Michael Orlitzky

@dimpase
Copy link
Member Author

dimpase commented May 18, 2020

comment:8

Replying to @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.

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!

@orlitzky
Copy link
Contributor

comment:9

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

@vbraun
Copy link
Member

vbraun commented Jun 3, 2020

Changed branch from u/dimpase/packages/sympowconfig to 99ebe04

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 14, 2020

Changed commit from 99ebe04 to none

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 14, 2020

comment:11

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 14, 2020

comment:12

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

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 14, 2020

comment:13

also fedora-28-standard (https://github.com/mkoeppe/sage/runs/867731144)

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 14, 2020

comment:14

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

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