Opened 9 years ago
Closed 9 years ago
#14465 closed enhancement (fixed)
Clean source.rst and include instructions for Cygwin
Reported by: | jpflori | Owned by: | mvngu |
---|---|---|---|
Priority: | major | Milestone: | sage-5.9 |
Component: | documentation | Keywords: | |
Cc: | leif, Snark, jdemeyer | Merged in: | sage-5.9.rc1 |
Authors: | Jean-Pierre Flori | Reviewers: | Karl-Dieter Crisman, Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #14031 | Stopgaps: |
Description (last modified by )
Apply source.4.patch and 14465_review.patch.
Attachments (5)
Change History (48)
Changed 9 years ago by
comment:1 Changed 9 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:2 follow-up: ↓ 3 Changed 9 years ago by
- Dependencies set to #14031
- Reviewers set to Karl-Dieter Crisman
comment:3 in reply to: ↑ 2 ; follow-up: ↓ 4 Changed 9 years ago by
Replying to kcrisman:
I like this! Comments:
- Small note; we don't necessarily need Xcode, but just the command line tools, which is FREE but which still does require a (free) Apple Developer account.
Ok, I'll try to add this though I never actually use a Mac.
- Also, we do supply
sage -i openssl
for exactly the reason that we want Sage to be self-contained, so your removal of this command is not helpful, even though I understand where you are going. On Mac, for instance,sage -i openssl
will probably be easier for a lot of people. At least in line 259 ff. I think that it is misleading to have a link to a huge set of instructions and then add these things, which then look very important.
It's still in the installing the notebook with ssl support part. And in the prereq part i mention that Sage ships all the needed stuff (alhtough you need an internet connection, what the "to download separatly" should suggest, any better phrasing welcome, I'm no native english speaker so excuse my french). Of course I could mention that you can run ./sage -i openssl in the prereq part, but the whole point of this section is to suggest to install system wide openssl (which on Linux/Solaris/Cygwin?/FreeBSD should be quite default, at least for ssh users)
- This apparently depends on #14031.
Correct!
- It would be really helpful to include a link or even just mention
MAKE="make -jNUM"
at the initial mention ofmake
.
Ok, I'll add that, makes sense with the usual current hardware.
I would prefer that at least a couple people review this, just to make sure that something didn't go missing during the conversion. But overall a nice improvement and it applies, builds doc, looks good and complete, etc.
comment:4 in reply to: ↑ 3 Changed 9 years ago by
Replying to jpflori:
Replying to kcrisman:
I like this! Comments:
- Small note; we don't necessarily need Xcode, but just the command line tools, which is FREE but which still does require a (free) Apple Developer account.
Could you provide more info about the current situation? The only online ressource I could find is https://developer.apple.com/xcode/. I also registered for an Apple dev account, but apart from Java stuff and old versions of Xcode there is not much to download on the website. I guess that only actual Mac owners can download Xcode from the "Mac App Store". It seems to be free now though. Is there still a separate "Command line tools" download? On the Xcode page it is mentioned as an optional download for Xcode.
comment:5 follow-up: ↓ 21 Changed 9 years ago by
Additional question: as it seems the "command line" part is now an optional dowload to perform from Xcode, is Xcode by itself sufficient? Or should the user download Xcode and then use it to download the "command line tools" part which is the only required part?
I'll post that question on Sage's devel.
comment:6 Changed 9 years ago by
- Cc leif added
comment:7 follow-up: ↓ 12 Changed 9 years ago by
- Cc jpuydt added
There is no such thing as x64
... ;-) (s/x64/x86_64/
)
There is no mention of ARM (afaics from the patch at least).
comment:9 Changed 9 years ago by
A GNU/Linux/ARM is just a GNU/Linux so there isn't anything special to document about it. Especially if it's already documented that you need a good amount of RAM to build.
comment:10 follow-up: ↓ 11 Changed 9 years ago by
Hmmm, what Sage version do I need to apply the patch... %-/
(Unfortunately, also feels like 90% of the patch was reST source reformatting.)
comment:11 in reply to: ↑ 10 Changed 9 years ago by
Replying to leif:
Hmmm, what Sage version do I need to apply the patch... %-/
I'd say 5.9.beta5, doesn't it work?
(Unfortunately, also feels like 90% of the patch was reST source reformatting.)
Yes, lot of reformatting, but including subtle changes here and there... Sorry about that.
comment:12 in reply to: ↑ 7 Changed 9 years ago by
Replying to leif:
There is no such thing as
x64
... ;-) (s/x64/x86_64/
)
Oops.
There is no mention of ARM (afaics from the patch at least).
Is it officially supported? :)
Changed 9 years ago by
comment:14 Changed 9 years ago by
- Cc jdemeyer added
Thank you for taking care of these things - looks great. I have no further problems with this patch, except for some instances of pakage-name
which should probably be package-name
.
As I said before, probably a couple people should be involved on "officially" reviewing this, since very few people will have had experience using all these things in building Sage! Probably Jeroen has the most wide experience in the last few years :)
Changed 9 years ago by
comment:16 follow-up: ↓ 26 Changed 9 years ago by
[Still haven't applied the patch... ;-) -- some random remarks:]
Not sure how much you intended to change on this ticket, but IMHO the overall structure is still suboptimal.
From what I've read, it e.g. sounds like building Sage's GCC was the "standard" way (or recommended), and the main advantage of building Sage from source was something like "full development capability", rather than (potentially much) better performance and compatibility (w.r.t. to available system libraries). Also, it is suggested to build in a directory below $HOME
(instead of some temporary directory on fast[er] media and moving the Sage directory afterwards).
The information a sysadmin needs certainly differs from that of a less experienced user; it doesn't seem the document currently reflects this.
Regarding CC
and CXX
(to some extent CFLAGS
etc. as well), the warnings are rather confusing, and to my best knowledge obsolete. (On the other hand, it is misleadingly emphasized that Sage would build with almost any C compiler [assuming Sage's GCC gets built and used, but that in contrast isn't that explicitly stated].)
It would IMHO be better to move the whole Sage Environment Variables section somewhere else (of course still mentioning the relevant ones here, and referring to their full description).
There's no mention of ccache
... ;-)
comment:17 Changed 9 years ago by
I think ccache is mentioned it the envvar section.
At first I just wanted to mention the Cygwin stuff but found the file in a bad shape in my opinin so decided to clean it up a little bit and it already took more time than what I wanted. So feel free to turn this ticket into a proper overhaul of the file :)
comment:18 follow-up: ↓ 19 Changed 9 years ago by
So feel free to turn this ticket into a proper overhaul of the file :)
NO!!! Feel free to create a minimal version of this patch that just documents the Cygwin changes and then open a new ticket with Glanz und Gloria derer von Leonhardy or whatever ;-) We want Cygwin out there for people to play with, and having this properly documented is part and parcel of it.
Of course most of the documentation could use extra work - it always can, and that's largely the nature of such open-source projects. But let's not keep that from making smaller improvements when we can.
comment:19 in reply to: ↑ 18 ; follow-up: ↓ 20 Changed 9 years ago by
Replying to kcrisman:
But let's not keep that from making smaller improvements when we can.
Exactly!
comment:20 in reply to: ↑ 19 Changed 9 years ago by
Replying to jdemeyer:
Replying to kcrisman:
But let's not keep that from making smaller improvements when we can.
Exactly!
Yes, we can! (Which doesn't exclude non-Cygwin-related smaller improvements; the ticket's title doesn't suggest it's limited to Cygwin stuff...)
In general, it is impossible to have N people, N>1, work on reST files at the same time, so I'm only suggesting things here.
comment:21 in reply to: ↑ 5 ; follow-up: ↓ 22 Changed 9 years ago by
Replying to jpflori:
Additional question: as it seems the "command line" part is now an optional dowload to perform from Xcode, is Xcode by itself sufficient? Or should the user download Xcode and then use it to download the "command line tools" part which is the only required part?
I'll post that question on Sage's devel.
AFAIK, Xcode 4 does not install command line tools by default. However, it appears to be possible to install command line tools only, and use them to build Sage, instead of the full-blown Xcode.
https://developer.apple.com/downloads/index.action for me (I just have a free developer account) mentions Xcode 4.6.2, and two versions of command line tools, dated April 2013, one for OSX 10.7, another for OSX 10.8. I can't test these, as I only have OSX 10.6.
comment:22 in reply to: ↑ 21 Changed 9 years ago by
Replying to dimpase:
Replying to jpflori:
Additional question: as it seems the "command line" part is now an optional dowload to perform from Xcode, is Xcode by itself sufficient? Or should the user download Xcode and then use it to download the "command line tools" part which is the only required part?
I'll post that question on Sage's devel.
AFAIK, Xcode 4 does not install command line tools by default. However, it appears to be possible to install command line tools only, and use them to build Sage, instead of the full-blown Xcode.
https://developer.apple.com/downloads/index.action for me (I just have a free developer account) mentions Xcode 4.6.2, and two versions of command line tools, dated April 2013, one for OSX 10.7, another for OSX 10.8. I can't test these, as I only have OSX 10.6.
That's what i gathered in sage-devel and mentioned in the patch (although both the free accounts I opened seem broken and only show downloads older then Otcober 2012...)
comment:23 Changed 9 years ago by
I'm not really worried about this situation; as long as we point out you don't actually need Xcode on newer systems and that the account is free (if annoying to acquire), we should be okay. The main problem is getting Xcode for older versions of OS X (if you don't have your install disk), which you sometimes need some good detective skills to find on the Apple site - but they are there, lurking...
comment:24 Changed 9 years ago by
Also note that at some point we should surely sync with README.txt, but unless someone feels really motivated in the next few days, I'd say to postpone that to a follow-up ticket.
comment:25 Changed 9 years ago by
And for some magical reason, my Apple developer account is now fixed (though I don't really care as I do not plan to use Mac OS X outside a virtual machine) :)
I'll give the file a last quick cleanup drawing some inspiration from README.txt.
comment:26 in reply to: ↑ 16 Changed 9 years ago by
- Description modified (diff)
Replying to leif:
[Still haven't applied the patch... ;-) -- some random remarks:]
Not sure how much you intended to change on this ticket, but IMHO the overall structure is still suboptimal.
From what I've read, it e.g. sounds like building Sage's GCC was the "standard" way (or recommended), and the main advantage of building Sage from source was something like "full development capability", rather than (potentially much) better performance and compatibility (w.r.t. to available system libraries). Also, it is suggested to build in a directory below
$HOME
(instead of some temporary directory on fast[er] media and moving the Sage directory afterwards).
Ive slightly reworked the top introduction, and the first mention of C++ and Fortran compilers in the deps part.
The information a sysadmin needs certainly differs from that of a less experienced user; it doesn't seem the document currently reflects this.
Regarding
CC
andCXX
(to some extentCFLAGS
etc. as well), the warnings are rather confusing, and to my best knowledge obsolete. (On the other hand, it is misleadingly
I'm not completely sure of the current status, i.e. if all or even most spkg take CC, CPP, CFLAGS ets correctly into account, I know there have been imporvements lately but i don't want to go through the hassle of checking the spkgs now. What I checked because I found it surprising is that CC, CPP and so one get overwritten if Safe has its own ones.
emphasized that Sage would build with almost any C compiler [assuming Sage's GCC gets built and used, but that in contrast isn't that explicitly stated].)
In the "using alternative compilers" seciton, it's stated:
Sage developers tend to use fairly recent versions of GCC, but Sage should compile with any reasonable C compiler. This is because Sage will build GCC first (if needed) and then use that newly built GCC to compile Sage.
Isn't that explicit enough?
I've slightly modified the first time comilers are mentioned in the next patch version as well as this part.
It would IMHO be better to move the whole Sage Environment Variables section somewhere else (of course still mentioning the relevant ones here, and referring to their full description).
Sure, but let's keep the fun for later.
There's no mention of
ccache
... ;-)
Ive rethought about that for maybe wo seconds, but as it needs an internet connection I've chosen to be lazy and not to mention it already in the install procedure, so for now its still hidden in the env var part.
But that will change when we refactor everything :)
ARM is now mentioned as well, I've upped the free disk space to 4GB, that's about the space my 5.9.beta5 install takes. I've not added anything about virtual memory as I have no clue with the new build system for the documentation, feel free to add it in a reviewer patch, or suggest it in the comments.
Changed 9 years ago by
comment:27 follow-up: ↓ 29 Changed 9 years ago by
For the RAM amount needed, there is an old ticket lurking at #2695 :)
comment:28 Changed 9 years ago by
Anything else needed here? Jeroen didn't say anything, though that doesn't mean he doesn't have any comments. I think you have addressed Leif's comments appropriately.
comment:29 in reply to: ↑ 27 Changed 9 years ago by
comment:30 Changed 9 years ago by
- Status changed from needs_review to needs_work
A few small comments:
- Make http://wiki.sagemath.org/SupportedPlatforms more prominent in the "Supported platforms" sections. I would start that section by "See also http://wiki.sagemath.org/SupportedPlatforms for more details".
- remove "or mismatching versions of the GCC compilers" (which is wrong)
- the paragraph starting with "If the build is successful, you will not see the word ERROR in the last 3-4 lines of output" should be improved because Sage now mentions the locations of the log files such that grepping
"^Error"
by hand is no longer needed.
- replace Sage 5.8 by Sage 5.10 at the end.
comment:31 Changed 9 years ago by
What's up with the doctest
sage: GF(7)(x^7) 5 * 401
In any case, I would remove the part advising the user to try the various commands. If you want to advise something, advise to run doctests.
comment:32 Changed 9 years ago by
The advise of using $*
in scripts is very bad: it should be "$@"
.
comment:33 Changed 9 years ago by
Thanks for the comments Jeroen, all of them make sense and stem mostly from the old file. I'm on vacations for 10 days abroad with a (really) slow notebook and sporadic internet access, so feel free to make the changes (but I guess you have other priorities), or anyone please step in.
comment:34 Changed 9 years ago by
Jeroen, if you make those changes, I would be happy to review that. Maybe you can make a reviewer patch.
comment:35 Changed 9 years ago by
- Milestone changed from sage-5.10 to sage-5.9
comment:36 Changed 9 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
comment:37 Changed 9 years ago by
- Reviewers changed from Karl-Dieter Crisman to Karl-Dieter Crisman, Jeroen Demeyer
Anybody wants to review my reviewer patch?
comment:38 follow-up: ↓ 39 Changed 9 years ago by
Just two comments:
- Maybe make at least the second reference to http://wiki.sagemath.org/SupportedPlatforms a hyperlink like
`supported platform list <http://wiki.sagemath.org/SupportedPlatforms>`_
- Do we really want to remove info about where to download Sage for Windows? I'm not necessarily against this, given that this page is about source, but it didn't seem to hurt; perhaps there is a rationale I missed.
I don't mind getting rid of all the examples testing the subsystems, but I also could go with keeping them if someone else felt strongly about it.
comment:39 in reply to: ↑ 38 Changed 9 years ago by
Replying to kcrisman:
- Do we really want to remove info about where to download Sage for Windows? I'm not necessarily against this, given that this page is about source, but it didn't seem to hurt; perhaps there is a rationale I missed.
I found it strange to have that link since this page is indeed about building from source and we don't put links to other binaries. Maybe I'll add a link to the general download (binaries) page instead.
Changed 9 years ago by
comment:40 Changed 9 years ago by
Updated patch.
comment:41 follow-up: ↓ 42 Changed 9 years ago by
Any chance for a quick review such that this can be merged in sage-5.9?
comment:42 in reply to: ↑ 41 Changed 9 years ago by
- Status changed from needs_review to positive_review
Replying to jdemeyer:
Any chance for a quick review such that this can be merged in sage-5.9?
The last patch looks good to me. Positive review.
comment:43 Changed 9 years ago by
- Merged in set to sage-5.9.rc1
- Resolution set to fixed
- Status changed from positive_review to closed
I like this! Comments:
sage -i openssl
for exactly the reason that we want Sage to be self-contained, so your removal of this command is not helpful, even though I understand where you are going. On Mac, for instance,sage -i openssl
will probably be easier for a lot of people. At least in line 259 ff. I think that it is misleading to have a link to a huge set of instructions and then add these things, which then look very important.MAKE="make -jNUM"
at the initial mention ofmake
.I would prefer that at least a couple people review this, just to make sure that something didn't go missing during the conversion. But overall a nice improvement and it applies, builds doc, looks good and complete, etc.