sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.37k stars 466 forks source link

Update the "Install from Source Code" section of the Sage Installation Guide #11081

Closed bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 closed 13 years ago

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago

The Sage installation guide has a section on installing Sage from source

http://www.sagemath.org/doc/installation/source.html

but it is inaccurate, incomplete and dangerous in certain places. Specific problems are below. These are roughly listed in the order they appear in the manual.

I've put quite a bit of work into this, and I think generally it is an improvement. I'll be receptive to constructive comments. But PLEASE PLEASE PLEASE can we avoid what happens on tickets some times, when tons of people argue about the smallest of details and tickets drag on for months. If the comments drag on and on, I might ask that someone review the patch as it is, and create another ticket to fix further changes. People can always create other tickets for other changes.

Notes for reviewers and the release manager

  1. Apply only attachment: 11081_all.patch

CC: @jhpalmieri @kcrisman

Component: documentation

Author: David Kirkby

Reviewer: Karl-Dieter Crisman, Florent Hivert, John Palmieri, Dmitrii Pasechnik, Jeroen Demeyer

Merged: sage-4.7.alpha4

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

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago

Mercurial patch for changes to documentation. I hope this is an improvement, and hope requested changes don't drag on for a long time.

jhpalmieri commented 13 years ago
comment:1

Attachment: 11081-install-from-source.patch.gz

kcrisman commented 13 years ago
comment:3

A few comments for improvement:

All the updates etc. and changes for more accurate documentation look fine, though I'm sure more knowledgeable folks will have some minor changes there, since it's a rule that any patch longer than 1 character has a bug :-)

Nice work cleaning this up, Dave. We are very fortunate that Sage has enough people who care about this kind of thing that we do eventually clean them up!

kcrisman commented 13 years ago

Reviewer: Karl-Dieter Crisman

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:4

Replying to @kcrisman:

A few comments for improvement:

  • I think it would be nice to have at least the main "fully supported" versions here. No need to go clicking through if you are on a normal system, then, and also it will be clear that e.g. Windows 7 is not on that list just by looking at it. Something like "supported on many Linux distributions, Mac OS X, and 32-bit Solaris; see ... for details and status of other systems." I'm not wedded to that phrasing, but it's reasonable to have at least minimal info.

OK, point taken. I'll keep that to the bare minimum though. I don't think its true to say Sage is "supported on many Linux distributions" when we only test on a few. The issue we had before was a huge range of "supported" Linux distributions, which nobody had tested on for ages, if at all.

  • A minor note should be added that gfortran is not provided by Xcode on OS X, but that Sage provides a suitable Fortran compiler for OS X.

I can add that, though the current documentation does say in this very file:

On Mac OS X, you are not required to have a Fortran compiler on your system. The Sage source distribution is shipped with a Fortran compiler for Mac OS X. This Fortran compiler is used, unless you specify another Fortran compiler via the variable SAGE_FORTRAN.

  • Even for my ancient < 1 GHz Emac (probably from around 2003), it takes less than a minute for Sage to start up the first time (though it's close!). I think it is more reasonable to say "starting the first time may take more than a few seconds; a few platforms will take longer for reason X" where it is clear that reason X is something about the platform, not Sage.

I hear plenty of complaints on sage-devel of Sage taking a very long time to start.

  • "Note from David Kirkby. " - is that going to look ok in the docs? Maybe instead using the ".. note:" syntax...

In my opinion it looked ok, but perhaps I'm biased. I don't know how to use the note syntax. I wanted to make this quite prominent, as I think its a pretty important point. We are documenting how to do something that any decent system administer knows one should never do.

All the updates etc. and changes for more accurate documentation look fine, though I'm sure more knowledgeable folks will have some minor changes there, since it's a rule that any patch longer than 1 character has a bug :-)

Yes, I just want to avoid it becoming like tickets have become. I feel quite sorry for John, as I feel that reviewers have requested changes far outside the scope of some of John's tickets.

Nice work cleaning this up, Dave. We are very fortunate that Sage has enough people who care about this kind of thing that we do eventually clean them up!

I think documentation will always tend to be out of date. We can however limit that by not being too specific. The original version saying developers use gcc 4.3.x was just asking for trouble.

Dave

kcrisman commented 13 years ago
comment:5
  • A minor note should be added that gfortran is not provided by Xcode on OS X, but that Sage provides a suitable Fortran compiler for OS X.

I can add that, though the current documentation does say in this very file:

On Mac OS X, you are not required to have a Fortran compiler on your system. The Sage source distribution is shipped with a Fortran compiler for Mac OS X. This Fortran compiler is used, unless you specify another Fortran compiler via the variable SAGE_FORTRAN.

Is it anywhere near where your thing is? My concern was people on OS X thinking they needed to get/had gfortran.

  • Even for my ancient < 1 GHz Emac (probably from around 2003), it takes less than a minute for Sage to start up the first time (though it's close!). I think it is more reasonable to say "starting the first time may take more than a few seconds; a few platforms will take longer for reason X" where it is clear that reason X is something about the platform, not Sage.

I hear plenty of complaints on sage-devel of Sage taking a very long time to start.

They mean >5 sec, or >2 sec. on a fresh cache. Not minutes. Anyway, there are very few systems, I think, where 'minutes' is the right word, and we don't want to scare people.

  • "Note from David Kirkby. " - is that going to look ok in the docs? Maybe instead using the ".. note:" syntax...

In my opinion it looked ok, but perhaps I'm biased. I don't know how to use the note syntax. I wanted to make this quite prominent, as I think its a pretty important point. We are documenting how to do something that any decent system administer knows one should never do.

I haven't looked at it in the doc yet, so we'll see when we get there.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:6

Replying to @kcrisman:

  • A minor note should be added that gfortran is not provided by Xcode on OS X, but that Sage provides a suitable Fortran compiler for OS X.

I can add that, though the current documentation does say in this very file:

On Mac OS X, you are not required to have a Fortran compiler on your system. The Sage source distribution is shipped with a Fortran compiler for Mac OS X. This Fortran compiler is used, unless you specify another Fortran compiler via the variable SAGE_FORTRAN.

Is it anywhere near where your thing is? My concern was people on OS X thinking they needed to get/had gfortran.

They mean >5 sec, or >2 sec. on a fresh cache. Not minutes. Anyway, there are very few systems, I think, where 'minutes' is the right word, and we don't want to scare people.

Fair enough. As a matter of interest, William wrote here

http://www.mail-archive.com/sage-devel@googlegroups.com/msg46733.html

''"Indeed, if it takes 5-10 minutes for Sage to start (and yes it does on some slow filesystems), then this wouldn't help at all"''

but I guess that is the exception rather than the rule. It takes 30 s on my 2.0 GHz laptop. It appears to be I/O limited. I'll mention that.

  • "Note from David Kirkby. " - is that going to look ok in the docs? Maybe instead using the ".. note:" syntax...

In my opinion it looked ok, but perhaps I'm biased. I don't know how to use the note syntax. I wanted to make this quite prominent, as I think its a pretty important point. We are documenting how to do something that any decent system administrator knows one should never do.

I haven't looked at it in the doc yet, so we'll see when we get there.

I'll make some revisions and post an update. Then perhaps you can generate the documentation and look at in a browser.

Dave

jdemeyer commented 13 years ago
comment:7

Line 131 in the new file: it is false that PARI needs perl (it might have been true some time in the past, I don't know). Only the PARI/GP documentation (not installed or used by default) needs perl.

Line 752 in the new file: the comment about PARI's galois data files is also completely outdated and should be removed.

By the way, "starting the first time can take a few minutes" is totally true when you have a slow disk on a busy system (e.g. sage.math.washington.edu)

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:8

Replying to @jdemeyer:

Line 131 in the new file: it is false that PARI needs perl (it might have been true some time in the past, I don't know). Only the PARI/GP documentation (not installed or used by default) needs perl.

Line 752 in the new file: the comment about PARI's galois data files is also completely outdated and should be removed.

I'll make those changes.

By the way, "starting the first time can take a few minutes" is totally true when you have a slow disk on a busy system (e.g. sage.math.washington.edu)

How about saying:

"Sage should take well under a minute to start for the first time, but it can take a few minutes on a slow or busy file system?"

I did also think of putting something to the effect of

If possible install Sage on a local disk or fast system, as Sage loads a large number of files, which can mean Sage is slow to start on some file systems.

The trouble with these changes is that they are subjective. Most of the other points are just correcting indisputable facts.

I think one useful addition to the bottom of a page might be

"Last updated on x/y/z"

so hopefully we can find pages that are well out of date.

It's hard to believe that a page as important as describing how to build Sage from source has so many inaccuracies in it. I suspect this has not been properly updated for a few years.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:9

Replying to @jdemeyer:

Line 131 in the new file: it is false that PARI needs perl (it might have been true some time in the past, I don't know). Only the PARI/GP documentation (not installed or used by default) needs perl.

Thinking about this particular point, I don't see the need for documenting what component(s) of Sage uses Perl. I added R, as there were already packages mentioned, but really all the user needs to know is that they need a version of Perl that is >= 5.8.0. Whether it's use by R, NTL, Pari or foobar is not really important.

Dave

dimpase commented 13 years ago

Replying to @sagetrac-drkirkby

regarding the version of XCode for MacOSX, I think it's safer to say that we expect a recent version of XCode for the relevant MacOSX platform version, as we don't test on outdated XCode versions.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:11

Replying to @dimpase:

Replying to @sagetrac-drkirkby

regarding the version of XCode for MacOSX, I think it's safer to say that we expect a recent version of XCode for the relevant MacOSX platform version, as we don't test on outdated XCode versions.

Thank you. I'll say "recent" and suggest looking at http://wiki.sagemath.org/SupportedPlatforms for the supported versions of Xcode. That way, we can more easily make changes on the Wiki. Since this page does not tend to get updated very often, I want to avoid putting anything that is likely to be out of date soon.

I'll apply another later, with changes people have suggested.

Dave

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:12

I'm attaching a patch which makes the following changes - again roughly listed in the order they are mentioned in the document.

grep "An error occurred" spkg/logs/*

if a build error occurs, and to put the output of that on sage-support. (Previously the documentation said to attach install.log, but its totally impractical to attach a file the size of install.log to an email).

I believe there are still improvements that could be made, but believe the changes made are a good compromise between improving the accuracy and spending too much time over minor details.

Dave

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago

Additioanl information, addressing reviewer comments. To be applied after 11081-install-from-source.patch

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago

Description changed:

--- 
+++ 
@@ -29,3 +29,8 @@

 I've put quite a bit of work into this, and I think generally it is an improvement. I'll be receptive to constructive comments. But **PLEASE PLEASE PLEASE** can we avoid what happens on tickets some times, when tons of people argue about the smallest of details and tickets drag on for months. If the comments drag on and on, I might ask that someone review the patch as it is, and create another ticket to fix further changes. People can always create other tickets for other changes. 
+
+## Notes for reviewers and the release manager
+* 1) Apply 11081-install-from-source.patch 
+* 2) Apply 11081-install-from-source-Additional-information.patch
+
bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:13

Attachment: 11081-install-from-source-Additional-information.patch.gz

jdemeyer commented 13 years ago
comment:14

Replying to @sagetrac-drkirkby:

I think one useful addition to the bottom of a page might be

"Last updated on x/y/z"

Personally, I dislike non-automatic "last updated" lines like this one. People will forget to update the "last updated" line when making changes to the file. Also, I don't see much usefulness in such a line.

It would be really good to have this in sage-4.7.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:15

Replying to @jdemeyer:

Replying to @sagetrac-drkirkby:

I think one useful addition to the bottom of a page might be

"Last updated on x/y/z"

Personally, I dislike non-automatic "last updated" lines like this one.

Do you know a relieable automated way?

People will forget to update the "last updated" line when making changes to the file.

Well, hopefully either the author or reviewer would notice.

The worst that can happen if someone forgets to update the line (and the reviewer does not notice), is that users will think the information is more outdated than it really is.

Also, I don't see much usefulness in such a line.

Users can determine if the information is current, which is quite important with information like this. It can tend to go out of date. If users see the information was written a year or more ago, they might realise its probably inaccurate.

It would be really good to have this in sage-4.7.

I'm glad you think it would be useful. I'm surprised you updated it to blocker (though I'm not complaining, but I'd like to see 'prerequisite checks #11070 / #9978 in more!) But more seriously, we really need to stop such an important part of the documentation getting so out of date. Clearly many people will build Sage from source, yet so much of the information was inaccurate.

If you feel strongly about it, I'll remove the "last update" bit.

Dave

hivert commented 13 years ago
comment:16

Just a quick remark: I was passing by and I noticed that the it following paragraph there is a parenthesis opened but not closed


    73  (You must have the GNU version of ``make`` installed and it must be the first 
    74  ``make`` in your PATH. On Solaris, a version of GNU ``make`` may be found  
    75  at ``/usr/sfw/bin/gmake`` but you will need to copy it somewhere else 
    76  and rename it to ``make``. The same is true for GNU tar - there is a version 
    77  called ``gtar`` in ``/usr/sfw/bin`` but it will need to be copied somwhere 
    78  else and renamed to ``tar``.  
jdemeyer commented 13 years ago
comment:17

Replying to @sagetrac-drkirkby:

If you feel strongly about it, I'll remove the "last update" bit.

I do not feel strongly about it.

jhpalmieri commented 13 years ago
comment:18

Would it be better to change the paragraph on supported systems from

Sage is supported on ...

to

As of April 2011, Sage is supported on ...

Then anyone who changes the list of supported platforms are more likely to see the date than if the date is at the bottom of the document. (I know the other information in the document is also time-sensitive, but not quite as much as the list of supported platforms.)

Alternatively, we could add something like this to the very top of the file:

.. comment:
   ****************************
   If you alter this document, please change the last line ("This page
   was last updated on ...")
   ****************************
bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:19

I'll attach another patch in a couple of minutes, using the comment as suggested by John. I also noticed another typo, and a couple of very minor changes.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago

Changed reviewer from Karl-Dieter Crisman to Karl-Dieter Crisman, Florent Hivert, John Palmieri, Dmitrii Pasechnik, Jeroen Demeyer

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago

Attachment: 11081-minor-changes.patch.gz

To be applied after the two earlier patches

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:20

I'm marking this for "needs review" now.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago

Description changed:

--- 
+++ 
@@ -33,4 +33,4 @@
 ## Notes for reviewers and the release manager
 * 1) Apply 11081-install-from-source.patch 
 * 2) Apply 11081-install-from-source-Additional-information.patch
-
+* 3) Apply 11081-minor-changes.patch 
bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago

Patch to advise uses not to use gcc 4.3.2, which Bill Hart said on sage-devel today that it mis-compiles MPIR on x64.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago

Description changed:

--- 
+++ 
@@ -34,3 +34,4 @@
 * 1) Apply 11081-install-from-source.patch 
 * 2) Apply 11081-install-from-source-Additional-information.patch
 * 3) Apply 11081-minor-changes.patch 
+* 4) Apply 11081-dont-use_GCC_4_3_2.patch
bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:21

Attachment: 11081-dont-use_GCC_4_3_2.patch.gz

I changed the supported version of gcc to say not to use gcc 4.3.2, as apparently this mis-compiles MPIR according to Bill Hart who is an MPIR developer.

jdemeyer commented 13 years ago
comment:22

David, there are so many bad versions of gcc miscompiling various packages that it's pointless to start making a list. Better state something along the lines of "(Version 4.0.1 or later, but preferably a recent version)"

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:23

Replying to @jdemeyer:

David, there are so many bad versions of gcc miscompiling various packages that it's pointless to start making a list. Better state something along the lines of "(Version 4.0.1 or later, but preferably a recent version)"

OK, will do. But is it worth documenting that some versions don't work on particular platforms, without going into details about which versions and on which platforms? (Such detail could be added to the Wiki at http://wiki.sagemath.org/SupportedPlatforms)

We currently have the situation that Sage will not build with the latest gcc. That's both the current version of Sage (4.6.2) and I suspect 4.7 will not build either, since there's no progress to my knowledge on the Singular issue - see #11084, although that has been reported upstream.

Dave

jdemeyer commented 13 years ago
comment:24

PARI does not build with:

  1. gcc 4.4.1 (#10120)
  2. gcc 4.5.1 on Itanium (#9897, http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46044)
bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago

Suggest use of recent gcc. Drop mention of gcc 4.3.2 being bad.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago

Description changed:

--- 
+++ 
@@ -35,3 +35,4 @@
 * 2) Apply 11081-install-from-source-Additional-information.patch
 * 3) Apply 11081-minor-changes.patch 
 * 4) Apply 11081-dont-use_GCC_4_3_2.patch
+* 5) Apply 11081-use-recent-gcc.patch
bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:25

Attachment: 11081-use-recent-gcc.patch.gz

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:27

OK, I dropped mention of that specific version, and made a general comment to use a recent one, but noted that particular versions have failed on specific platforms.

It would be good if there was a version which worked on every platform.

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:28

Does anyone feel able to give this a positive review? I'm sure this whole section could be cleaned up further, but I think this is a substantial improvement over what we currently have.

Dave

jdemeyer commented 13 years ago

All patches folded

jdemeyer commented 13 years ago

Description changed:

--- 
+++ 
@@ -31,8 +31,4 @@
 I've put quite a bit of work into this, and I think generally it is an improvement. I'll be receptive to constructive comments. But **PLEASE PLEASE PLEASE** can we avoid what happens on tickets some times, when tons of people argue about the smallest of details and tickets drag on for months. If the comments drag on and on, I might ask that someone review the patch as it is, and create another ticket to fix further changes. People can always create other tickets for other changes. 

 ## Notes for reviewers and the release manager
-* 1) Apply 11081-install-from-source.patch 
-* 2) Apply 11081-install-from-source-Additional-information.patch
-* 3) Apply 11081-minor-changes.patch 
-* 4) Apply 11081-dont-use_GCC_4_3_2.patch
-* 5) Apply 11081-use-recent-gcc.patch
+1. Apply only [attachment: 11081_all.patch](https://github.com/sagemath/sage-prod/files/10652525/11081_all.patch.gz)
jdemeyer commented 13 years ago
comment:29

Attachment: 11081_all.patch.gz

dimpase commented 13 years ago
comment:30

Replying to @sagetrac-drkirkby:

Does anyone feel able to give this a positive review? I'm sure this whole section could be cleaned up further, but I think this is a substantial improvement over what we currently have.

Dave

Hi Dave,

tar        (GNU tar, version 1.17 or later)

is not quite right. On MacOSX the tar is a BSD tar:

$ tar --version
bsdtar 2.6.2 - libarchive 2.6.2

Perhaps GNU tar is a Solaris-only requirement...

dimpase commented 13 years ago
comment:31

Replying to @dimpase:

Replying to @sagetrac-drkirkby:

Does anyone feel able to give this a positive review? I'm sure this whole section could be cleaned up further, but I think this is a substantial improvement over what we currently have.

OK, so that tar thing above is the only complaint I can make about the contents. The other thing one might want to think about is how to make this more version-independent, i.e. so that the text does not get obsolete when the current Sage version changes. But I leave it to the next update.

Positive review (assuming the tar comment gets fixed). Dima

jdemeyer commented 13 years ago

Merged: sage-4.7.alpha4

bac7d3ea-3f1b-4826-8464-f0b53d5e12d2 commented 13 years ago
comment:33

Replying to @dimpase:

Positive review (assuming the tar comment gets fixed). Dima

Sorry, I missed your "tar" comment, but this now now been merged and closed. I have opened #11159 to address the error about "tar". I'll fix that soon. I believe the number of errors corrected by this update far exceeds the number introduced!

Dave