From cf22d460ff702fc68a73ff790238999df72f51ff Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Fri, 28 Aug 2026 10:29:28 +0200 Subject: [PATCH] feat(developer): support output folder and `--continue-on-error` Add support for specifying an output folder instead of filename for the `--out-file` parameter of kmc. This allows for batch builds, which can be much faster than invoking kmc for each file separately. Alongside that, add a `--continue-on-error` parameter that allows for batch builds to continue even if a single file fails to build. (Note that if a fatal error is encountered, kmc aborts with exit code 70). --- .../docs/help/reference/kmc/cli/reference.md | 21 ++- developer/src/kmc/src/commands/build.ts | 120 +++++++++++++++--- .../src/messages/infrastructureMessages.ts | 17 ++- developer/src/kmc/src/util/baseOptions.ts | 8 +- developer/src/kmc/src/util/sysexits.ts | 1 + developer/src/kmc/test/build.tests.ts | 104 ++++++++++++++- .../kmc/test/infrastructureMessages.tests.ts | 5 +- 7 files changed, 249 insertions(+), 27 deletions(-) diff --git a/developer/docs/help/reference/kmc/cli/reference.md b/developer/docs/help/reference/kmc/cli/reference.md index 662d85a0a3..943e181ba5 100644 --- a/developer/docs/help/reference/kmc/cli/reference.md +++ b/developer/docs/help/reference/kmc/cli/reference.md @@ -185,9 +185,24 @@ The following parameters are available: `-o `, `--out-file ` -: Overrides the default path and filename for the output file(s). Note that - some compilers emit multiple files, in which case, the output filenames - will vary by file extension. +: Overrides the default path and filename for the output file(s). Note that some + compilers emit multiple files, in which case, the output filenames will vary + by file extension. + + If the output filename ends in a forward slash (`/`) or backslash (`\\`), or + if multiple input files are specified, or if the output filename exists and is + already a folder, then the filename will be treated as a folder, and all + output files will be written within that. + + An error will be raised if an output file is specified and already exists, and + is a regular file, but is expected to be a folder. + +`--continue-on-error` + +: When building multiple input files, continue building subsequent files even if + a file fails to build. Note that `kmc` will still abort if an internal error + is encountered. + ## `kmc build file` additional options diff --git a/developer/src/kmc/src/commands/build.ts b/developer/src/kmc/src/commands/build.ts index 7e21f99f9c..b45dc23e97 100644 --- a/developer/src/kmc/src/commands/build.ts +++ b/developer/src/kmc/src/commands/build.ts @@ -1,3 +1,8 @@ +/* + * Keyman is copyright (C) SIL Global. MIT License. + * + * Implementation of the `kmc build` command + */ import * as fs from 'fs'; import * as path from 'path'; import { Command } from 'commander'; @@ -13,7 +18,12 @@ import { isProject } from '../util/projectLoader.js'; import { buildTestData } from './buildTestData/index.js'; import { buildWindowsPackageInstaller } from './buildWindowsPackageInstaller/index.js'; import { commanderOptionsToCompilerOptions } from '../util/extendedCompilerOptions.js'; -import { exitProcess } from '../util/sysexits.js'; +import { exitProcess, SysExits } from '../util/sysexits.js'; + +interface OutFileSpecification { + isFolder?: boolean; + path?: string; +}; export function declareBuild(program: Command) { // TODO: localization? @@ -28,7 +38,8 @@ export function declareBuild(program: Command) { .option('-m, --message ', 'Adjust severity of info, hint or warning message to Disable (default), Info, Hint, Warn or Error (option can be repeated)', (value, previous) => previous.concat([value]), []) .option('--no-compiler-version', 'Exclude compiler version metadata from output') - .option('--no-warn-deprecated-code', 'Turn off warnings for deprecated code styles'); + .option('--no-warn-deprecated-code', 'Turn off warnings for deprecated code styles') + .option('--continue-on-error', 'Continue build of subsequent files even if earlier files have errors'); BuildBaseOptions.addAll(buildCommand); @@ -92,27 +103,43 @@ async function buildFile(filenames: string[], _options: any, commander: any): Pr filenames.push('.'); } - /* c8 ignore next 6 */ - // full test on console log of error message not justified; check with user test recommended - if(filenames.length > 1 && commanderOptions.outFile) { - // -o can only be specified with a single input file - callbacks.reportMessage(InfrastructureMessages.Error_OutFileCanOnlyBeSpecifiedWithSingleInfile()); - return await exitProcess(1); - } - if(!expandFileLists(filenames, callbacks)) { return await exitProcess(1); } + const spec = interpretOutFile(commanderOptions.outFile, filenames.length, fs.statSync); + if(!spec) { + callbacks.reportMessage(InfrastructureMessages.Error_OutFileMustBeAFolder()); + return await exitProcess(1); + } + + let result = true; for(const filename of filenames) { - if(!await build(filename, commanderOptions.outFile, callbacks, options)) { - // Once a file fails to build, we bail on subsequent builds - return await exitProcess(1); + const buildResult = await build(filename, spec, callbacks, options); + if(!buildResult) { + result = false; + if(buildResult === null) { + // We always die on fatal exceptions + return await exitProcess(SysExits.EX_SOFTWARE); + } + if(!commanderOptions.continueOnError) { + // Once a file fails to build, we bail on subsequent builds + return await exitProcess(1); + } } } + + if(!result) { + // If any file failed to build, then we return failure, + return await exitProcess(1); + } } -async function build(filename: string, outfile: string, parentCallbacks: NodeCompilerCallbacks, options: CompilerOptions): Promise { +/** + * Build a single file + * @returns true on success, false on build failure, null on internal errors + */ +async function build(filename: string, outfileSpec: OutFileSpecification, parentCallbacks: NodeCompilerCallbacks, options: CompilerOptions): Promise { try { // TEST: allow command-line simulation of infrastructure fatal errors, and // also for unit tests @@ -158,6 +185,21 @@ async function build(filename: string, outfile: string, parentCallbacks: NodeCom const callbacks = new CompilerFileCallbacks(buildFilename, options, parentCallbacks); callbacks.reportMessage(InfrastructureMessages.Info_BuildingFile({filename:buildFilename, relativeFilename})); + let outfile = outfileSpec?.path; + + // Special case for directory outfile - create folder if required + if(outfileSpec?.isFolder) { + if(!fs.existsSync(outfileSpec.path)) { + fs.mkdirSync(outfileSpec.path, { recursive: true }); + } + + let base = path.basename(filename); + if(base.endsWith(builder.sourceExtension)) { + base = base.substring(base.length - builder.sourceExtension.length) + builder.compiledExtension; + } + outfile = path.join(outfile, base); + } + let result = await builder.build(filename, outfile, callbacks, options); result = result && !callbacks.hasFailureMessage(); if(result) { @@ -178,13 +220,61 @@ async function build(filename: string, outfile: string, parentCallbacks: NodeCom return result; } catch(e) { parentCallbacks.reportMessage(InfrastructureMessages.Fatal_UnexpectedException({e})); - return false; + return null; } } +/** + * Interpret the output filename based on shape of input and filesystem status. + * + * The output path is treated as a folder if: + * (a) more than one input file is specified, or + * (b) it ends with a trailing slash or backslash, or + * (c) it already exists and is a folder + * + * @param path + * @param inputFilenameCount + * @returns null if a regular file is in the way of a folder, otherwise a specification + */ +function interpretOutFile(path: string|undefined, inputFilenameCount: number, statSync: (path: fs.PathLike, options?: fs.StatSyncOptions) => {isDirectory?: ()=>boolean}): OutFileSpecification { + if(path === null || !inputFilenameCount || inputFilenameCount < 1) { + throw new Error('Invalid parameters'); + } + + const spec: OutFileSpecification = { + isFolder: false, + path + }; + + if(!path) { + return spec; + } + + if(inputFilenameCount > 1 || + (path.endsWith('/') || path.endsWith('\\')) || + statSync(path, {throwIfNoEntry: false})?.isDirectory()) { + + spec.isFolder = true; + + if(path.endsWith('/') || path.endsWith('\\')) { + // remove trailing delimiter + spec.path = path.substring(0, path.length-1); + } + + const stats = statSync(spec.path, {throwIfNoEntry: false}); + if(stats && !stats.isDirectory()) { + // existing file is in the way + return null; + } + } + + return spec; +} + /** * these are exported only for unit tests, do not use */ export const unitTestEndpoints = { build, + interpretOutFile, }; diff --git a/developer/src/kmc/src/messages/infrastructureMessages.ts b/developer/src/kmc/src/messages/infrastructureMessages.ts index 338af89073..7a7e8f00bd 100644 --- a/developer/src/kmc/src/messages/infrastructureMessages.ts +++ b/developer/src/kmc/src/messages/infrastructureMessages.ts @@ -1,3 +1,6 @@ +/* + * Keyman is copyright (C) SIL Global. MIT License. + */ import { CompilerError, CompilerErrorNamespace, CompilerErrorSeverity, CompilerMessageSpec as m, CompilerMessageDef as def, CompilerMessageSpecWithException } from "@keymanapp/developer-utils"; const Namespace = CompilerErrorNamespace.Infrastructure; @@ -88,10 +91,6 @@ export class InfrastructureMessages { static Hint_ProjectIsVersion10 = () => m(this.HINT_ProjectIsVersion10, `The project file is an older version and can be upgraded to version 17.0`); - static ERROR_OutFileCanOnlyBeSpecifiedWithSingleInfile = SevError | 0x0015; - static Error_OutFileCanOnlyBeSpecifiedWithSingleInfile = () => m(this.ERROR_OutFileCanOnlyBeSpecifiedWithSingleInfile, - `Parameter --out-file can only be used with a single input file.`); - static ERROR_InvalidMessageFormat = SevError | 0x0016; static Error_InvalidMessageFormat = (o:{message:string}) => m(this.ERROR_InvalidMessageFormat, `Invalid parameter: --message ${def(o.message)} must match format '[KM]#####[:Disable|Info|Hint|Warn|Error]'`); @@ -218,5 +217,15 @@ export class InfrastructureMessages { `${def(o.relativeFilename)} failed to validate.` )}); + static ERROR_OutFileMustBeAFolder = SevError | 0x002C; + static Error_OutFileMustBeAFolder = () => m( + this.ERROR_OutFileMustBeAFolder, + `Parameter --out-file must refer to a folder.`, ` + If multiple input files are specified, or if the parameter --out-file ends + with a slash (/) or backslash (\\), or if the folder referenced by the + parameter already exists, then kmc will treat the parameter as a folder. + However, as a file already exists with the same name, kmc cannot continue. + `); + } diff --git a/developer/src/kmc/src/util/baseOptions.ts b/developer/src/kmc/src/util/baseOptions.ts index f3223238ec..0b6636705c 100644 --- a/developer/src/kmc/src/util/baseOptions.ts +++ b/developer/src/kmc/src/util/baseOptions.ts @@ -1,3 +1,9 @@ +/* + * Keyman is copyright (C) SIL Global. MIT License. + * + * Basic options for kmc - shared across commands + */ +import * as path from 'node:path'; import { ALL_COMPILER_LOG_FORMATS, ALL_COMPILER_LOG_LEVELS, CompilerLogFormat, CompilerLogLevel } from "@keymanapp/developer-utils"; import { Command, Option } from "commander"; @@ -45,7 +51,7 @@ export class BaseOptions { export class BuildBaseOptions extends BaseOptions { public static addOutFile(program: Command) { - return program.option('-o, --out-file ', 'Override the default path and filename for the output file') + return program.option('-o, --out-file ', 'Override default folder and/or filename for the output file; terminate with '+path.delimiter+' for folder') } public static addAll(program: Command) { diff --git a/developer/src/kmc/src/util/sysexits.ts b/developer/src/kmc/src/util/sysexits.ts index 7256dd4e77..27a90f4b7c 100644 --- a/developer/src/kmc/src/util/sysexits.ts +++ b/developer/src/kmc/src/util/sysexits.ts @@ -7,6 +7,7 @@ import { KeymanSentry } from './KeymanSentry.js'; export const enum SysExits { EX_USAGE = 64, EX_DATAERR = 65, + EX_SOFTWARE = 70, }; export async function exitProcess(exitCode?: number): Promise { diff --git a/developer/src/kmc/test/build.tests.ts b/developer/src/kmc/test/build.tests.ts index b615c97f49..0abe1446f6 100644 --- a/developer/src/kmc/test/build.tests.ts +++ b/developer/src/kmc/test/build.tests.ts @@ -1,12 +1,14 @@ /* * Keyman is copyright (C) SIL Global. MIT License. */ -import { TestCompilerCallbacks } from '@keymanapp/developer-test-helpers'; -import { clearOptions } from '../src/util/options.js'; +import * as fs from 'node:fs'; import { assert } from 'chai'; import 'mocha'; -import { BuildProject } from '../src/commands/buildClasses/BuildProject.js'; +import { TestCompilerCallbacks } from '@keymanapp/developer-test-helpers'; import { makePathToFixture } from './helpers/index.js'; +import { clearOptions } from '../src/util/options.js'; +import { BuildProject } from '../src/commands/buildClasses/BuildProject.js'; +import { unitTestEndpoints } from '../src/commands/build.js'; interface CompilerWarningsAsErrorsTruthTable { cli: boolean; @@ -54,3 +56,99 @@ describe('compilerWarningsAsErrors', function () { }); } }); + +describe('interpretOutFile()', function() { + + function statSyncStub(path: fs.PathLike, options?: fs.StatSyncOptions): fs.Stats { + if(path === 'does_not_exist') return null; + return { + isFile: () => !path.toString().startsWith('existing_folder'), + isDirectory: () => path.toString().startsWith('existing_folder'), + isBlockDevice: () => false, + isCharacterDevice: () => false, + isSymbolicLink: () => false, + isFIFO: () => false, + isSocket: () => false, + dev: null, + ino: null, + mode: null, + nlink: null, + uid: null, + gid: null, + rdev: null, + size: null, + blksize: null, + blocks: null, + atimeMs: null, + mtimeMs: null, + ctimeMs: null, + birthtimeMs: null, + atime: null, + mtime: null, + ctime: null, + birthtime: null + } + } + + it(`should throw with invalid parameters`, function() { + assert.throws(() => unitTestEndpoints.interpretOutFile(null, 1, statSyncStub)); + assert.throws(() => unitTestEndpoints.interpretOutFile(undefined, null, statSyncStub)); + assert.throws(() => unitTestEndpoints.interpretOutFile(undefined, -1, statSyncStub)); + assert.throws(() => unitTestEndpoints.interpretOutFile(undefined, 0, statSyncStub)); + assert.doesNotThrow(() => unitTestEndpoints.interpretOutFile(undefined, 1, statSyncStub)); + assert.doesNotThrow(() => unitTestEndpoints.interpretOutFile(undefined, 2, statSyncStub)); + }); + + it(`should return undefined path if no path is specified`, function() { + const spec1 = unitTestEndpoints.interpretOutFile(undefined, 1, statSyncStub); + assert.isFalse(spec1.isFolder); + assert.isUndefined(spec1.path); + + const spec2 = unitTestEndpoints.interpretOutFile(undefined, 2, statSyncStub); + assert.isFalse(spec2.isFolder); + assert.isUndefined(spec2.path); + }); + + it(`should return isFolder=true if path is specified and more than 1 input filename is given`, function() { + const spec1 = unitTestEndpoints.interpretOutFile('does_not_exist', 2, statSyncStub); + assert.isNotNull(spec1); + assert.isTrue(spec1.isFolder); + assert.equal(spec1.path, 'does_not_exist'); + }); + + it(`should return path without trailing delimiter if trailing delimiter is specified`, function() { + const spec1 = unitTestEndpoints.interpretOutFile('does_not_exist/', 1, statSyncStub); + assert.isNotNull(spec1); + assert.isTrue(spec1.isFolder); + assert.equal(spec1.path, 'does_not_exist'); + + // existing folder test for trimming delimiter is covered in next test + }); + + it(`should return path as a folder if target path is a folder`, function() { + const spec1 = unitTestEndpoints.interpretOutFile('existing_folder', 1, statSyncStub); + assert.isNotNull(spec1); + assert.isTrue(spec1.isFolder); + assert.equal(spec1.path, 'existing_folder'); + + const spec2 = unitTestEndpoints.interpretOutFile('existing_folder/', 1, statSyncStub); + assert.isNotNull(spec2); + assert.isTrue(spec2.isFolder); + assert.equal(spec2.path, 'existing_folder'); + }); + + it(`should return null if a file exists in the target folder location`, function() { + const spec1 = unitTestEndpoints.interpretOutFile('existing_file', 2, statSyncStub); + assert.isNull(spec1); + + const spec2 = unitTestEndpoints.interpretOutFile('existing_file/', 1, statSyncStub); + assert.isNull(spec2); + }); + + it(`should return isFolder = false if a single input file and no trailing delimiter and no folder exists`, function() { + const spec1 = unitTestEndpoints.interpretOutFile('anything', 1, statSyncStub); + assert.isNotNull(spec1); + assert.isFalse(spec1.isFolder); + assert.equal(spec1.path, 'anything'); + }); +}); \ No newline at end of file diff --git a/developer/src/kmc/test/infrastructureMessages.tests.ts b/developer/src/kmc/test/infrastructureMessages.tests.ts index 68edaa7a10..d44cc8e972 100644 --- a/developer/src/kmc/test/infrastructureMessages.tests.ts +++ b/developer/src/kmc/test/infrastructureMessages.tests.ts @@ -1,3 +1,6 @@ +/* + * Keyman is copyright (C) SIL Global. MIT License. + */ import 'mocha'; import * as fs from 'fs'; import { assert } from 'chai'; @@ -66,7 +69,7 @@ describe('InfrastructureMessages', function () { const projectPath = makePathToFixture('kpj-2.0/khmer_angkor', 'khmer_angkor.kpj'); const outFilePath = makePathToFixture('kpj-2.0/khmer_angkor', 'khmer_angkor.kmx'); const options: CompilerOptions = {...defaultCompilerOptions}; - await unitTestEndpoints.build(projectPath, outFilePath, ncb, options); + await unitTestEndpoints.build(projectPath, {path: outFilePath, isFolder: false}, ncb, options); assert.isTrue(ncb.hasMessage(InfrastructureMessages.ERROR_OutFileNotValidForProjects), 'ERROR_OutFileNotValidForProjects not generated, instead got: '+JSON.stringify(ncb.messages,null,2)); });