Opened 7 years ago

Closed 7 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:

Attachments (5)

source.patch (98.5 KB) - added by jpflori 7 years ago.
source.2.patch (99.3 KB) - added by jpflori 7 years ago.
source.3.patch (99.3 KB) - added by jpflori 7 years ago.
source.4.patch (100.5 KB) - added by jpflori 7 years ago.
14465_review.patch (10.7 KB) - added by jdemeyer 7 years ago.

Download all attachments as: .zip

Change History (48)

Changed 7 years ago by jpflori

comment:1 Changed 7 years ago by jpflori

  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 follow-up: Changed 7 years ago by kcrisman

  • Dependencies set to #14031
  • Reviewers set to Karl-Dieter Crisman

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.
  • 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.
  • This apparently depends on #14031.
  • It would be really helpful to include a link or even just mention MAKE="make -jNUM" at the initial mention of make.

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:3 in reply to: ↑ 2 ; follow-up: Changed 7 years ago by 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.

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 of make.

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 7 years ago by jpflori

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: Changed 7 years ago by 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.

comment:6 Changed 7 years ago by leif

  • Cc leif added

comment:7 follow-up: Changed 7 years ago by leif

  • 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:8 Changed 7 years ago by leif

  • Cc Snark added; jpuydt removed

Oooops.

comment:9 Changed 7 years ago by Snark

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: Changed 7 years ago by leif

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 7 years ago by jpflori

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 7 years ago by jpflori

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 7 years ago by jpflori

comment:13 Changed 7 years ago by jpflori

  • Description modified (diff)

Updated patch.

comment:14 Changed 7 years ago by kcrisman

  • 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 7 years ago by jpflori

comment:15 Changed 7 years ago by jpflori

  • Description modified (diff)

Fixed.

comment:16 follow-up: Changed 7 years ago by 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).

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 7 years ago by jpflori

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: Changed 7 years ago by kcrisman

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: Changed 7 years ago by jdemeyer

Replying to kcrisman:

But let's not keep that from making smaller improvements when we can.

Exactly!

comment:20 in reply to: ↑ 19 Changed 7 years ago by leif

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: Changed 7 years ago by 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.

comment:22 in reply to: ↑ 21 Changed 7 years ago by jpflori

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 7 years ago by kcrisman

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...

(So what I mean is that JP's current wording is fine to me.)

Last edited 7 years ago by kcrisman (previous) (diff)

comment:24 Changed 7 years ago by jpflori

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 7 years ago by jpflori

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 7 years ago by jpflori

  • 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 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

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 7 years ago by jpflori

comment:27 follow-up: Changed 7 years ago by jpflori

For the RAM amount needed, there is an old ticket lurking at #2695 :)

comment:28 Changed 7 years ago by kcrisman

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 7 years ago by leif

Replying to jpflori:

For the RAM amount needed, there is an old ticket lurking at #2695 :)

I've commented on that ticket.

comment:30 Changed 7 years ago by jdemeyer

  • Status changed from needs_review to needs_work

A few small comments:

  • 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 7 years ago by jdemeyer

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 7 years ago by jdemeyer

The advise of using $* in scripts is very bad: it should be "$@".

comment:33 Changed 7 years ago by jpflori

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 7 years ago by kcrisman

Jeroen, if you make those changes, I would be happy to review that. Maybe you can make a reviewer patch.

comment:35 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.10 to sage-5.9

comment:36 Changed 7 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:37 Changed 7 years ago by jdemeyer

  • Reviewers changed from Karl-Dieter Crisman to Karl-Dieter Crisman, Jeroen Demeyer

Anybody wants to review my reviewer patch?

comment:38 follow-up: Changed 7 years ago by kcrisman

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 7 years ago by jdemeyer

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 7 years ago by jdemeyer

comment:40 Changed 7 years ago by jdemeyer

Updated patch.

comment:41 follow-up: Changed 7 years ago by jdemeyer

Any chance for a quick review such that this can be merged in sage-5.9?

comment:42 in reply to: ↑ 41 Changed 7 years ago by dimpase

  • 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 7 years ago by jdemeyer

  • Merged in set to sage-5.9.rc1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.