build: do not build doc in source tarball

[joyeecheung]

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

build

Fixes: #16650

This is the real fix for #16650 , which makes sure that GNU make v4.x won't build the docs if the source is extracted from the source tarball (i.e. doc/api contains built docs) (see the theory in #16650 (comment))

Also during the investigation of this issue I think I have a better idea about #17043, I'll open a separate PR with a more robust available-node and try to fix all the $(NODE) usage there, hopefully fixing the makefile regression. For this PR the current implementation is enough.

cc @nodejs/build

[MylesBorins]

@joyeecheung what are the steps to verify this works?

[refack]

@joyeecheung any chance this whole thing could be refactored out of the main Makefile?
As I see it (1) we lint the sources (2) we have a test that checks the output independently of the build process (3) it's not needed for the tarball (4) AFAIK it's output is used only once and only during release.

@MylesBorins, one possible option is to use a new test file written by @joyeecheung to validate the output of the doctool - make doc && node test/doctool/test-make-doc.js.

[joyeecheung]

@MylesBorins My way is:

# Or get any other nightly builds
wget https://nodejs.org/dist/v9.2.0/node-v9.2.0.tar.gz
tar zxvf node-v9.2.0.tar.gz
cd node-v9.2.0
# Move this makefile to the source tarball
cp ../node/Makefile .

# Or if you have a global node, it can use that one as well
./configure && make -j8

# observe that this should not install or try to build anything
make doc-only
# or you can use make --trace doc-only or make -n doc-only to see a trace

Also if you are on Mac, make sure you are testing it with GNU Make v4.x (the default one is 3.x, which does not really have a problem with the previous configuration)

[joyeecheung]

@refack Sorry, I am not sure I am following, by "the whole thing" do you mean "make doc-only"? But I am pretty sure there are people actually reading its output e.g. people working on doc tools, writing docs, or just wanting to read the latest docs..

[refack]

I'm not saying remove it, I'm suggesting moving it to it's own Makefile (possibly tools/doc/Makefile) or to a script in tools/doc/package.json. Then the doc target could be something like cd tools/doc && make or cd tools/doc && npm run build

AFAICT make's change tracking mechanism doesn't give us a huge benefit, while having these targets in the main Makefile adds to it's complexity...

[joyeecheung]

@refack I think we can refactor it out to another Makefile (still nice to avoid rebuilding unmodified docs), but probably in another PR.

[joyeecheung]

BTW both #16661 and its previous implementation could result in concurrent npm install, which apparently does not play well with master and npm with locks..also npm is giving a funky warning about not supporting node-v10.0.0-pre whenever installing something with the locally built node..

[joyeecheung]

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

[joyeecheung]

Landed in 289fcb0, thanks!

[MylesBorins]

opting to not land this on v6.x. Please feel free to change the labels and open a backport