build: allow proper generation of html docs

[maclover7]

gen-doc always calls gen-json, which means it's impossible to generate
html docs. Changed this to pass in the command the user wants to run.
I'm a Makefile novice, so please let me know if there's a better way to
do this :)

Fixes: #14930

Checklist

• tests and/or benchmarks are included
• commit message follows commit guidelines

Affected core subsystem(s)

build

[Trott]

Hi, @maclover7! We use build as the subsystem for Makefile rather than doc. If you could update the commit message and the PR title, that would be great!

[Trott]

gen-doc always calls gen-json, which means it's impossible to generate
html docs.

This perhaps explains #14930. /cc @vsemozhetbyt

Assuming these bug reports are correct, I wonder if it was introduced in #13482 which landed 3 days ago.

[refack]

I think if you simply remove $(gen-json)/$(1) you won't need the change in the other too lines
(this will simply resolve to the path of the node exe)

[maclover7]

Bearing in mind I'm a Makefile novice, when I have this diff:

diff --git a/Makefile b/Makefile
index 91466e1003..384164e24d 100644
--- a/Makefile
+++ b/Makefile
@@ -503,14 +503,14 @@ gen-doc = \
                else \
                        cd tools/doc && node ../../$(NPM) install; \
                fi;\
-       [ -x $(NODE) ] && $(NODE) $(1) || node
+       [ -x $(NODE) ] && $(NODE) || node
 
 out/doc/api/%.json: doc/api/%.md
-       $(call gen-doc, $(gen-json))
+       $(gen-doc) $(gen-json)
 
 # check if ./node is actually set, else use user pre-installed binary
 out/doc/api/%.html: doc/api/%.md
-       $(call gen-doc, $(gen-html))
+       $(gen-doc) $(gen-html)
 
 docopen: $(apidocs_html)
        @$(PYTHON) -mwebbrowser file://$(PWD)/out/doc/api/all.html

It causes this to be executed (this is a long makefile command, I omitted non-relevant sections):

[ -x ./node ] && ./node || node tools/doc/generate.js --node-version=v9.0.0 --format=html --template=doc/template.html --analytics= doc/api/_toc.md > out/doc/api/_toc.html

It looks like the $(gen-json/html) arg isn't being passed to $(NODE), but is being passed to node. Would appreciate any suggestions for how to fix this (I've already tried some SO and Google searches) 😞

[refack]

Hello @maclover7 thanks for the fix 🥇

[krydos]

Oh my god. It's definitely me broke the html doc generation. I'm so sorry 😢

Thank you @maclover7 for the fix!

[addaleax]

Marking both this and #13482 as dont-land for now

[tniessen]

This works for me on Ubuntu. Would be great if someone else can verify this.

[krydos]

Works for me on MacOS. Can't understand why I didn't run make doc in #13482 :(

[krydos]

I guess this comment is misused here. We have copy of this comment above which is probably should be placed right above gen-doc definition.

[krydos]

Just to clarify, in our Makefile we have two comments:

 # check if ./node is actually set, else use user pre-installed binary

Both of them are not in the correct place. Line 511 is definitely strange comment I don't see any check for ./node there. Line 493 seems not correct as well since there are no checks too. But I see some checks in gen-doc. So one comment should be removed and another one placed to the gen-doc definition.

[refack]

Pre land CI: https://ci.nodejs.org/job/node-test-pull-request/9773/

[tniessen]

Landed in 8850fd4, thank you!

[refack]

Confirmed fix - https://nodejs.org/download/nightly/v9.0.0-nightly20170824abced13e29/docs/api/
Should only land on v8.x with #13482 a.k.a. 2710616

[MylesBorins]

This does not land cleanly in LTS. Please feel free to manually backport by following the guide. Please also feel free to replace do-not-land if it is being backported