8000 tools: only fetch previous versions when necessary · nodejs/node@a175b8d · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit a175b8d

Browse files
richardlauBethGriggs
richardlau
authored and
BethGriggs
committed
tools: only fetch previous versions when necessary
Refactor the logic for working out the previous versions of Node.js for the API documentation so that the parsing (including the potential https get) happens at most once per build (as opposed to the current once per generated API doc). Signed-off-by: Richard Lau <riclau@uk.ibm.com> Backport-PR-URL: #32642 PR-URL: #32518 Fixes: #32512 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
1 parent 3756be8 commit a175b8d

File tree

6 files changed

+132
-88
lines changed

6 files changed

+132
-88
lines changed

‎Makefile‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -649,15 +649,22 @@ out/doc/api/assets/%: doc/api_assets/% out/doc/api/assets
649649
run-npm-ci = $(PWD)/$(NPM) ci
650650

651651
LINK_DATA = out/doc/apilinks.json
652+
VERSIONS_DATA = out/doc/previous-versions.json
652653
gen-api = tools/doc/generate.js --node-version=$(FULLVERSION) \
653-
--apilinks=$(LINK_DATA) $< --output-directory=out/doc/api
654+
--apilinks=$(LINK_DATA) $< --output-directory=out/doc/api \
655+
--versions-file=$(VERSIONS_DATA)
654656
gen-apilink = tools/doc/apilinks.js $(LINK_DATA) $(wildcard lib/*.js)
655657

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

661+
# Regenerate previous versions data if the current version changes
662+
$(VERSIONS_DATA): CHANGELOG.md src/node_version.h tools/doc/versions.js
663+
$(call available-node, tools/doc/versions.js $@)
664+
659665
out/doc/api/%.json out/doc/api/%.html: doc/api/%.md tools/doc/generate.js \
660-
tools/doc/html.js tools/doc/json.js tools/doc/apilinks.js | $(LINK_DATA)
666+
tools/doc/html.js tools/doc/json.js tools/doc/apilinks.js \
667+
$(VERSIONS_DATA) | $(LINK_DATA)
661668
$(call available-node, $(gen-api))
662669

663670
out/doc/api/all.html: $(apidocs_html) tools/doc/allhtml.js \

‎test/doctool/test-doctool-html.js‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const remark2rehype = require('remark-rehype');
2222
const raw = require('rehype-raw');
2323
const htmlStringify = require('rehype-stringify');
2424

25-
async function toHTML({ input, filename, nodeVersion }) {
25+
function toHTML({ input, filename, nodeVersion, versions }) {
2626
const content = unified()
2727
.use(markdown)
2828
.use(html.firstHeader)
@@ -34,7 +34,7 @@ async function toHTML({ input, filename, nodeVersion }) {
3434
.use(htmlStringify)
3535
.processSync(input);
3636

37-
return html.toHTML({ input, content, filename, nodeVersion });
37+
return html.toHTML({ input, content, filename, nodeVersion, versions });
3838
}
3939

4040
// Test data is a list of objects with two properties.
@@ -99,16 +99,27 @@ const testData = [
9999
];
100100

101101
const spaces = /\s/g;
102+
const versions = [
103+
{ num: '10.x', lts: true },
104+
{ num: '9.x' },
105+
{ num: '8.x' },
106+
{ num: '7.x' },
107+
{ num: '6.x' },
108+
{ num: '5.x' },
109+
{ num: '4.x' },
110+
{ num: '0.12.x' },
111+
{ num: '0.10.x' }];
102112

103113
testData.forEach(({ file, html }) => {
104114
// Normalize expected data by stripping whitespace.
105115
const expected = html.replace(spaces, '');
106116

107117
readFile(file, 'utf8', common.mustCall(async (err, input) => {
108118
assert.ifError(err);
109-
const output = await toHTML({ input: input,
110-
filename: 'foo',
111-
nodeVersion: process.version });
119+
const output = toHTML({ input: input,
120+
filename: 'foo',
121+
nodeVersion: process.version,
122+
versions: versions });
112123

113124
const actual = output.replace(spaces, '');
114125
// Assert that the input stripped of all whitespace contains the

‎test/doctool/test-doctool-versions.js‎

Lines changed: 49 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,14 @@
22

33
require('../common');
44
const assert = require('assert');
5+
const { spawnSync } = require('child_process');
6+
const fs = require('fs');
7+
const path = require('path');
8+
const tmpdir = require('../common/tmpdir');
59
const util = require('util');
6-
const { versions } = require('../../tools/doc/versions.js');
10+
11+
const debuglog = util.debuglog('test');
12+
const versionsTool = path.join('../../tools/doc/versions.js');
713

814
// At the time of writing these are the minimum expected versions.
915
// New versions of Node.js do not have to be explicitly added here.
@@ -21,39 +27,48 @@ const expected = [
2127
'0.10.x',
2228
];
2329

24-
async function test() {
25-
const vers = await versions();
26-
// Coherence checks for each returned version.
27-
for (const version of vers) {
28-
const tested = util.inspect(version);
29-
const parts = version.num.split('.');
30-
const expectedLength = parts[0] === '0' ? 3 : 2;
31-
assert.strictEqual(parts.length, expectedLength,
32-
`'num' from ${tested} should be '<major>.x'.`);
33-
assert.strictEqual(parts[parts.length - 1], 'x',
34-
`'num' from ${tested} doesn't end in '.x'.`);
35-
const isEvenRelease = Number.parseInt(parts[expectedLength - 2]) % 2 === 0;
36-
const hasLtsProperty = version.hasOwnProperty('lts');
37-
if (hasLtsProperty) {
38-
// Odd-numbered versions of Node.js are never LTS.
39-
assert.ok(isEvenRelease, `${tested} should not be an 'lts' release.`);
40-
assert.ok(version.lts, `'lts' from ${tested} should 'true'.`);
41-
}
42-
}
30+
tmpdir.refresh();
31+
const versionsFile = path.join(tmpdir.path, 'versions.json');
32+
debuglog(versionsFile);
33+
const opts = { cwd: tmpdir.path, encoding: 'utf8' };
34+
const cp = spawnSync(process.execPath, [ versionsTool, versionsFile ], opts);
35+
debuglog(cp.stderr);
36+
debuglog(cp.stdout);
37+
assert.strictEqual(cp.stdout, '');
38+
assert.strictEqual(cp.signal, null);
39+
assert.strictEqual(cp.status, 0);
40+
const versions = JSON.parse(fs.readFileSync(versionsFile));
41+< E880 div class="diff-text-inner">debuglog(versions);
4342

44-
// Check that the minimum number of versions were returned.
45-
// Later versions are allowed, but not checked for here (they were checked
46-
// above).
47-
// Also check for the previous semver major -- From master this will be the
48-
// most recent major release.
49-
const thisMajor = Number.parseInt(process.versions.node.split('.')[0]);
50-
const prevMajorString = `${thisMajor - 1}.x`;
51-
if (!expected.includes(prevMajorString)) {
52-
expected.unshift(prevMajorString);
53-
}
54-
for (const version of expected) {
55-
assert.ok(vers.find((x) => x.num === version),
56-
`Did not find entry for '${version}' in ${util.inspect(vers)}`);
43+
// Coherence checks for each returned version.
44+
for (const version of versions) {
45+
const tested = util.inspect(version);
46+
const parts = version.num.split('.');
47+
const expectedLength = parts[0] === '0' ? 3 : 2;
48+
assert.strictEqual(parts.length, expectedLength,
49+
`'num' from ${tested} should be '<major>.x'.`);
50+
assert.strictEqual(parts[parts.length - 1], 'x',
51+
`'num' from ${tested} doesn't end in '.x'.`);
52+
const isEvenRelease = Number.parseInt(parts[expectedLength - 2]) % 2 === 0;
53+
const hasLtsProperty = version.hasOwnProperty('lts');
54+
if (hasLtsProperty) {
55+
// Odd-numbered versions of Node.js are never LTS.
56+
assert.ok(isEvenRelease, `${tested} should not be an 'lts' release.`);
57+
assert.ok(version.lts, `'lts' from ${tested} should 'true'.`);
5758
}
5859
}
59-
test();
60+
61+
// Check that the minimum number of versions were returned.
62+
// Later versions are allowed, but not checked for here (they were checked
63+
// above).
64+
// Also check for the previous semver major -- From master this will be the
65+
// most recent major release.
66+
const thisMajor = Number.parseInt(process.versions.node.split('.')[0]);
67+
const prevMajorString = `${thisMajor - 1}.x`;
68+
if (!expected.includes(prevMajorString)) {
69+
expected.unshift(prevMajorString);
70+
}
71+
for (const version of expected) {
72+
assert.ok(versions.find((x) => x.num === version),
73+
`Did not find entry for '${version}' in ${util.inspect(versions)}`);
74+
}

‎tools/doc/generate.js‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ let filename = null;
4040
let nodeVersion = null;
4141
let outputDir = null;
4242
let apilinks = {};
43+
let versions = {};
4344

4445
args.forEach(function(arg) {
4546
if (!arg.startsWith('--')) {
@@ -55,6 +56,13 @@ args.forEach(function(arg) {
5556
throw new Error(`${linkFile} is empty`);
5657
}
5758
apilinks = JSON.parse(data);
59+
} else if (arg.startsWith('--versions-file=')) {
60+
const versionsFile = arg.replace(/^--versions-file=/, '');
61+
const data = fs.readFileSync(versionsFile, 'utf8');
62+
if (!data.trim()) {
63+
throw new Error(`${versionsFile} is empty`);
64+
}
65+
versions = JSON.parse(data);
5866
}
5967
});
6068

@@ -84,7 +92,8 @@ fs.readFile(filename, 'utf8', async (er, input) => {
8492

8593
const basename = path.basename(filename, '.md');
8694

87-
const myHtml = await html.toHTML({ input, content, filename, nodeVersion });
95+
const myHtml = html.toHTML({ input, content, filename, nodeVersion,
96+
versions });
8897
const htmlTarget = path.join(outputDir, `${basename}.html`);
8998
fs.writeFileSync(htmlTarget, myHtml);
9099

‎tools/doc/html.js‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323

2424
const common = require('./common.js');
2525
const fs = require('fs');
26-
const getVersions = require('./versions.js');
2726
const unified = require('unified');
2827
const find = require('unist-util-find');
2928
const visit = require('unist-util-visit');
@@ -63,7 +62,7 @@ const gtocHTML = unified()
6362
const templatePath = path.join(docPath, 'template.html');
6463
const template = fs.readFileSync(templatePath, 'utf8');
6564

66-
async function toHTML({ input, content, filename, nodeVersion }) {
65+
function toHTML({ input, content, filename, nodeVersion, versions }) {
6766
filename = path.basename(filename, '.md');
6867

6968
const id = filename.replace(/\W+/g, '-');
@@ -81,7 +80,7 @@ async function toHTML({ input, content, filename, nodeVersion }) {
8180
const docCreated = input.match(
8281
/<!--\s*introduced_in\s*=\s*v([0-9]+)\.([0-9]+)\.[0-9]+\s*-->/);
8382
if (docCreated) {
84-
HTML = HTML.replace('__ALTDOCS__', await altDocs(filename, docCreated));
83+
HTML = HTML.replace('__ALTDOCS__', altDocs(filename, docCreated, versions));
8584
} else {
8685
console.error(`Failed to add alternative version links to ${filename}`);
8786
HTML = HTML.replace('__ALTDOCS__', '');
@@ -381,10 +380,9 @@ function getId(text, idCounters) {
381380
return text;
382381
}
383382

384-
async function altDocs(filename, docCreated) {
383+
function altDocs(filename, docCreated, versions) {
385384
const [, docCreatedMajor, docCreatedMinor] = docCreated.map(Number);
386385
const host = 'https://nodejs.org';
387-
const versions = await getVersions.versions();
388386

389387
const getHref = (versionNum) =>
390388
`${host}/docs/latest-v${versionNum}/api/${filename}.html`;

‎tools/doc/versions.js‎

Lines changed: 45 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
'use strict';
22

3-
const { readFileSync } = require('fs');
3+
const { readFileSync, writeFileSync } = require('fs');
44
const path = require('path');
55
const srcRoot = path.join(__dirname, '..', '..');
66

7-
let _versions;
8-
97
const isRelease = () => {
108
const re = /#define NODE_VERSION_IS_RELEASE 0/;
119
const file = path.join(srcRoot, 'src', 'node_version.h');
@@ -15,7 +13,7 @@ const isRelease = () => {
1513
const getUrl = (url) => {
1614
return new Promise((resolve, reject) => {
1715
const https = require('https');
18-
const request = https.get(url, { timeout: 5000 }, (response) => {
16+
const request = https.get(url, { timeout: 30000 }, (response) => {
1917
if (response.statusCode !== 200) {
2018
reject(new Error(
2119
`Failed to get ${url}, status code ${response.statusCode}`));
@@ -32,45 +30,51 @@ const getUrl = (url) => {
3230
};
3331

3432
const kNoInternet = !!process.env.NODE_TEST_NO_INTERNET;
33+
const outFile = (process.argv.length > 2 ? process.argv[2] : undefined);
3534

36-
module.exports = {
37-
async versions() {
38-
if (_versions) {
39-
return _versions;
40-
}
41-
42-
// The CHANGELOG.md on release branches may not reference newer semver
43-
// majors of Node.js so fetch and parse the version from the master branch.
44-
const url =
45-
'https://raw.githubusercontent.com/nodejs/node/master/CHANGELOG.md';
46-
let changelog;
47-
const file = path.join(srcRoot, 'CHANGELOG.md');
48-
if (kNoInternet) {
49-
changelog = readFileSync(file, { encoding: 'utf8' });
50-
} else {
51-
try {
52-
changelog = await getUrl(url);
53-
} catch (e) {
54-
// Fail if this is a release build, otherwise fallback to local files.
55-
if (isRelease()) {
56-
throw e;
57-
} else {
58-
console.warn(`Unable to retrieve ${url}. Falling back to ${file}.`);
59-
changelog = readFileSync(file, { encoding: 'utf8' });
60-
}
35+
async function versions() {
36+
// The CHANGELOG.md on release branches may not reference newer semver
37+
// majors of Node.js so fetch and parse the version from the master branch.
38+
const url =
39+
'https://raw.githubusercontent.com/nodejs/node/master/CHANGELOG.md';
40+
let changelog;
41+
const file = path.join(srcRoot, 'CHANGELOG.md');
42+
if (kNoInternet) {
43+
changelog = readFileSync(file, { encoding: 'utf8' });
44+
} else {
45+
try {
46+
changelog = await getUrl(url);
47+
} catch (e) {
48+
// Fail if this is a release build, otherwise fallback to local files.
49+
if (isRelease()) {
50+
throw e;
51+
} else {
52+
console.warn(`Unable to retrieve ${url}. Falling back to ${file}.`);
53+
changelog = readFileSync(file, { encoding: 'utf8' });
6154
}
6255
}
63-
const ltsRE = /Long Term Support/i;
64-
const versionRE = /\* \[Node\.js ([0-9.]+)\][^-—]+[-—]\s*(.*)\r?\n/g;
65-
_versions = [];
66-
let match;
67-
while ((match = versionRE.exec(changelog)) != null) {
68-
const entry = { num: `${match[1]}.x` };
69-
if (ltsRE.test(match[2])) {
70-
entry.lts = true;
71-
}
72-
_versions.push(entry);
56+
}
57+
const ltsRE = /Long Term Support/i;
58+
const versionRE = /\* \[Node\.js ([0-9.]+)\]\S+ (.*)\r?\n/g;
59+
const _versions = [];
60+
let match;
61+
while ((match = versionRE.exec(changelog)) != null) {
62+
const entry = { num: `${match[1]}.x` };
63+
if (ltsRE.test(match[2])) {
64+
entry.lts = true;
7365
}
74-
return _versions;
66+
_versions.push(entry);
7567
}
76-
};
68+
return _versions;
69+
}
70+
71+
versions().then((v) => {
72+
if (outFile) {
73+
writeFileSync(outFile, JSON.stringify(v));
74+
} else {
75+
console.log(JSON.stringify(v));
76+
}
77+
}).catch((err) => {
78+
console.error(err);
79+
process.exit(1);
80+
});

0 commit comments

Comments
 (0)
0