diff --git a/.changeset/chilly-bikes-rule.md b/.changeset/chilly-bikes-rule.md new file mode 100644 index 0000000000..894999cf56 --- /dev/null +++ b/.changeset/chilly-bikes-rule.md @@ -0,0 +1,5 @@ +--- +'@techdocs/cli': patch +--- + +Updated to use new utilities from `@backstage/cli-common`. diff --git a/packages/techdocs-cli/src/commands/serve/mkdocs.ts b/packages/techdocs-cli/src/commands/serve/mkdocs.ts index 00c5e5f0ed..3938764fed 100644 --- a/packages/techdocs-cli/src/commands/serve/mkdocs.ts +++ b/packages/techdocs-cli/src/commands/serve/mkdocs.ts @@ -18,7 +18,7 @@ import { OptionValues } from 'commander'; import openBrowser from 'react-dev-utils/openBrowser'; import { createLogger } from '../../lib/utility'; import { runMkdocsServer } from '../../lib/mkdocsServer'; -import { LogFunc, waitForSignal } from '../../lib/run'; +import { RunLogFunc } from '@backstage/cli-common'; import { getMkdocsYml } from '@backstage/plugin-techdocs-node'; import fs from 'fs-extra'; import { checkIfDockerIsOperational } from './utils'; @@ -45,7 +45,7 @@ export default async function serveMkdocs(opts: OptionValues) { // We want to open browser only once based on a log. let boolOpenBrowserTriggered = false; - const logFunc: LogFunc = data => { + const logFunc: RunLogFunc = data => { // Sometimes the lines contain an unnecessary extra new line in between const logLines = data.toString().split('\n'); const logPrefix = opts.docker ? '[docker/mkdocs]' : '[mkdocs]'; @@ -74,7 +74,7 @@ export default async function serveMkdocs(opts: OptionValues) { // Had me questioning this whole implementation for half an hour. // Commander stores --no-docker in cmd.docker variable - const childProcess = await runMkdocsServer({ + const childProcess = runMkdocsServer({ port: opts.port, dockerImage: opts.dockerImage, dockerEntrypoint: opts.dockerEntrypoint, @@ -85,7 +85,7 @@ export default async function serveMkdocs(opts: OptionValues) { }); // Keep waiting for user to cancel the process - await waitForSignal([childProcess]); + await childProcess.waitForExit(); if (configIsTemporary) { process.on('exit', async () => { diff --git a/packages/techdocs-cli/src/commands/serve/serve.ts b/packages/techdocs-cli/src/commands/serve/serve.ts index 27f432c22d..c325a99871 100644 --- a/packages/techdocs-cli/src/commands/serve/serve.ts +++ b/packages/techdocs-cli/src/commands/serve/serve.ts @@ -17,10 +17,9 @@ import { OptionValues } from 'commander'; import path from 'path'; import openBrowser from 'react-dev-utils/openBrowser'; -import { findPaths } from '@backstage/cli-common'; +import { findPaths, RunLogFunc } from '@backstage/cli-common'; import HTTPServer from '../../lib/httpServer'; import { runMkdocsServer } from '../../lib/mkdocsServer'; -import { LogFunc, waitForSignal } from '../../lib/run'; import { createLogger } from '../../lib/utility'; import { getMkdocsYml } from '@backstage/plugin-techdocs-node'; import fs from 'fs-extra'; @@ -83,7 +82,7 @@ export default async function serve(opts: OptionValues) { } let mkdocsServerHasStarted = false; - const mkdocsLogFunc: LogFunc = data => { + const mkdocsLogFunc: RunLogFunc = data => { // Sometimes the lines contain an unnecessary extra new line const logLines = data.toString().split('\n'); const logPrefix = opts.docker ? '[docker/mkdocs]' : '[mkdocs]'; @@ -107,7 +106,7 @@ export default async function serve(opts: OptionValues) { // https://github.com/mkdocs/mkdocs/issues/879#issuecomment-203536006 // Had me questioning this whole implementation for half an hour. logger.info('Starting mkdocs server.'); - const mkdocsChildProcess = await runMkdocsServer({ + const mkdocsChildProcess = runMkdocsServer({ port: opts.mkdocsPort, dockerImage: opts.dockerImage, dockerEntrypoint: opts.dockerEntrypoint, @@ -161,7 +160,7 @@ export default async function serve(opts: OptionValues) { ); }); - await waitForSignal([mkdocsChildProcess]); + await mkdocsChildProcess.waitForExit(); if (configIsTemporary) { process.on('exit', async () => { diff --git a/packages/techdocs-cli/src/commands/serve/utils.ts b/packages/techdocs-cli/src/commands/serve/utils.ts index 2b19b0fbc0..9ecb8bc29b 100644 --- a/packages/techdocs-cli/src/commands/serve/utils.ts +++ b/packages/techdocs-cli/src/commands/serve/utils.ts @@ -14,25 +14,22 @@ * limitations under the License. */ -import { promisify } from 'util'; import * as winston from 'winston'; -import { execFile } from 'child_process'; +import { runCheck } from '@backstage/cli-common'; export async function checkIfDockerIsOperational( logger: winston.Logger, ): Promise { logger.info('Checking Docker status...'); - try { - const runCheck = promisify(execFile); - await runCheck('docker', ['info'], { shell: true }); + const isOperational = await runCheck(['docker', 'info']); + if (isOperational) { logger.info( 'Docker is up and running. Proceed to starting up mkdocs server', ); return true; - } catch { - logger.error( - 'Docker is not running. Exiting. Please check status of Docker daemon with `docker info` before re-running', - ); - return false; } + logger.error( + 'Docker is not running. Exiting. Please check status of Docker daemon with `docker info` before re-running', + ); + return false; } diff --git a/packages/techdocs-cli/src/lib/mkdocsServer.test.ts b/packages/techdocs-cli/src/lib/mkdocsServer.test.ts index 83f9c9f382..5acd1784a3 100644 --- a/packages/techdocs-cli/src/lib/mkdocsServer.test.ts +++ b/packages/techdocs-cli/src/lib/mkdocsServer.test.ts @@ -15,9 +15,9 @@ */ import { runMkdocsServer } from './mkdocsServer'; -import { run } from './run'; +import { run } from '@backstage/cli-common'; -jest.mock('./run', () => { +jest.mock('@backstage/cli-common', () => { return { run: jest.fn(), }; @@ -29,12 +29,12 @@ describe('runMkdocsServer', () => { }); describe('docker', () => { - it('should run docker directly by default', async () => { - await runMkdocsServer({}); + it('should run docker directly by default', () => { + runMkdocsServer({}); expect(run).toHaveBeenCalledWith( - 'docker', expect.arrayContaining([ + 'docker', 'run', `${process.cwd()}:/content`, '8000:8000', @@ -47,26 +47,24 @@ describe('runMkdocsServer', () => { ); }); - it('should accept port option', async () => { - await runMkdocsServer({ port: '5678' }); + it('should accept port option', () => { + runMkdocsServer({ port: '5678' }); expect(run).toHaveBeenCalledWith( - 'docker', - expect.arrayContaining(['5678:5678', '0.0.0.0:5678']), + expect.arrayContaining(['docker', '5678:5678', '0.0.0.0:5678']), expect.objectContaining({}), ); }); - it('should accept custom docker image', async () => { - await runMkdocsServer({ dockerImage: 'my-org/techdocs' }); + it('should accept custom docker image', () => { + runMkdocsServer({ dockerImage: 'my-org/techdocs' }); expect(run).toHaveBeenCalledWith( - 'docker', - expect.arrayContaining(['my-org/techdocs']), + expect.arrayContaining(['docker', 'my-org/techdocs']), expect.objectContaining({}), ); }); - it('should accept custom docker options', async () => { - await runMkdocsServer({ + it('should accept custom docker options', () => { + runMkdocsServer({ dockerOptions: [ '--add-host=internal.host:192.168.11.12', '--name', @@ -75,8 +73,8 @@ describe('runMkdocsServer', () => { }); expect(run).toHaveBeenCalledWith( - 'docker', expect.arrayContaining([ + 'docker', 'run', '--rm', '-w', @@ -98,14 +96,14 @@ describe('runMkdocsServer', () => { ); }); - it('should accept additinoal mkdocs CLI parameters', async () => { - await runMkdocsServer({ + it('should accept additinoal mkdocs CLI parameters', () => { + runMkdocsServer({ mkdocsParameterClean: true, mkdocsParameterStrict: true, }); expect(run).toHaveBeenCalledWith( - 'docker', expect.arrayContaining([ + 'docker', 'serve', '--dev-addr', '0.0.0.0:8000', @@ -118,21 +116,24 @@ describe('runMkdocsServer', () => { }); describe('mkdocs', () => { - it('should run mkdocs if specified', async () => { - await runMkdocsServer({ useDocker: false }); + it('should run mkdocs if specified', () => { + runMkdocsServer({ useDocker: false }); expect(run).toHaveBeenCalledWith( - 'mkdocs', - expect.arrayContaining(['serve', '--dev-addr', '127.0.0.1:8000']), + expect.arrayContaining([ + 'mkdocs', + 'serve', + '--dev-addr', + '127.0.0.1:8000', + ]), expect.objectContaining({}), ); }); - it('should accept port option', async () => { - await runMkdocsServer({ useDocker: false, port: '5678' }); + it('should accept port option', () => { + runMkdocsServer({ useDocker: false, port: '5678' }); expect(run).toHaveBeenCalledWith( - 'mkdocs', - expect.arrayContaining(['127.0.0.1:5678']), + expect.arrayContaining(['mkdocs', '127.0.0.1:5678']), expect.objectContaining({}), ); }); diff --git a/packages/techdocs-cli/src/lib/mkdocsServer.ts b/packages/techdocs-cli/src/lib/mkdocsServer.ts index a793643367..929ee77fb1 100644 --- a/packages/techdocs-cli/src/lib/mkdocsServer.ts +++ b/packages/techdocs-cli/src/lib/mkdocsServer.ts @@ -14,30 +14,29 @@ * limitations under the License. */ -import { ChildProcess } from 'child_process'; -import { run, LogFunc } from './run'; +import { run, RunChildProcess, RunLogFunc } from '@backstage/cli-common'; -export const runMkdocsServer = async (options: { +export const runMkdocsServer = (options: { port?: string; useDocker?: boolean; dockerImage?: string; dockerEntrypoint?: string; dockerOptions?: string[]; - stdoutLogFunc?: LogFunc; - stderrLogFunc?: LogFunc; + stdoutLogFunc?: RunLogFunc; + stderrLogFunc?: RunLogFunc; mkdocsConfigFileName?: string; mkdocsParameterClean?: boolean; mkdocsParameterDirtyReload?: boolean; mkdocsParameterStrict?: boolean; -}): Promise => { +}): RunChildProcess => { const port = options.port ?? '8000'; const useDocker = options.useDocker ?? true; const dockerImage = options.dockerImage ?? 'spotify/techdocs'; if (useDocker) { - return await run( - 'docker', + return run( [ + 'docker', 'run', '--rm', '-w', @@ -69,9 +68,9 @@ export const runMkdocsServer = async (options: { ); } - return await run( - 'mkdocs', + return run( [ + 'mkdocs', 'serve', '--dev-addr', `127.0.0.1:${port}`, diff --git a/packages/techdocs-cli/src/lib/run.ts b/packages/techdocs-cli/src/lib/run.ts deleted file mode 100644 index 13ac7d9d4c..0000000000 --- a/packages/techdocs-cli/src/lib/run.ts +++ /dev/null @@ -1,99 +0,0 @@ -/* - * Copyright 2020 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import { spawn, SpawnOptions, ChildProcess } from 'child_process'; - -export type LogFunc = (data: Buffer | string) => void; -type SpawnOptionsPartialEnv = Omit & { - env?: Partial; - // Pipe stdout to this log function - stdoutLogFunc?: LogFunc; - // Pipe stderr to this log function - stderrLogFunc?: LogFunc; -}; - -// TODO: Accept log functions to pipe logs with. -// Runs a child command, returning the child process instance. -// Use it along with waitForSignal to run a long running process e.g. mkdocs serve -export const run = async ( - name: string, - args: string[] = [], - options: SpawnOptionsPartialEnv = {}, -): Promise => { - const { stdoutLogFunc, stderrLogFunc } = options; - - const env: NodeJS.ProcessEnv = { - ...process.env, - FORCE_COLOR: 'true', - ...(options.env ?? {}), - }; - - // Refer: https://nodejs.org/api/child_process.html#child_process_subprocess_stdio - const stdio = [ - 'inherit', - stdoutLogFunc ? 'pipe' : 'inherit', - stderrLogFunc ? 'pipe' : 'inherit', - ] as ('inherit' | 'pipe')[]; - - const childProcess = spawn(name, args, { - stdio: stdio, - ...options, - env, - }); - - if (stdoutLogFunc && childProcess.stdout) { - childProcess.stdout.on('data', stdoutLogFunc); - } - if (stderrLogFunc && childProcess.stderr) { - childProcess.stderr.on('data', stderrLogFunc); - } - - return childProcess; -}; - -// Block indefinitely and wait for a signal to stop the child process(es) -// Throw error if any child process errors -// Resolves only when all processes exit with status code 0 -export async function waitForSignal( - childProcesses: Array, -): Promise { - const promises: Array> = []; - - for (const signal of ['SIGINT', 'SIGTERM'] as const) { - process.on(signal, () => { - childProcesses.forEach(childProcess => { - childProcess.kill(); - }); - }); - } - - childProcesses.forEach(childProcess => { - if (typeof childProcess.exitCode === 'number') { - if (childProcess.exitCode) { - throw new Error(`Non zero exit code from child process`); - } - return; - } - - promises.push( - new Promise((resolve, reject) => { - childProcess.once('error', reject); - childProcess.once('exit', resolve); - }), - ); - }); - - await Promise.all(promises); -}