Add documentation on build systems#5015
Conversation
f1c9ce6 to
246e96b
Compare
|
💯 I had a first look at I think this is great! Let me know when this is ready to be reviewed. |
|
Unfortunately, grad school has kept me much too busy to work on this. I would like to see this get merged before SC17 though. If anyone has the time to contribute to this PR, feel free to add information on the build systems I didn't get to. The structure of what I had intended should be pretty obvious from the other build systems. |
sure, see adamjstewart#4 |
|
@hartzell If you get a chance, would you mind adding a page for |
5842aa5 to
6a50686
Compare
|
Ping ping |
|
I'll look into reviewing and merging this starting on 6/25. I anticipate it will take a day or two. |
|
Take your time, I'm happy to tweak any sections, just let me know. |
scheibelp
left a comment
There was a problem hiding this comment.
Let me know if you don't have time to address these and I can look into fixing these issues myself.
Most importantly: the autotools section could be simplified to omit all the detail about the workings of the autotools system to generate a configure file. If you disagree then the citation needs to be added (I have a couple comments on that).
The other comments suggest rewording for clarity etc. and while those would be good for this PR (and I've generally tried to suggest small changes) I wouldn't consider them essential.
I have build the docs and looked through them and the formatting appears generally good. As a side note, do you know how to view the end product of the documentation (i.e. without building the docs yourself)? If I view the files in the diff it doesn't render everything properly.
Let me know if any of the suggestions seem unclear or off track - thanks!
lib/spack/docs/build_systems.rst
Outdated
|
|
||
| This guide provides information specific to a particular | ||
| `build system <https://en.wikipedia.org/wiki/List_of_build_automation_software>`_. | ||
| It assumes knowledge of general Spack packaging capabilities and expands |
There was a problem hiding this comment.
Reference the packaging guide here?
lib/spack/docs/build_systems.rst
Outdated
| ============= | ||
|
|
||
| This guide provides information specific to a particular | ||
| `build system <https://en.wikipedia.org/wiki/List_of_build_automation_software>`_. |
There was a problem hiding this comment.
This article starts out with:
Build automation involves scripting or automating the process of compiling computer source code into binary code.
So IMO it may be useful to start with our own 1-sentence description (but including the link is still useful):
Spack defines a number of classes which understand how to use common build systems (makefiles, cmake, etc.). Spack package definitions can inherit these classes in order to streamline their builds.
| ``Makefile`` for you. The following diagram provides a high-level overview | ||
| of the process: | ||
|
|
||
| .. image:: Autoconf-automake-process.* |
There was a problem hiding this comment.
The citation does not show up when I build the docs. I think you may need to use a figure (see: http://docutils.sourceforge.net/0.7/docs/ref/rst/directives.html):
.. image:: Autoconf-automake-process.*
:target: https://commons.wikimedia.org/w/index.php?curid=15581407
This is the caption, which should be a citation
I have a separate comment on the citation format.
| :target: https://commons.wikimedia.org/w/index.php?curid=15581407 | ||
|
|
||
| .. | ||
| By Jdthood - Own work, based on https://commons.wikimedia.org/wiki/File:Autoconf.svg, |
There was a problem hiding this comment.
Based on looking at https://commons.wikimedia.org/wiki/File:Autoconf-automake-process.svg, this appears to be Jdthood's citation. Our citation of Jdthood's work would be different I think:
`GNU autoconf and automake process for generating makefiles <https://commons.wikimedia.org/wiki/File:Autoconf-automake-process.svg>`_ by `Jdthood` under `CC BY-SA 3.0 <https://creativecommons.org/licenses/by-sa/3.0/deed.en>`_
(that is my attempt at .rst-formatting to produce a citation which follows https://wiki.creativecommons.org/wiki/Best_practices_for_attribution and accounting for the fact that Jdthood does not appear to have a user account)
|
|
||
| Like Autotools, CMake is a build-script generator. Designed by Kitware, | ||
| CMake is a popular up-and-coming build system. It is | ||
| `becoming the dominant build system <https://trends.google.com/trends/explore?date=all&q=autoconf,automake,cmake,scons>`_ |
There was a problem hiding this comment.
This is interesting but IMO controversial as a form of evidence (to be clear: I'm not contesting the claim, just the evidence for the claim). For example, "scons" is the most popular term in Argentina (among the 4, according to the "trends" link), but I think that may translate from "biscuit" (I think there's a similar phenomenon in South Africa). I think we can just state that CMake is a popular build system and leave it at that.
There was a problem hiding this comment.
I can't remember where I found this (maybe @citibeth pointed me to it), but I found a talk on build systems that used these trends to argue this point. Of course, that doesn't mean it's a valid point.
There was a problem hiding this comment.
It's considered the "standard" by the C++ tooling committee, at least, and new IDEs like CLion are making the choice to only support CMake. There are still lots of autotools builds out there but I see them less and less.
"Dominant build system" is probably too general -- there are other very big ecosystems and "build system" is vaguely defined. I would say it's probably the most popular build system for new C, C++, and Fortran projects.
| non-Python dependencies. Anaconda contains many Python packages that | ||
| are not yet in Spack, and Spack contains many Python packages that are | ||
| not yet in Anaconda. The main advantage of Spack over Anaconda is its | ||
| ability to choose a specific compiler and BLAS/LAPACK or MPI library. |
There was a problem hiding this comment.
I definitely agree that the Windows support on Conda should be mentioned as an advantage over Spack.
| > update.packages(ask = FALSE) | ||
|
|
||
|
|
||
| This works great for users who has internet access, but those on an |
There was a problem hiding this comment.
Typo, should read:
this works great for users who have internet access
|
|
||
| In the case of cantera, using ``env_vars=all`` allows us to use | ||
| Spack's compiler wrappers. If you don't see an option related to | ||
| environment variables, try using Spack's compiler wrappers. The full |
There was a problem hiding this comment.
I think a bit more detail would be useful:
... try using Spack's compiler wrappers by passing spack_cc, spack_cxx, and spack_fc via the CC, CXX, and FORTRAN arguments, respectively.
| Obviously, these commands can only be run if you have root privileges. | ||
| Furthermore, ``cpanm`` is not capable of installing non-Perl dependencies. | ||
| If you need to install to your home directory or need to install a module | ||
| with non-Perl dependencies, Spack is a better option. |
There was a problem hiding this comment.
That sounds useful to mention but not essential for this PR. Views may allow a user to symlink a package into a new prefix, then install addons without modifying the Spack installation prefix, (e.g. this is possible for python with virtualenv once #7152 is merged).
| necessary for installation. If this is the case, your package does not | ||
| require any Autotools dependencies. | ||
|
|
||
| However, a basic rule of version control systems is to never commit |
There was a problem hiding this comment.
I think this is a bit more detail than is required for a Spack user. IMO you could refocus the conversation:
Some packages use higher-level specification files (e.g.
``configure.ac``and``Makefile.am``) to generate a configure script. In this case, Spack will automatically runautoreconfto generate the configure script, but you need to add some dependencies...
That also sidesteps issues related to properly citing the .svg file (although that by itself is not a good reason to omit it).
|
In #5015 (comment) I forgot to mention that I created #8571 so you could reference that in a TODO if you wish. |
|
Thanks @scheibelp, I'll try to address these comments by the end of the week. |
I don't build them, I just assume I know what they will look like. But if there's anyone who knows a good way to preview .rst files, it would be @svenevs. |
|
Addressed most of @scheibelp's comments. Still thinking about some of the comments on AutotoolsPackage and CMakePackage. |
Shame on you 😛 After the initial build, edits should be fast if you're only changing a couple files.
I defer to the (excellent) advice of @alalazo here: #5861 (comment) I think you can also However, fast options for viewing a reasonable rendering would be to first copy the contents of the entire file you want to check (for GitHub PRs you can click "View File"). Then
In both cases, you won't get the full effect (e.g. references to other parts of the docs). It's best to render it with sphinx. Additionally, the Sphinx parser and writer is different than both Don't forget you can also |
|
Just remembered that you can click "View" for a specific file to see the .rst rendered properly. Of course, it won't process TOCs or refs, but it's a good start.
I'm pretty sure that doesn't help. Once upon a time, the Makefile explicitly contained logic to run Sphinx in parallel, but it secretly wasn't being used because |
Is this still true? I wanted to make sure I wasn't letting this slide |
|
@scheibelp Thanks for checking in!
It's not not true. I'm still on the fence on your remaining comments. If anyone else agrees, I can make the change, otherwise I think I'll leave it as is. |
| Make has multiple types of assignment operators. Some Makefiles | ||
| use ``=`` to assign variables. The only way to override these | ||
| variables is to edit the Makefile or override them on the command-line. | ||
| However, Makefiles that use ``?=`` for assignment honor environment |
There was a problem hiding this comment.
| Variables to watch out for | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| The following is a list of common variables to watch out for: |
There was a problem hiding this comment.
Is it worth adding a link to https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html in here somewhere?
Given that someone is unlikely to drop in and review my review of this, I think we can proceed. I've rebuilt the docs and the citation for the Autotools figure looks good now, so all of my primary concerns are resolved. |
|
I'll have a chance to format a reasonable commit message and merge this tomorrow. |
|
Thanks! |
|
Awesome to see this merged! Thanks @adamjstewart, all for the excellent docs! |
|
This is why I refer to him as SpackMan. He may not be the hero we deserve... But definitely the one we need ❤️ |
|
Thank you Adam
…On Tue, Jul 17, 2018 at 8:54 PM Stephen McDowell ***@***.***> wrote:
This is why I refer to him as SpackMan. He may not be the hero we
deserve... But definitely the one we need ❤️
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5015 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AB1cd93OLvgcaJz0ubReehe-6LXPRqmzks5uHodHgaJpZM4OwBuN>
.
|
Adds documentation for the following build systems:
Make-based
MakefilePackageMake-incompatible
SConsPackageWafPackageBuild-script generation
AutotoolsPackageCMakePackageQMakePackageLanguage-specific
OctavePackagePerlPackagePythonPackageRPackageRubyPackage- waiting on Adding a RubyPackage to enable ruby extensions. #3127Other
AspellDictPackage- not really a build system?CudaPackage- not really a build system?IntelPackage- waiting on Intel prefixes #7469We never really got around to documenting how to use each build system when we introduced separate base classes. This PR is an attempt to solve this. It is also an attempt to pass on knowledge of each build system to new package developers who may not be familiar with SCons or QMake or Waf. If there's anything you think I'm missing, let me know!
Closes #3228
Closes #2951
Closes #2222