Add task for building docs only, using existing Node

[jmm]

This is a WIP attempt to make it easy to build just the docs using an existing Node, instead of having to build Node first, which can be prohibitively slow. Related #3749. The primary reason I call it a WIP is because Make is not my forte :/

/cc @nodejs/documentation @chrisdickinson (per suggestion).

I saw that in Makefile there's a conditionally set NODE var that's used for building the docs (among other things), so I based this on utilizing that: NODE=node make doc-only.

Summary of changes:

• When it's not explicitly set, set NODE using NODE_EXE to reduce duplication.
• Setup tools/doc/generate.js to read a --node-version argument, which will be used to output the version to the docs instead of process.version. Setup the relevant Make recipe to pass it.
• Refactor tools/doc/html.js's toHTML and render() to accept options hashes (the signatures would be getting pretty unwieldy if I'd added node_version, which I'm propagating to those functions from generate.js).
• Make targets
  • I added a doc-only phony target.
  • I made $(NODE_EXE) and doc-only prerequisites of doc.
  • I removed $(NODE_EXE) as a prerequisite of the out/doc/api/%.json and out/doc/api/%.html targets. I'm hoping that it being a prerequisite of doc takes care of it for any case where it's needed by these targets (the only one I'm sure of is make doc).

The changes to the Make targets are the parts I'm least confident about. I'd appreciate if someone who understands Make better can check that it's a sensible setup, and especially that it won't break stuff.

[jasnell]

@nodejs/documentation

[rvagg]

I think this should probably be FULLVERSION to take into account nightlies and other other versions with prerelease tags

[jmm]

@rvagg Ah, thank you! I pushed a new commit that corrects that. I realized it was a mistake anyway since I overlooked that process.version is prefixed with v and RAWVER isn't.

[rvagg]

nice, I like this. /cc @nodejs/build

[chrisdickinson]

This LGTM once @rvagg's suggestion has been addressed. Great work!

[jasnell]

+1 on this... I haven't gone through the commit myself so I won't sign off but this is quite valuable I think. Thank you!

[tflanagan]

Hmmm,

~/$ node -v
v5.0.0
~/$ git clone https://github.com/jmm/node.git
~/$ cd node
~/node$ git checkout build-just-docs
~/node$ ./configure
~/node$ make doc-only
./node tools/doc/generate.js --node-version=6.0.0 --format=html --template=doc/template.html doc/api/addons.markdown > out/doc/api/addons.html
/bin/sh: 1: ./node: not found
make: *** [out/doc/api/addons.html] Error 127

Looks like this is the culprit:

NODE ?= ./$(NODE_EXE)

If you are trying, as stated, to build just the docs using an existing Node, the ./ shouldn't be there?

[jmm]

@tflanagan Thanks for taking a look. The issue that I was having is that the make doc task and its prerequisites depend on the task that builds the Node executable, which takes a long time and isn't necessary for building the docs if you already have a working Node as I do (and I suspect most people do where they'd be working on the docs). The line you highlighted there provides a default value if NODE isn't set, defaulting to the path of the built executable. So because that line conditionally sets NODE that actually provided a ready solution for part of the problem: pointing to the existing Node. I utilized that to make this work via NODE=node make doc-only. The other part was making it so there's a task for building the docs that doesn't build the executable as a prerequisite.

[tflanagan]

@jmm, ah! NODE=node, missed that!

With that, its working like a charm. Well done, I like it!

Just one more issue:

~/node$ NODE=node make doc-only
...
~/node$ stat out/doc/api/assert.html
File: `out/doc/api/assert.html'
Size: 0 Blocks: 0 IO Block: 4096 regular empty file
Device: ca01h/51713d Inode: 2074372 Links: 1
Access: (0644/-rw-r--r--) Uid: ( 1001/***) Gid: ( 1001/ ***)
Access: 2015-11-18 18:58:10.000000000 +0000
Modify: 2015-11-18 18:58:10.000000000 +0000
Change: 2015-11-18 18:58:10.000000000 +0000
Birth: -

Getting blank HTML files.

Edit:
The JSON files look good though.

[tflanagan]

@jmm Hmm, started from scratch and the HTML files have content now. Not sure what happened, going to chalk it up to a dirty environment.

[jmm]

Thanks @jasnell@rvagg@chrisdickinson! I pushed new commits that fix the issue @rvagg pointed out and I took a crack at updating the README. Not sure if you need me to do anything with combining any / all commits or prefixing the messages with anything particular (since these changes affect the build system / README, not an actual subsystem)

[jmm]

@tflanagan Thanks! Not sure what would've resulted in the blank HTML files, I haven't run into that. Hopefully it is just a fluke like you said.

[rvagg]

I'd rather this line not be modified, it doesn't add enough value to justify the noise. If someone comes looking for how to build docs they will see the section below.

[rvagg]

@jmm squash commits and prefix the message with build: and it should be fine.

[jmm]

@rvagg Ok, thanks for the feedback, I removed that line and squashed, with build: prefix.

[rvagg]

lgtm, thanks for the work on this @jmm

[Fishrock123]

I think part of the slowness of building node is due to not recommending make -j8 or make -j4, would that help more?

[orangemocha]

Good point @Fishrock123 . make -j $(getconf _NPROCESSORS_ONLN) is how we build Node in CI so it is supported and probably worth adding to the README.

[Fishrock123]

getconf _NPROCESSORS_ONLN

Huh, didn't know about that.

[jmm]

Thanks @rvagg.

@Fishrock123 If there's an easy recommendation that speeds up the build in general that sounds great, but for practical purposes it doesn't help [me at least] with this. With make -j $(getconf _NPROCESSORS_ONLN) (2) or make -j4 it takes the Intel Pentium T4500 laptop dev server I have this running on ~20 minutes to build. (With -j8 the system is pretty choked and the total time is much slower, at least in one trial run.) With this PR I can do NODE=node make doc-only in ~20 seconds.

[MylesBorins]

what is holding this up?

@jmm can you do a rebase against master and force push. I'll see about getting this landed 😄

[jmm]

@thealphanerd Thanks, done 🎉. Slight change: moved this part after the code:

(Where node is the path to your executable.)

BTW tools/doc/json.js is currently issuing a warning and generating incorrect output for dgram socket.send() because it's not designed to handle this kind of signature with multiple params that are optional as a group:

(msg,[offset,length,]port,address[,callback])

I can probably fix that if this gets merged and anyone cares about that.

[MylesBorins]

ci: https://ci.nodejs.org/job/node-test-pull-request/1919/

I'm going to test this more manually later today