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

Various prereq fixes #14406

Closed
jdemeyer opened this issue Apr 3, 2013 · 64 comments
Closed

Various prereq fixes #14406

jdemeyer opened this issue Apr 3, 2013 · 64 comments

Comments

@jdemeyer
Copy link
Contributor

jdemeyer commented Apr 3, 2013

  • Allow Sage to build on Cygwin without setting SAGE_PORT.
  • Don't check for sqrtl() which is apparently not needed (Cygwin doesn't have it, yet Sage builds).
  • Remove all checks for the deprecated variable SAGE_FORTRAN_LIB.
  • Remove AC_ARG_VAR(SAGE_FORTRAN) which is a deprecated variable.
  • Check for tar before actually untarring the prereq tarball (originally Test for a GNU tar on Solaris is broken #14407), only do these checks if SAGE_PORT is unset.

Copy http://boxen.math.washington.edu/home/jdemeyer/spkg/prereq-1.2.tar.gz to spkg/base (diff)

Apply attachment: trac_14406+reviewer-v3.patch

CC: @jdemeyer @nexttime @jpflori

Component: porting

Author: Jeroen Demeyer, Jean-Pierre Flori

Reviewer: Jean-Pierre Flori, Jeroen Demeyer

Merged: sage-5.9.beta4

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

@jdemeyer jdemeyer added this to the sage-5.9 milestone Apr 3, 2013
@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Apr 3, 2013

Author: Jeroen Demeyer

@jdemeyer

This comment has been minimized.

@jdemeyer jdemeyer changed the title Update prereq for Cygwin Various prereq fixes Apr 3, 2013
@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Apr 3, 2013

comment:5

Attachment: prereq-1.2.diff.gz

@jpflori
Copy link
Contributor

jpflori commented Apr 3, 2013

comment:6

In the prereq install script, the Solaris part could be a little bit polished.
The first sentence includes a disturbing also:

You MUST also use the GNU version of tar...

whereas nothing precedes it.

I don't really like the fact that both tar and make are tested together in the constructions

if [ -f "/usr/sfw/bin/gtar" ]  && [ -f "/usr/sfw/bin/gmake" ] ; then

The only advantage of doing this is to easily suggest to do

   echo "$ mkdir \$HOME/bins-for-sage" 
   echo "$ cp /usr/sfw/bin/gmake \$HOME/bins-for-sage/make" 
   echo "$ cp /usr/sfw/bin/gtar \$HOME/bins-for-sage/tar" 
   echo "$ export PATH=\$HOME/bins-for-sage:\$PATH" 

I think we'd better split the two tests, i.e. only check for tar in the tar part and for make in the make part.
At worst, a naive user will have to run make twice before it passes the prereq checks and will have twice $HAME\bins-for-sage in its PATH.

There are also ugly double spaces before "&&" on the (new) lines 132 and 165 :)

@jpflori
Copy link
Contributor

jpflori commented Apr 3, 2013

comment:7

We could also include the Solaris parts together with the HP-UX and other strange Unix oses as elif clauses a little down below.

That may look nicer and prevent running "tar --version" twice.

If so, the commented out comments will need just a little rewriting and relocation.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Apr 3, 2013

comment:8

OK, making changes...

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 3, 2013

comment:9

Didn't we want to replace the check for sqrtl() by a check for ?gammal()? (OTOH I don't recall why exactly... ;-) )

Actually, we need both sqrtl() and [lt]gammal() unless we build Cephes, i.e., unless we're on Cygwin(?) or on FreeBSD, or skip the test because SAGE_PORT is set.

@kcrisman
Copy link
Member

kcrisman commented Apr 3, 2013

comment:10

Also recall we still don't have logl (#14078) and so things are still weird, though we worked around it there. But I don't have a problem with the Cygwin stuff here; Sage builds as is on Cygwin. I think David originally added that check just to make sure one didn't need stuff like Cephes, but apparently we don't actually use sqrtl() and friends.

@jpflori
Copy link
Contributor

jpflori commented Apr 3, 2013

comment:11

Replying to @nexttime:

Didn't we want to replace the check for sqrtl() by a check for ?gammal()? (OTOH I don't recall why exactly... ;-) )

Actually, we need both sqrtl() and [lt]gammal() unless we build Cephes, i.e., unless we're on Cygwin(?) or on FreeBSD, or skip the test because SAGE_PORT is set.

We don't build Cephes on Cygwin.
On Cygwin, the only *l functions actually needed anywhere in Sage and which caused troubles (and that Cygwin libm does not provide, not sure what it actually provides, but I seem to remember it provides no "long double" functions at all) was "logl" because of R, but we patched R so that its not needed anymore, and the next R upstream version provides a configure option to disable the use of "long double" functions anyway.

@jpflori
Copy link
Contributor

jpflori commented Apr 3, 2013

comment:12

Here is what was recently included in Cygwin:

Support for the C99 complex functions, except for the "long double" implementations. New APIs: cacos, cacosf, cacosh, cacoshf, carg, cargf, casin, casinf, casinh, casinhf, catan, catanf, catanh, catanhf, ccos, ccosf, ccosh, ccoshf, cexp, cexpf, cimag, cimagf, clog, clogf, conj, conjf, cpow, cpowf, cproj, cprojf, creal, crealf, csin, csinf, csinh, csinhf, csqrt, csqrtf, ctan, ctanf, ctanh, ctanhf. 

See http://cygwin.com/cygwin-ug-net/ov-new1.7.html.

@jpflori
Copy link
Contributor

jpflori commented Apr 3, 2013

comment:13

And I don't know what the FreeBSD libm lacks that make Sage's compilation fail (except for the logl function for R of course).

@jpflori
Copy link
Contributor

jpflori commented Apr 3, 2013

comment:14

And of course we could revert the patch for R from #14078 and let Cephes install again on Cygwin.
It is just that at the time we were working on #14078, patching R, which is about to get the "without long double" support in a future release seemed easier.
I cannot not promise that installing Cephes and then trying to install R without the patch will just work out of the box.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Apr 3, 2013

comment:15

Attachment: 14406_cygwin_prereq.patch.gz

Replying to @nexttime:

Actually, we need both sqrtl() and [lt]gammal() unless we build Cephes

It seems not, Cygwin doesn't have sqrtl(), nor does it build Cephes. Despite this, Sage builds.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Apr 3, 2013

comment:16

OK, simplified the tar/make checks.

@jpflori
Copy link
Contributor

jpflori commented Apr 3, 2013

comment:17

Ok, I've slightly recomplexified it :)

@jpflori
Copy link
Contributor

jpflori commented Apr 3, 2013

comment:18

Oops, something is wrong, coming back with a proper patch.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 3, 2013

comment:19

Replying to @jdemeyer:

Replying to @nexttime:

Actually, we need both sqrtl() and [lt]gammal() unless we build Cephes

It seems not, Cygwin doesn't have sqrtl(), nor does it build Cephes. Despite this, Sage builds.

IIRC, because we (still) have some special-casing on Cygwin w.r.t. at least (just?) the (long double) gamma functions.

To repeat Karl-Dieter's question: What is Cephes then needed for at all [on e.g. FreeBSD]?

I.e., we should probably check for those functions Cephes provides (and Sage needs) [unless we're on FreeBSD where Cephes gets built anyway, or SAGE_PORT is set].

@jpflori
Copy link
Contributor

jpflori commented Apr 3, 2013

comment:20

Replying to @nexttime:

Replying to @jdemeyer:

Replying to @nexttime:

Actually, we need both sqrtl() and [lt]gammal() unless we build Cephes

It seems not, Cygwin doesn't have sqrtl(), nor does it build Cephes. Despite this, Sage builds.

IIRC, because we (still) have some special-casing on Cygwin w.r.t. at least (just?) the (long double) gamma functions.

I'll have a look.

To repeat Karl-Dieter's question: What is Cephes then needed for at all [on e.g. FreeBSD]?

Maybe for complex functions "c*" (which are now in Cygwin libm but used not to be)?

I.e., we should probably check for those functions Cephes provides (and Sage needs) [unless we're on FreeBSD where Cephes gets built anyway, or SAGE_PORT is set].

@jpflori
Copy link
Contributor

jpflori commented Apr 3, 2013

comment:39

Replying to @jdemeyer:

Replying to @kcrisman:

Unless I'm misunderstanding something about your question?

You didn't really answer my question, you confirmed that it is needed without mentioning why it is needed. Perhaps a more explicit question is: what goes wrong if you don't have lapack?

On Cygwin the lapack packages provide both BLAS and LAPACK.
And the current Sage's ATLAS spkg, or the one from #10508, does not build ATLAS on Cygwin.
It will not be really hard, but requires some more thoughts and work.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Apr 3, 2013

comment:40

Replying to @jpflori:

But will there really be anything else than SunOS where /usr/sfw/bin exists?

Assuming not, the check for SunOS is redundant anyway.

It all goes back to the philosophy of not checking the system, but checking features.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Apr 3, 2013

comment:41

Replying to @jpflori:

And the current Sage's ATLAS spkg, or the one from #10508, does not build ATLAS on Cygwin.
It will not be really hard, but requires some more thoughts and work.

OK, so it's a problem with the ATLAS spkg. In that case, would it not make more sense to add the check to the ATLAS spkg instead of prereq?

@jpflori
Copy link
Contributor

jpflori commented Apr 3, 2013

comment:42

Attachment: trac_14406+reviewer-v3.patch.gz

Replying to @jdemeyer:

Replying to @jpflori:

And the current Sage's ATLAS spkg, or the one from #10508, does not build ATLAS on Cygwin.
It will not be really hard, but requires some more thoughts and work.

OK, so it's a problem with the ATLAS spkg. In that case, would it not make more sense to add the check to the ATLAS spkg instead of prereq?

Its already there and provides info.
I'm ok to not check anything in prereq and let the ATLAS spkg do the job (although the user might say "hey it passed prereq and now its asking for something more to be installed so much later!!!").

(And I just uploaded a new patch without redundant SunOS check.)

@jpflori

This comment has been minimized.

@jpflori
Copy link
Contributor

jpflori commented Apr 3, 2013

comment:43

Replying to @jdemeyer:

Replying to @jpflori:

Indeed, we cheat for logl/sqrtl/tgammal/lgammal in sage/symbolic/pynac_cc.h just as we do with the R patch for logl.

I wouldn't mind always "cheating", i.e. not supporting logl() at all and drop all the Cygwin/FreeBSD hassle. Especially because long doubles don't have a portable meaning (they have a different precision on different systems).

Ive got a slight preference for the SAGE_INST_CEPHES solution, espoecially it "could" be easier to maintain, but for the moment I could go with not even testing sqrtl anywhere assuming that its in the libm of supported platforms or were dealing with Cygwin or FreeBSD.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Apr 3, 2013

comment:44

Reviewer patch is okay for me.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Apr 3, 2013

comment:45

Replying to @jpflori:

Ive got a slight preference for the SAGE_INST_CEPHES solution

I don't really know what that "solution" would be.

The main thing I don't want is to spread the same checks in different places. For example, the lapack check should be either in prereq, either in ATLAS, not in both. Since it currently cannot be done only in prereq, it must be done in ATLAS.

@jpflori
Copy link
Contributor

jpflori commented Apr 3, 2013

comment:46

Replying to @jdemeyer:

Replying to @jpflori:

Ive got a slight preference for the SAGE_INST_CEPHES solution

I don't really know what that "solution" would be.

That would be:

  • check for sqrtl.
  • if not available trigger installation of the cephes spkg.

The main thing I don't want is to spread the same checks in different places. For example, the lapack check should be either in prereq, either in ATLAS, not in both. Since it currently cannot be done only in prereq, it must be done in ATLAS.

@kcrisman
Copy link
Member

kcrisman commented Apr 4, 2013

comment:47

Unless I'm misunderstanding something about your question?

You didn't really answer my question, you confirmed that it is needed without mentioning why it is needed. Perhaps a more explicit question is: what goes wrong if you don't have lapack?

On Cygwin the lapack packages provide both BLAS and LAPACK. And the current Sage's ATLAS spkg, or the one from #10508, does not build ATLAS on Cygwin. It will not be really hard, but requires some more thoughts and work.

I thought that was basically what I was saying... not sure what happened here.

Anyway, as to the check, the reason to have it early is that atlas might build quite late in a process, and then someone says "Oh, Mxyzptlk, why did I waste all that time building Sage and I didn't have the right prereqs?" Which I guess JP just said too.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Apr 4, 2013

comment:48

Replying to @jpflori:

That would be:

  • check for sqrtl.
  • if not available trigger installation of the cephes spkg.

And how would a prereq check "trigger" the installation of the cephes spkg? I don't think that can currently be done.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Apr 4, 2013

comment:49

Replying to @kcrisman:

Anyway, as to the check, the reason to have it early is that atlas might build quite late in a process, and then someone says "Oh, Mxyzptlk, why did I waste all that time building Sage and I didn't have the right prereqs?" Which I guess JP just said too.

I certainly don't disagree with this, but for this to work (and also jpflori's idea of "triggering"), we need to change the prereq mechanism which is a lot less trivial than this ticket.

@jpflori
Copy link
Contributor

jpflori commented Apr 4, 2013

comment:50

Replying to @jdemeyer:

Replying to @jpflori:

That would be:

  • check for sqrtl.
  • if not available trigger installation of the cephes spkg.

And how would a prereq check "trigger" the installation of the cephes spkg? I don't think that can currently be done.

I thought we could do that just like you do for GCC.
But that might be impossible, i did not check.

@jpflori
Copy link
Contributor

jpflori commented Apr 4, 2013

comment:51

Replying to @jpflori:

Replying to @jdemeyer:

Replying to @jpflori:

That would be:

  • check for sqrtl.
  • if not available trigger installation of the cephes spkg.

And how would a prereq check "trigger" the installation of the cephes spkg? I don't think that can currently be done.

I thought we could do that just like you do for GCC.
But that might be impossible, i did not check.

Just a little more details of the way I see this, but that may be wrong:

  • at prereq time (or whenever the gcc version checks are done), set an env variable if the test fails (or the user can override it if he wants),
  • then either our spkg system is smart enough to build the cephes spkg iff this variable is set, or the spkg-install script from cephes spkg checks its value by itself and decide whether it should build or not (just like it now checks it is running on FreeBSD).

@jpflori
Copy link
Contributor

jpflori commented Apr 4, 2013

comment:52

And I'm in for the idea of "let's postpone this complicated stuff" (for the ATLAS and Cephes stuff).

If nobody rants too loud, I'll put this ticket to positive review so that we have official "experimental" for Cygwin in 5.9 or 5.10.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Apr 4, 2013

comment:53

Replying to @jpflori:

I thought we could do that just like you do for GCC.

Not really, the check for GCC is done in spkg/install, not prereq.

I agree that all these checks should eventually move to a new file $SAGE_ROOT/configure, but that would require some refactoring of the build system. Since the Sage+GIT people are also refactoring the build system, it might make sense to wait for sage-6.1 to do this.

@jpflori
Copy link
Contributor

jpflori commented Apr 4, 2013

comment:54

Agreed, let's move on and merge this considering:

  • sqrtl is not really a prereq as it should be on all supported platform or provided either by cheating or by building cephes (and note FreeBSD is even not officialy supported). Later we can give it a similar treatment as GCC. We can wait for 6.1 to do this.
  • the ATLAS thing is currently only somehow boring on Cygwin, but the ATLAS spkg takes care of this (although later than the prereq one) and that is planned to change. For the moment what will happen is that ATLAS build will fail and tell the user to install the needed packages, he will do that and then the build process will go on. I guess someone actually trying to compile a huge thing as Sage can live with that. We should already wait for Update ATLAS to stable version 3.10.1 #10508 to be merged before doing that.

@jpflori
Copy link
Contributor

jpflori commented Apr 4, 2013

Reviewer: Jean-Pierre Flori, Jeroen Demeyer

@jpflori
Copy link
Contributor

jpflori commented Apr 4, 2013

Changed author from Jeroen Demeyer to Jeroen Demeyer, Jean-Pierre Flori

@jpflori
Copy link
Contributor

jpflori commented Apr 4, 2013

comment:55

I've opened #14410 for the ATLAS on Cygwin stuff.

@kcrisman
Copy link
Member

kcrisman commented Apr 4, 2013

comment:56

Okay, this is better than the alternatives.

@jdemeyer
Copy link
Contributor Author

jdemeyer commented Apr 4, 2013

Merged: sage-5.9.beta4

@jpflori
Copy link
Contributor

jpflori commented Apr 11, 2013

comment:58

Groumpf, I just realized that the last elif clause filtering OS names should include Cygwin (and beware that uname does not just return CYGWIN but CYGWIN... where ... gives additional precision which we don't really care about at the moment).
Should we fix that on another ticket?

@jdemeyer
Copy link
Contributor Author

comment:59

Replying to @jpflori:

Should we fix that on another ticket?

Yes, I made #14447 but I'll let you write the patch.

As for the Cygwin version thing: I propose

if uname | grep CYGWIN >/dev/null; then
    ...
fi

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

3 participants