Skip to content

Commit

Permalink
tools: make apilinks building more robust
Browse files Browse the repository at this point in the history
1. Move the apilinks.json file into out/doc so it gets cleaned when
  running `make docclean`
2. When the apilinks.json generated is empty, throw a specific error
  so it's easier to understand what's wrong
3. Write to a file passed through CLI arguments instead writing to
  stdout in apilinks.js so the build process is more robust in
  the case of a bad binary

PR-URL: #25019
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
joyeecheung authored and MylesBorins committed Dec 25, 2018
1 parent 4561e2c commit a5bccc2
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 17 deletions.
10 changes: 5 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -692,16 +692,16 @@ out/doc/api/assets/%: doc/api_assets/% out/doc/api/assets

run-npm-ci = $(PWD)/$(NPM) ci

LINK_DATA = out/doc/apilinks.json
gen-api = tools/doc/generate.js --node-version=$(FULLVERSION) \
--apilinks=out/apilinks.json $< --output-directory=out/doc/api
gen-apilink = tools/doc/apilinks.js $(wildcard lib/*.js) > $@
--apilinks=$(LINK_DATA) $< --output-directory=out/doc/api
gen-apilink = tools/doc/apilinks.js $(LINK_DATA) $(wildcard lib/*.js)

out/apilinks.json: $(wildcard lib/*.js) tools/doc/apilinks.js
$(LINK_DATA): $(wildcard lib/*.js) tools/doc/apilinks.js
$(call available-node, $(gen-apilink))

out/doc/api/%.json out/doc/api/%.html: doc/api/%.md tools/doc/generate.js \
tools/doc/html.js tools/doc/json.js tools/doc/apilinks.js | \
out/apilinks.json
tools/doc/html.js tools/doc/json.js tools/doc/apilinks.js | $(LINK_DATA)
$(call available-node, $(gen-api))

out/doc/api/all.html: $(apidocs_html) tools/doc/allhtml.js \
Expand Down
17 changes: 10 additions & 7 deletions test/doctool/test-apilinks.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,31 @@

require('../common');
const fixtures = require('../common/fixtures');
const tmpdir = require('../common/tmpdir');
const fs = require('fs');
const assert = require('assert');
const path = require('path');
const { execFileSync } = require('child_process');

const script = path.join(__dirname, '..', '..', 'tools', 'doc', 'apilinks.js');

const apilinks = fixtures.path('apilinks');

tmpdir.refresh();

fs.readdirSync(apilinks).forEach((fixture) => {
if (!fixture.endsWith('.js')) return;
const file = path.join(apilinks, fixture);

const expectedContent = fs.readFileSync(file + 'on', 'utf8');
const input = path.join(apilinks, fixture);

const output = execFileSync(
const expectedContent = fs.readFileSync(`${input}on`, 'utf8');
const outputPath = path.join(tmpdir.path, `${fixture}on`);
execFileSync(
process.execPath,
[script, file],
[script, outputPath, input],
{ encoding: 'utf-8' }
);

const expectedLinks = JSON.parse(expectedContent);
const actualLinks = JSON.parse(output);
const actualLinks = JSON.parse(fs.readFileSync(outputPath));

for (const [k, v] of Object.entries(expectedLinks)) {
assert.ok(k in actualLinks, `link not found: ${k}`);
Expand Down
6 changes: 4 additions & 2 deletions tools/doc/apilinks.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ const tag = execSync(`git describe --contains ${hash}`).split('\n')[0] || hash;

// Extract definitions from each file specified.
const definition = {};
process.argv.slice(2).forEach((file) => {
const output = process.argv[2];
const inputs = process.argv.slice(3);
inputs.forEach((file) => {
const basename = path.basename(file, '.js');

// Parse source.
Expand Down Expand Up @@ -206,4 +208,4 @@ process.argv.slice(2).forEach((file) => {
}
});

console.log(JSON.stringify(definition, null, 2));
fs.writeFileSync(output, JSON.stringify(definition, null, 2), 'utf8');
9 changes: 6 additions & 3 deletions tools/doc/generate.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,12 @@ args.forEach(function(arg) {
} else if (arg.startsWith('--output-directory=')) {
outputDir = arg.replace(/^--output-directory=/, '');
} else if (arg.startsWith('--apilinks=')) {
apilinks = JSON.parse(
fs.readFileSync(arg.replace(/^--apilinks=/, ''), 'utf8')
);
const linkFile = arg.replace(/^--apilinks=/, '');
const data = fs.readFileSync(linkFile, 'utf8');
if (!data.trim()) {
throw new Error(`${linkFile} is empty`);
}
apilinks = JSON.parse(data);
}
});

Expand Down

0 comments on commit a5bccc2

Please sign in to comment.