From 161fb924b9b978f1f492000ed9b5d275fcfcc29a Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 19 Apr 2023 11:57:02 +0700 Subject: [PATCH 1/4] feat(developer): add CompilerMessages support to kmc-package Adds basic infrastructure for CompilerMessages events. Additional warnings, errors, and hints to come. --- developer/src/kmc-package/src/kmp-compiler.ts | 9 ++++++-- developer/src/kmc-package/src/messages.ts | 17 ++++++++++++++ .../kmc-package/test/test-package-compiler.ts | 9 ++++++-- .../kmc/src/commands/build/BuildPackage.ts | 23 +++++++++++-------- 4 files changed, 45 insertions(+), 13 deletions(-) create mode 100644 developer/src/kmc-package/src/messages.ts diff --git a/developer/src/kmc-package/src/kmp-compiler.ts b/developer/src/kmc-package/src/kmp-compiler.ts index 718403d18b..c28106b909 100644 --- a/developer/src/kmc-package/src/kmp-compiler.ts +++ b/developer/src/kmc-package/src/kmp-compiler.ts @@ -6,6 +6,8 @@ import KEYMAN_VERSION from "@keymanapp/keyman-version"; import type { KpsFile, KpsFileContentFile, KpsFileInfo, KpsFileKeyboard, KpsFileLanguage, KpsFileLexicalModel, KpsFileOptions, KpsPackage } from './kps-file.js'; import type { KmpJsonFile, KmpJsonFileInfo, KmpJsonFileLanguage, KmpJsonFileOptions } from './kmp-json-file.js'; +import { CompilerCallbacks } from 'common/web/types/build/src/main.js'; +import { CompilerMessages } from './messages.js'; export { type KmpJsonFile } from './kmp-json-file.js'; @@ -13,6 +15,9 @@ const FILEVERSION_KMP_JSON = '12.0'; export default class KmpCompiler { + constructor(private callbacks: CompilerCallbacks) { + } + public transformKpsToKmpObject(kpsString: string, kpsPath: string): KmpJsonFile { // Load the KPS data from XML as JS structured data. @@ -234,7 +239,7 @@ export default class KmpCompiler { data.files = []; } - data.files.forEach(function(value) { + data.files.forEach((value) => { // Get the path of the file let filename = value.name; @@ -246,7 +251,7 @@ export default class KmpCompiler { if(path.isAbsolute(value.name)) { // absolute paths are not very cross-platform compatible -- we are going to have trouble // with path separators and roots - // TODO: emit a warning + this.callbacks.reportMessage(CompilerMessages.Warn_AbsolutePath({filename: value.name})); } else { // Transform separators to platform separators -- kps files may use // either / or \, although older kps files were always \. diff --git a/developer/src/kmc-package/src/messages.ts b/developer/src/kmc-package/src/messages.ts new file mode 100644 index 0000000000..c7558527cc --- /dev/null +++ b/developer/src/kmc-package/src/messages.ts @@ -0,0 +1,17 @@ +import { CompilerErrorNamespace, CompilerErrorSeverity, CompilerMessageSpec as m } from "@keymanapp/common-types"; + +const Namespace = CompilerErrorNamespace.PackageCompiler; +// const SevInfo = CompilerErrorSeverity.Info | Namespace; +// const SevHint = CompilerErrorSeverity.Hint | Namespace; +const SevWarn = CompilerErrorSeverity.Warn | Namespace; +// const SevError = CompilerErrorSeverity.Error | Namespace; +const SevFatal = CompilerErrorSeverity.Fatal | Namespace; + +export class CompilerMessages { + static Fatal_UnexpectedException = (o:{e: any}) => m(this.FATAL_UnexpectedException, `Unexpected exception: ${(o.e ?? 'unknown error').toString()}\n\nCall stack:\n${(o.e instanceof Error ? o.e.stack : (new Error()).stack)}`); + static FATAL_UnexpectedException = SevFatal | 0x0001; + + static Warn_AbsolutePath = (o:{filename: string}) => m(this.WARN_AbsolutePath, `File ${o.filename} has an absolute path, which is not portable`); + static WARN_AbsolutePath = SevWarn | 0x0002; +} + diff --git a/developer/src/kmc-package/test/test-package-compiler.ts b/developer/src/kmc-package/test/test-package-compiler.ts index a2850e7fc2..86eac87ff8 100644 --- a/developer/src/kmc-package/test/test-package-compiler.ts +++ b/developer/src/kmc-package/test/test-package-compiler.ts @@ -7,6 +7,8 @@ import {makePathToFixture} from './helpers/index.js'; import JSZip from 'jszip'; import KEYMAN_VERSION from "@keymanapp/keyman-version"; import { type KmpJsonFile } from '../src/kmp-json-file.js'; +import { TestCompilerCallbacks } from '@keymanapp/developer-test-helpers'; + describe('KmpCompiler', function () { const MODELS : string[] = [ @@ -14,7 +16,8 @@ describe('KmpCompiler', function () { 'withfolders.qaa.sencoten', ]; - let kmpCompiler = new KmpCompiler(); + const callbacks = new TestCompilerCallbacks(); + let kmpCompiler = new KmpCompiler(callbacks); for (let modelID of MODELS) { const kpsPath = modelID.includes('withfolders') ? @@ -78,11 +81,13 @@ describe('KmpCompiler', function () { } it('should generates a valid .kmp (zip) file', async function() { + this.timeout(10000); // building a zip file can sometimes be slow + // const kmpPath = makePathToFixture('khmer_angkor', 'build', 'khmer_angkor.kmp'); const kpsPath = makePathToFixture('khmer_angkor', 'source', 'khmer_angkor.kps'); const kmpJsonRefPath = makePathToFixture('khmer_angkor', 'ref', 'kmp.json'); - const kmpCompiler = new KmpCompiler(); + const kmpCompiler = new KmpCompiler(callbacks); const source = fs.readFileSync(kpsPath, 'utf-8'); const kmpJsonFixture: KmpJsonFile = JSON.parse(fs.readFileSync(kmpJsonRefPath, 'utf-8')); diff --git a/developer/src/kmc/src/commands/build/BuildPackage.ts b/developer/src/kmc/src/commands/build/BuildPackage.ts index 13c74cdb5b..012112987f 100644 --- a/developer/src/kmc/src/commands/build/BuildPackage.ts +++ b/developer/src/kmc/src/commands/build/BuildPackage.ts @@ -1,6 +1,8 @@ import * as fs from 'fs'; import { BuildActivity, BuildActivityOptions } from './BuildActivity.js'; import KmpCompiler from '@keymanapp/kmc-package'; +import { CompilerCallbacks } from '@keymanapp/common-types'; +import { NodeCompilerCallbacks } from 'src/util/NodeCompilerCallbacks.js'; export class BuildPackage extends BuildActivity { public get name(): string { return 'Package'; } @@ -8,28 +10,31 @@ export class BuildPackage extends BuildActivity { public get compiledExtension(): string { return '.kmp'; } public get description(): string { return 'Build a Keyman package'; } public async build(infile: string, options: BuildActivityOptions): Promise { - let outfile = this.getOutputFilename(infile, options); + const c: CompilerCallbacks = new NodeCompilerCallbacks(); + + const outfile = this.getOutputFilename(infile, options); // // Load .kps source data // - let kpsString: string = fs.readFileSync(infile, 'utf8'); - let kmpCompiler = new KmpCompiler(); - let kmpJsonData = kmpCompiler.transformKpsToKmpObject(kpsString, infile); + const kpsString: string = fs.readFileSync(infile, 'utf8'); + const kmpCompiler = new KmpCompiler(c); + const kmpJsonData = kmpCompiler.transformKpsToKmpObject(kpsString, infile); + if(!kmpJsonData) { + return false; + } // // Build the .kmp package file // - let data = await kmpCompiler.buildKmpFile(infile, kmpJsonData); - if(data) { - fs.writeFileSync(outfile, data, 'binary'); - } else { - // TODO error logging + const data = await kmpCompiler.buildKmpFile(infile, kmpJsonData); + if(!data) { return false; } + fs.writeFileSync(outfile, data, 'binary'); return true; } } From 7644979deec0421eb2da85b86712eee0976e843c Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 19 Apr 2023 12:17:29 +0700 Subject: [PATCH 2/4] chore(developer): unit test for absolute path warning --- .../absolute_path/source/absolute_path.kps | 30 +++++++++++++++++++ .../kmc-package/test/test-package-compiler.ts | 27 +++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 developer/src/kmc-package/test/fixtures/absolute_path/source/absolute_path.kps diff --git a/developer/src/kmc-package/test/fixtures/absolute_path/source/absolute_path.kps b/developer/src/kmc-package/test/fixtures/absolute_path/source/absolute_path.kps new file mode 100644 index 0000000000..a1ebde691a --- /dev/null +++ b/developer/src/kmc-package/test/fixtures/absolute_path/source/absolute_path.kps @@ -0,0 +1,30 @@ + + + + 15.0.266.0 + 7.0 + + + + + + + + + + + + + + + Absolute Path + + + + \build\absolute_path.kmx + File absolute_path.kmx + 0 + .kmx + + + diff --git a/developer/src/kmc-package/test/test-package-compiler.ts b/developer/src/kmc-package/test/test-package-compiler.ts index 86eac87ff8..318675ea8c 100644 --- a/developer/src/kmc-package/test/test-package-compiler.ts +++ b/developer/src/kmc-package/test/test-package-compiler.ts @@ -8,6 +8,7 @@ import JSZip from 'jszip'; import KEYMAN_VERSION from "@keymanapp/keyman-version"; import { type KmpJsonFile } from '../src/kmp-json-file.js'; import { TestCompilerCallbacks } from '@keymanapp/developer-test-helpers'; +import { CompilerMessages } from '../src/messages.js'; describe('KmpCompiler', function () { @@ -122,4 +123,30 @@ describe('KmpCompiler', function () { assert.deepEqual(kmpJsonData, kmpJsonFixture); }); + it('should warn on absolute paths', async function() { + this.timeout(10000); // building a zip file can sometimes be slow + + callbacks.clear(); + + // const kmpPath = makePathToFixture('khmer_angkor', 'build', 'khmer_angkor.kmp'); + const kpsPath = makePathToFixture('absolute_path', 'source', 'absolute_path.kps'); + const kmpCompiler = new KmpCompiler(callbacks); + const source = fs.readFileSync(kpsPath, 'utf-8'); + + let kmpJson: KmpJsonFile = null; + + assert.doesNotThrow(() => { + kmpJson = kmpCompiler.transformKpsToKmpObject(source, kpsPath); + }); + + assert.doesNotThrow(async () => { + //assert.isNull( TODO: this should fail to build because of missing files. but that will come + await kmpCompiler.buildKmpFile(kpsPath, kmpJson) + //); + }); + + assert.lengthOf(callbacks.messages, 1); + assert.deepEqual(callbacks.messages[0].code, CompilerMessages.WARN_AbsolutePath); + }); + }); From 88601f2409681ba01a303ae7e7a76fc87918dd68 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 19 Apr 2023 12:44:52 +0700 Subject: [PATCH 3/4] feat(developer): add file-existence check to kmc-package --- developer/src/kmc-package/src/kmp-compiler.ts | 21 ++++++++++++++++++- developer/src/kmc-package/src/messages.ts | 8 ++++++- .../kmc-package/test/test-package-compiler.ts | 9 +++----- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/developer/src/kmc-package/src/kmp-compiler.ts b/developer/src/kmc-package/src/kmp-compiler.ts index c28106b909..91b52f0a2b 100644 --- a/developer/src/kmc-package/src/kmp-compiler.ts +++ b/developer/src/kmc-package/src/kmp-compiler.ts @@ -239,6 +239,7 @@ export default class KmpCompiler { data.files = []; } + let failed = false; data.files.forEach((value) => { // Get the path of the file let filename = value.name; @@ -263,13 +264,31 @@ export default class KmpCompiler { filename = path.resolve(basePath, filename); } const basename = path.basename(filename); - let data = fs.readFileSync(filename); + + if(!fs.existsSync(filename)) { + this.callbacks.reportMessage(CompilerMessages.Error_FileDoesNotExist({filename: filename})); + failed = true; + return; + } + + let data; + try { + data = fs.readFileSync(filename); + } catch(e) { + this.callbacks.reportMessage(CompilerMessages.Error_FileCouldNotBeRead({filename: filename, e: e})); + failed = true; + return; + } zip.file(basename, data); // Remove path data from files before JSON save value.name = basename; }); + if(failed) { + return null; + } + zip.file(kmpJsonFileName, JSON.stringify(data, null, 2)); // Generate kmp file diff --git a/developer/src/kmc-package/src/messages.ts b/developer/src/kmc-package/src/messages.ts index c7558527cc..34db1d175f 100644 --- a/developer/src/kmc-package/src/messages.ts +++ b/developer/src/kmc-package/src/messages.ts @@ -4,7 +4,7 @@ const Namespace = CompilerErrorNamespace.PackageCompiler; // const SevInfo = CompilerErrorSeverity.Info | Namespace; // const SevHint = CompilerErrorSeverity.Hint | Namespace; const SevWarn = CompilerErrorSeverity.Warn | Namespace; -// const SevError = CompilerErrorSeverity.Error | Namespace; +const SevError = CompilerErrorSeverity.Error | Namespace; const SevFatal = CompilerErrorSeverity.Fatal | Namespace; export class CompilerMessages { @@ -13,5 +13,11 @@ export class CompilerMessages { static Warn_AbsolutePath = (o:{filename: string}) => m(this.WARN_AbsolutePath, `File ${o.filename} has an absolute path, which is not portable`); static WARN_AbsolutePath = SevWarn | 0x0002; + + static Error_FileDoesNotExist = (o:{filename: string}) => m(this.ERROR_FileDoesNotExist, `File ${o.filename} does not exist.`); + static ERROR_FileDoesNotExist = SevError | 0x0003; + + static Error_FileCouldNotBeRead = (o:{filename: string; e: any}) => m(this.ERROR_FileCouldNotBeRead, `File ${o.filename} could not be read: ${(o.e ?? 'unknown error').toString()}.`); + static ERROR_FileCouldNotBeRead = SevError | 0x0004; } diff --git a/developer/src/kmc-package/test/test-package-compiler.ts b/developer/src/kmc-package/test/test-package-compiler.ts index 318675ea8c..97a193fbbc 100644 --- a/developer/src/kmc-package/test/test-package-compiler.ts +++ b/developer/src/kmc-package/test/test-package-compiler.ts @@ -139,14 +139,11 @@ describe('KmpCompiler', function () { kmpJson = kmpCompiler.transformKpsToKmpObject(source, kpsPath); }); - assert.doesNotThrow(async () => { - //assert.isNull( TODO: this should fail to build because of missing files. but that will come - await kmpCompiler.buildKmpFile(kpsPath, kmpJson) - //); - }); + await assert.isNull(kmpCompiler.buildKmpFile(kpsPath, kmpJson)); - assert.lengthOf(callbacks.messages, 1); + assert.lengthOf(callbacks.messages, 2); assert.deepEqual(callbacks.messages[0].code, CompilerMessages.WARN_AbsolutePath); + assert.deepEqual(callbacks.messages[1].code, CompilerMessages.ERROR_FileDoesNotExist); //TODO: this should be a file-missing-error }); }); From e89568c2d300b00216f4b0b69190e8ecf7bf688b Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 19 Apr 2023 15:31:28 +0700 Subject: [PATCH 4/4] fix(developer): KmpCompiler call in kmlmp, kmlmi --- developer/src/kmc/src/kmlmi.ts | 4 +++- developer/src/kmc/src/kmlmp.ts | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/developer/src/kmc/src/kmlmi.ts b/developer/src/kmc/src/kmlmi.ts index b3549f4271..2f3963f469 100644 --- a/developer/src/kmc/src/kmlmi.ts +++ b/developer/src/kmc/src/kmlmi.ts @@ -10,6 +10,7 @@ import KmpCompiler from '@keymanapp/kmc-package'; import { ModelInfoOptions as ModelInfoOptions, writeMergedModelMetadataFile } from '@keymanapp/kmc-model-info'; import { SysExits } from './util/sysexits.js'; import KEYMAN_VERSION from "@keymanapp/keyman-version"; +import { NodeCompilerCallbacks } from './util/NodeCompilerCallbacks.js'; let inputFilename: string; const program = new Command(); @@ -45,8 +46,9 @@ let jsFilename = program.opts().jsFilename ? program.opts().jsFilename : path.jo // Load .kps source data // +const callbacks = new NodeCompilerCallbacks(); let kpsString: string = fs.readFileSync(kpsFilename, 'utf8'); -let kmpCompiler = new KmpCompiler(); +let kmpCompiler = new KmpCompiler(callbacks); let kmpJsonData = kmpCompiler.transformKpsToKmpObject(kpsString, kpsFilename); // diff --git a/developer/src/kmc/src/kmlmp.ts b/developer/src/kmc/src/kmlmp.ts index 6d51b4f58b..6e66d00bbc 100644 --- a/developer/src/kmc/src/kmlmp.ts +++ b/developer/src/kmc/src/kmlmp.ts @@ -8,6 +8,7 @@ import { Command } from 'commander'; import KmpCompiler from '@keymanapp/kmc-package'; import { SysExits } from './util/sysexits.js'; import KEYMAN_VERSION from "@keymanapp/keyman-version"; +import { NodeCompilerCallbacks } from './util/NodeCompilerCallbacks.js'; let inputFilename: string; const program = new Command(); @@ -34,8 +35,9 @@ let outputFilename: string = program.opts().outFile ? program.opts().outFile : i // Load .kps source data // +const callbacks = new NodeCompilerCallbacks(); let kpsString: string = fs.readFileSync(inputFilename, 'utf8'); -let kmpCompiler = new KmpCompiler(); +let kmpCompiler = new KmpCompiler(callbacks); let kmpJsonData = kmpCompiler.transformKpsToKmpObject(kpsString, inputFilename); //