Skip to content

Add documentation on build systems#5015

Merged
scheibelp merged 28 commits intospack:developfrom
adamjstewart:docs/build-systems
Jul 17, 2018
Merged

Add documentation on build systems#5015
scheibelp merged 28 commits intospack:developfrom
adamjstewart:docs/build-systems

Conversation

@adamjstewart
Copy link
Copy Markdown
Member

@adamjstewart adamjstewart commented Aug 7, 2017

Adds documentation for the following build systems:

Make-based

  • MakefilePackage

Make-incompatible

  • SConsPackage
  • WafPackage

Build-script generation

  • AutotoolsPackage
  • CMakePackage
  • QMakePackage

Language-specific

Other

  • AspellDictPackage - not really a build system?
  • CudaPackage - not really a build system?
  • Custom build systems
  • IntelPackage - waiting on Intel prefixes #7469

We 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

@alalazo
Copy link
Copy Markdown
Member

alalazo commented Sep 16, 2017

💯 I had a first look at I think this is great! Let me know when this is ready to be reviewed.

@adamjstewart
Copy link
Copy Markdown
Member Author

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.

@tgamblin tgamblin added this to the v0.12.0 milestone Nov 12, 2017
@adamjstewart
Copy link
Copy Markdown
Member Author

@davydden Now that #6746 has been merged, would you mind adding a page on OctavePackage to this PR while it's still fresh in memory?

@davydden
Copy link
Copy Markdown
Member

davydden commented Jan 3, 2018

@adamjstewart

Now that #6746 has been merged, would you mind adding a page on OctavePackage to this PR while it's still fresh in memory?

sure, see adamjstewart#4

@adamjstewart
Copy link
Copy Markdown
Member Author

@hartzell If you get a chance, would you mind adding a page for AspellDictPackage? I think I'll try to finish this PR up before the semester starts.

@alalazo alalazo self-assigned this Jan 20, 2018
@tgamblin tgamblin modified the milestones: v0.12.0, v0.13.0 Feb 21, 2018
@adamjstewart adamjstewart mentioned this pull request Mar 15, 2018
@adamjstewart
Copy link
Copy Markdown
Member Author

Ping ping

@scheibelp scheibelp self-assigned this Jun 22, 2018
@scheibelp
Copy link
Copy Markdown
Member

I'll look into reviewing and merging this starting on 6/25. I anticipate it will take a day or two.

@adamjstewart
Copy link
Copy Markdown
Member Author

Take your time, I'm happy to tweak any sections, just let me know.

Copy link
Copy Markdown
Member

@scheibelp scheibelp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!


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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reference the packaging guide here?

=============

This guide provides information specific to a particular
`build system <https://en.wikipedia.org/wiki/List_of_build_automation_software>`_.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.*
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>`_
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

@tgamblin tgamblin Jul 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 run autoreconf to 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).

@scheibelp
Copy link
Copy Markdown
Member

In #5015 (comment) I forgot to mention that I created #8571 so you could reference that in a TODO if you wish.

@adamjstewart
Copy link
Copy Markdown
Member Author

Thanks @scheibelp, I'll try to address these comments by the end of the week.

@adamjstewart
Copy link
Copy Markdown
Member Author

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.

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.

@adamjstewart
Copy link
Copy Markdown
Member Author

Addressed most of @scheibelp's comments. Still thinking about some of the comments on AutotoolsPackage and CMakePackage.

@svenevs
Copy link
Copy Markdown
Member

svenevs commented Jun 27, 2018

I don't build them, I just assume I know what they will look like

Shame on you 😛 After the initial build, edits should be fast if you're only changing a couple files.

But if there's anyone who knows a good way to preview .rst files, it would be @svenevs.

I defer to the (excellent) advice of @alalazo here: #5861 (comment)

I think you can also SPHINXOPTS="" make html if you don't want to edit the Makefile but don't recall, and I'm currently revolting against python because it gives me ModuleNotFoundError: No module named '_struct'.

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

  1. Create a secret gist on gist.github.com, create a file called say README.rst. As long as it has .rst, GitHub does a reasonable job rendering reStructuredText.
  2. Paste the contents into a file and run rst2html.py your_file.rst > index.html.

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 docutils (the rst2html.py script origin), and GitHub. So the only way to know for sure is to build them yourself.

Don't forget you can also make html -j 10 for parallel (but that usually doesn't help much...). I hope one of those two is marginally useful!

@adamjstewart
Copy link
Copy Markdown
Member Author

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.

Don't forget you can also make html -j 10 for parallel

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 sphinxcontrib.programoutput doesn't declare whether or not it is parallel-safe. I tried making it parallel-safe one time, but literally everything broke. I don't think multiple files can cross-reference each other if you want things to build in parallel.

@scheibelp
Copy link
Copy Markdown
Member

Still thinking about some of the comments on AutotoolsPackage and CMakePackage.

Is this still true? I wanted to make sure I wasn't letting this slide

@adamjstewart
Copy link
Copy Markdown
Member Author

@scheibelp Thanks for checking in!

Is this still true? I wanted to make sure I wasn't letting this slide

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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variables to watch out for
^^^^^^^^^^^^^^^^^^^^^^^^^^

The following is a list of common variables to watch out for:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scheibelp
Copy link
Copy Markdown
Member

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.

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.

@scheibelp
Copy link
Copy Markdown
Member

I'll have a chance to format a reasonable commit message and merge this tomorrow.

@scheibelp scheibelp merged commit 8ce62ba into spack:develop Jul 17, 2018
@scheibelp
Copy link
Copy Markdown
Member

Thanks!

@adamjstewart adamjstewart deleted the docs/build-systems branch July 17, 2018 19:44
@tgamblin
Copy link
Copy Markdown
Member

Awesome to see this merged! Thanks @adamjstewart, all for the excellent docs!

@svenevs
Copy link
Copy Markdown
Member

svenevs commented Jul 18, 2018

This is why I refer to him as SpackMan. He may not be the hero we deserve... But definitely the one we need ❤️

@citibeth
Copy link
Copy Markdown
Member

citibeth commented Jul 18, 2018 via email

mgsternberg added a commit to mgsternberg/spack that referenced this pull request Aug 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation intel perl python R

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants