Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc/manual/stdenv: improve cmake documentation #103506

Closed
wants to merge 2 commits into from

Conversation

jonringer
Copy link
Contributor

@jonringer jonringer commented Nov 11, 2020

Motivation for this change

closes: #102104

Changes:

  • list attrs which affects cmake builds
  • add anchor E.g. manual#cmake (although there's probably a better way)
  • add some examples

modeled it after the meson section

since stable manual is the default one on the website, this should also be backported.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

doc/stdenv/stdenv.xml Show resolved Hide resolved
<varlistentry>
<term>
cmake
<anchor xml:id="cmake"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

You can add xml:id attribute directly to term element.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea, I thought about that after I created the PR

Copy link
Contributor Author

@jonringer jonringer Nov 12, 2020

Choose a reason for hiding this comment

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

do you know of a way that i can get the "link" element without using a section?

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean by using a section?

You can make any element linkable by adding xlink:href="#targetid attribute, or if you want to linkify arbitrary text, wrap it with <link> element and add the xlink:href argument to that. Or if you want to automatically generate the link title, use <xref linkend="targetid" /> – that should work for chapter, sections, tables, terms…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I meant like https://nixos.org/manual/nixpkgs/stable/#chap-overlays where if you hover over the text, it will give you a "link" to that location

Copy link
Contributor Author

Choose a reason for hiding this comment

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

xlink:href might work though, as that will at least make the text a link. better than the anchor to nowhere at least

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, right, we do not have that. Maybe I could try to write a XSL transformation later but not sure if it is worth the effort with impending markdownification. For now, I just use developer tools to get the anchor links. And there are probably browser extensions that can make it easier too,

doc/stdenv/stdenv.xml Show resolved Hide resolved
</term>
<listitem>
<para>
Overrides the default configure phase to run the CMake command. By default, we use the Make generator of CMake. In addition, dependencies are added automatically to CMAKE_PREFIX_PATH so that packages are correctly detected by CMake. Some additional flags are passed in to give similar behavior to configure-based packages. You can disable this hook’s behavior by setting configurePhase to a custom value, or by setting dontUseCmakeConfigure. cmakeFlags controls flags passed only to CMake. By default, parallel building is enabled as CMake supports parallel building almost everywhere. When Ninja is also in use, CMake will detect that and use the ninja generator.
Overrides the default configure phase to run the CMake command. By default, we use the Make generator of CMake, however, if ninja is present, then the ninja generator will be used instead. In addition, dependencies are added automatically to CMAKE_PREFIX_PATH so that packages are correctly detected by CMake. By default, the cmake configurePhase can be thought of as:
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to add envar and literal tags where appropriate.

<title>Variables controlling CMake</title>
<varlistentry>
<term>
<varname>cmakeFlags</varname>
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be also nice to add anchors to the individual variables – I did that for most of stdenv in 3f2b7be but somehow missed Meson, which was annoying when I wanted to link it recently.

doc/stdenv/stdenv.xml Show resolved Hide resolved
doc/stdenv/stdenv.xml Show resolved Hide resolved
doc/stdenv/stdenv.xml Show resolved Hide resolved
@jonringer
Copy link
Contributor Author

@jtojnar just a reminder, I'm just trying to improve the documentation, not necessarily making it perfect :)

</term>
<listitem>
<para>
Overrides the default configure phase to run the CMake command. By default, we use the Make generator of CMake. In addition, dependencies are added automatically to CMAKE_PREFIX_PATH so that packages are correctly detected by CMake. Some additional flags are passed in to give similar behavior to configure-based packages. You can disable this hook’s behavior by setting configurePhase to a custom value, or by setting dontUseCmakeConfigure. cmakeFlags controls flags passed only to CMake. By default, parallel building is enabled as CMake supports parallel building almost everywhere. When Ninja is also in use, CMake will detect that and use the ninja generator.
Overrides the default configure phase to run the CMake command. By default, we use the Make generator of CMake, however, if ninja is present, then the ninja generator will be used instead. In addition, dependencies are added automatically to CMAKE_PREFIX_PATH so that packages are correctly detected by CMake. By default, the cmake configurePhase can be thought of as:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's still worthwhile mentioning somewhere in the section that enableParallelBuilding = true by default for CMake, since there are still regularly PRs with enableParallelBuilding = true when the derivation uses CMake.

@jonringer
Copy link
Contributor Author

jonringer commented Nov 20, 2020

if someone with more docbook knowledge than me would like to take up this PR then please do so. but to be honest, I don't want to put in the time to learn docbook (nixpkgs is the only ecosystem I contribute to which uses it, and it's significantly different).

I really want to improve documentation, but I really don't want to touch nixpkgs' documentation (docbook).

I'll just wait for the commonmark conversion to start happening, and contribute then.

@jonringer jonringer closed this Nov 20, 2020
@jtojnar
Copy link
Contributor

jtojnar commented Nov 20, 2020

If someone wants to adopt this, we also have a visual DocBook editor packaged: #95593

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/gnu-guix-1-2-0-released-2020-blog-gnu-guix/10167/5

@erictapen erictapen removed the 9.needs: port to stable A PR needs a backport to the stable release. label Jan 13, 2021
@jtojnar
Copy link
Contributor

jtojnar commented May 11, 2021

@jonringer stdenv is now in markdown so you might want to revisit this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nixpkgs manual: Add section about using CMake
5 participants