tools: don't build node for doc build

[thefourtheye]

Checklist

• make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

build tools

Description of change

Building node binary is not necessary for doc builds.

Refer: #9436 (comment)

---

cc @silverwind

[silverwind]

LGTM. @nodejs/build can you check if make doc-only is currently in use by the CI?

[sam-github]

no comment on implementation, but +1 for the feature

[rvagg]

But it is required to run tools/doc/generate.js (just below).

I'd be OK if you can replace the use of $(NODE) to run it or mess with it another way such that if an alternative NODE is provided then it won't build it, otherwise it will. Or maybe as a simpler version you could make it check if the binary at $(NODE) exists and if not then print an error that you need to make $(NODE_EXE) or similar.

[thefourtheye]

you could make it check if the binary at $(NODE) exists and if not then print an error that you need to make $(NODE_EXE) or similar.

Ya, I have already started working on it. I'll update the PR soon.

[thefourtheye]

you could make it check if the binary at $(NODE) exists and if not then print an error that you need to make $(NODE_EXE) or similar.

Ya, I have already started working on it. I'll update the PR soon.

I tried so many things, but the last commit is the best I could come up with. I could use some help here @nodejs/build

[rvagg]

This is only used to find one for the purpose of printing an error and not actually using it. Note that $(NODE) is still invoked for gen-json and gen-html which could be different to ./node or command -v node.

It also may be prudent to use which in here rather than command (see the XZ check).

So, perhaps this check should be used to determine which node to invoke but it should default to testing for $(NODE) rather than ./node (since ./node is the default but it can be overridden at runtime, e.g. NODE=blergh make .... The final command/which is a nice fallback that should get us by but that needs to carry on down to the execution of gen-json and gen-html.

[jasnell]

Status updates on this one?

[addaleax]

I think this has been resolved by introducing the doc-only target, so I’m closing. Feel free to reopen if I’m wrong.