Merge pull request #95 from bcmi-labs/pro-ide-142

Align language server spawning with arduino-cli

Author: pirropirro

.gitignore

@@ -2,15 +2,16 @@ node_modules/
 # .node_modules is a hack for the electron builder.
 .node_modules/
 lib/
-build/
 downloads/
+build/
 !electron/build/
 src-gen/
 browser-app/webpack.config.js
 electron-app/webpack.config.js
-.DS_Store
 /workspace/static
+.DS_Store
 # switching from `electron` to `browser` in dev mode.
 .browser_modules
 # LS logs
 inols*.log
+yarn-error.log

arduino-ide-extension/package.json

@@ -22,8 +22,8 @@
     "@theia/search-in-workspace": "next",
     "@theia/terminal": "next",
     "@theia/workspace": "next",
-    "@types/google-protobuf": "^3.7.1",
     "@types/dateformat": "^3.0.1",
+    "@types/google-protobuf": "^3.7.1",
     "@types/ps-tree": "^1.1.0",
     "@types/react-select": "^3.0.0",
     "@types/which": "^1.3.1",
@@ -47,19 +47,37 @@
     "generate-protocol": "node ./scripts/generate-protocol.js",
     "lint": "tslint -c ./tslint.json --project ./tsconfig.json",
     "build": "tsc && ncp ./src/node/cli-protocol/ ./lib/node/cli-protocol/ && yarn lint",
-    "watch": "tsc -w"
+    "watch": "tsc -w",
+    "test": "mocha \"./test/**/*.test.ts\""
+  },
+  "mocha": {
+    "require": [
+      "ts-node/register",
+      "reflect-metadata/Reflect"
+    ],
+    "reporter": "spec",
+    "colors": true,
+    "watch-extensions": "ts,tsx",
+    "timeout": 10000
   },
   "devDependencies": {
+    "@types/chai": "^4.2.7",
+    "@types/chai-string": "^1.4.2",
+    "@types/mocha": "^5.2.7",
+    "chai": "^4.2.0",
+    "chai-string": "^1.5.0",
     "decompress": "^4.2.0",
     "decompress-targz": "^4.1.1",
     "decompress-unzip": "^4.0.1",
     "download": "^7.1.0",
     "grpc-tools": "^1.8.0",
     "grpc_tools_node_protoc_ts": "^2.5.8",
+    "mocha": "^7.0.0",
     "moment": "^2.24.0",
     "ncp": "^2.0.0",
     "rimraf": "^2.6.1",
     "shelljs": "^0.8.3",
+    "ts-node": "^8.6.2",
     "tslint": "^5.5.0",
     "typescript": "3.5.3",
     "uuid": "^3.2.1",

arduino-ide-extension/src/node/arduino-cli.ts

@@ -1,12 +1,8 @@
-import * as os from 'os';
-import * as which from 'which';
-import * as semver from 'semver';
-import { spawn } from 'child_process';
-import { join } from 'path';
 import { injectable, inject } from 'inversify';
 import { ILogger } from '@theia/core';
 import { FileUri } from '@theia/core/lib/node/file-uri';
 import { Config } from '../common/protocol/config-service';
+import { spawnCommand, getExecPath } from './exec-util';
 
 @injectable()
 export class ArduinoCli {
@@ -20,33 +16,19 @@ export class ArduinoCli {
         if (this.execPath) {
             return this.execPath;
         }
-        const version = /\d+\.\d+\.\d+/;
-        const cli = `arduino-cli${os.platform() === 'win32' ? '.exe' : ''}`;
-        const buildCli = join(__dirname, '..', '..', 'build', cli);
-        const buildVersion = await this.spawn(`"${buildCli}"`, ['version']);
-        const buildShortVersion = (buildVersion.match(version) || [])[0];
-        this.execPath = buildCli;
-        const pathCli = await new Promise<string | undefined>(resolve => which(cli, (error, path) => resolve(error ? undefined : path)));
-        if (!pathCli) {
-            return buildCli;
-        }
-        const pathVersion = await this.spawn(`"${pathCli}"`, ['version']);
-        const pathShortVersion = (pathVersion.match(version) || [])[0];
-        if (semver.gt(pathShortVersion, buildShortVersion)) {
-            this.execPath = pathCli;
-            return pathCli;
-        }
-        return buildCli;
+        const path = await getExecPath('arduino-cli', this.logger, 'version');
+        this.execPath = path;
+        return path;
     }
 
     async getVersion(): Promise<string> {
         const execPath = await this.getExecPath();
-        return this.spawn(`"${execPath}"`, ['version']);
+        return spawnCommand(`"${execPath}"`, ['version'], this.logger);
     }
 
     async getDefaultConfig(): Promise<Config> {
         const execPath = await this.getExecPath();
-        const result = await this.spawn(`"${execPath}"`, ['config', 'dump', '--format', 'json']);
+        const result = await spawnCommand(`"${execPath}"`, ['config', 'dump', '--format', 'json'], this.logger);
         const { directories } = JSON.parse(result);
         if (!directories) {
             throw new Error(`Could not parse config. 'directories' was missing from: ${result}`);
@@ -64,33 +46,4 @@ export class ArduinoCli {
         };
     }
 
-    private spawn(command: string, args?: string[]): Promise<string> {
-        return new Promise<string>((resolve, reject) => {
-            const buffers: Buffer[] = [];
-            const cp = spawn(command, args, { windowsHide: true, shell: true });
-            cp.stdout.on('data', (b: Buffer) => buffers.push(b));
-            cp.on('error', error => {
-                this.logger.error(`Error executing ${command} with args: ${JSON.stringify(args)}.`, error);
-                reject(error);
-            });
-            cp.on('exit', (code, signal) => {
-                if (code === 0) {
-                    const result = Buffer.concat(buffers).toString('utf8').trim()
-                    resolve(result);
-                    return;
-                }
-                if (signal) {
-                    this.logger.error(`Unexpected signal '${signal}' when executing ${command} with args: ${JSON.stringify(args)}.`);
-                    reject(new Error(`Process exited with signal: ${signal}`));
-                    return;
-                }
-                if (code) {
-                    this.logger.error(`Unexpected exit code '${code}' when executing ${command} with args: ${JSON.stringify(args)}.`);
-                    reject(new Error(`Process exited with exit code: ${code}`));
-                    return;
-                }
-            });
-        });
-    }
-
 }

arduino-ide-extension/src/node/exec-util.ts

@@ -0,0 +1,60 @@
+import * as os from 'os';
+import * as which from 'which';
+import * as semver from 'semver';
+import { spawn } from 'child_process';
+import { join } from 'path';
+import { ILogger } from '@theia/core';
+
+export async function getExecPath(commandName: string, logger: ILogger, versionArg?: string, inBinDir?: boolean): Promise<string> {
+    const execName = `${commandName}${os.platform() === 'win32' ? '.exe' : ''}`;
+    const relativePath = ['..', '..', 'build'];
+    if (inBinDir) {
+        relativePath.push('bin');
+    }
+    const buildCommand = join(__dirname, ...relativePath, execName);
+    if (!versionArg) {
+        return buildCommand;
+    }
+    const versionRegexp = /\d+\.\d+\.\d+/;
+    const buildVersion = await spawnCommand(`"${buildCommand}"`, [versionArg], logger);
+    const buildShortVersion = (buildVersion.match(versionRegexp) || [])[0];
+    const pathCommand = await new Promise<string | undefined>(resolve => which(execName, (error, path) => resolve(error ? undefined : path)));
+    if (!pathCommand) {
+        return buildCommand;
+    }
+    const pathVersion = await spawnCommand(`"${pathCommand}"`, [versionArg], logger);
+    const pathShortVersion = (pathVersion.match(versionRegexp) || [])[0];
+    if (semver.gt(pathShortVersion, buildShortVersion)) {
+        return pathCommand;
+    }
+    return buildCommand;
+}
+
+export function spawnCommand(command: string, args: string[], logger: ILogger): Promise<string> {
+    return new Promise<string>((resolve, reject) => {
+        const cp = spawn(command, args, { windowsHide: true, shell: true });
+        const buffers: Buffer[] = [];
+        cp.stdout.on('data', (b: Buffer) => buffers.push(b));
+        cp.on('error', error => {
+            logger.error(`Error executing ${command} with args: ${JSON.stringify(args)}.`, error);
+            reject(error);
+        });
+        cp.on('exit', (code, signal) => {
+            if (code === 0) {
+                const result = Buffer.concat(buffers).toString('utf8').trim()
+                resolve(result);
+                return;
+            }
+            if (signal) {
+                logger.error(`Unexpected signal '${signal}' when executing ${command} with args: ${JSON.stringify(args)}.`);
+                reject(new Error(`Process exited with signal: ${signal}`));
+                return;
+            }
+            if (code) {
+                logger.error(`Unexpected exit code '${code}' when executing ${command} with args: ${JSON.stringify(args)}.`);
+                reject(new Error(`Process exited with exit code: ${code}`));
+                return;
+            }
+        });
+    });
+}

arduino-ide-extension/src/node/language/arduino-language-server-contribution.ts

@@ -1,9 +1,9 @@
-import * as which from 'which';
 import * as os from 'os';
-import { join, delimiter } from 'path';
-import { injectable } from 'inversify';
+import { injectable, inject } from 'inversify';
+import { ILogger } from '@theia/core';
 import { BaseLanguageServerContribution, IConnection, LanguageServerStartOptions } from '@theia/languages/lib/node';
 import { Board } from '../../common/protocol/boards-service';
+import { getExecPath } from '../exec-util';
 
 @injectable()
 export class ArduinoLanguageServerContribution extends BaseLanguageServerContribution {
@@ -23,10 +23,13 @@ export class ArduinoLanguageServerContribution extends BaseLanguageServerContrib
         return this.description.name;
     }
 
+    @inject(ILogger)
+    protected logger: ILogger;
+
     async start(clientConnection: IConnection, options: LanguageServerStartOptions): Promise<void> {
-        const languageServer = await this.resolveExecutable('arduino-language-server');
-        const clangd = await this.resolveExecutable('clangd');
-        const cli = await this.resolveExecutable('arduino-cli');
+        const languageServer = await getExecPath('arduino-language-server', this.logger);
+        const clangd = await getExecPath('clangd', this.logger, '--version', os.platform() !== 'win32');
+        const cli = await getExecPath('arduino-cli', this.logger, 'version');
         // Add '-log' argument to enable logging to files
         const args: string[] = ['-clangd', clangd, '-cli', cli];
         if (options.parameters && options.parameters.selectedBoard) {
@@ -45,21 +48,4 @@ export class ArduinoLanguageServerContribution extends BaseLanguageServerContrib
         serverConnection.onClose(() => (clientConnection as any).reader.socket.close());
     }
 
-    protected resolveExecutable(name: string): Promise<string> {
-        return new Promise<string>((resolve, reject) => {
-            const segments = ['..', '..', '..', 'build'];
-            if (name === 'clangd' && os.platform() !== 'win32') {
-                segments.push('bin');
-            }
-            const path = `${process.env.PATH}${delimiter}${join(__dirname, ...segments)}`;
-            const suffix = os.platform() === 'win32' ? '.exe' : '';
-            which(name + suffix, { path }, (err, execPath) => {
-                if (err) {
-                    reject(err);
-                } else {
-                    resolve(execPath);
-                }
-            });
-        });
-    }
 }

arduino-ide-extension/test/node/exec-util.test.ts

@@ -0,0 +1,32 @@
+import * as os from 'os';
+import { expect, use } from 'chai';
+import { NullLogger } from './logger';
+import { getExecPath } from '../../lib/node/exec-util'
+
+use(require('chai-string'));
+
+describe('getExecPath', () => {
+    it('should resolve arduino-cli', async () => {
+        const path = await getExecPath('arduino-cli', new NullLogger(), 'version');
+        if (os.platform() === 'win32')
+            expect(path).to.endsWith('\\arduino-cli.exe');
+        else
+            expect(path).to.endsWith('/arduino-cli');
+    });
+
+    it('should resolve arduino-language-server', async () => {
+        const path = await getExecPath('arduino-language-server', new NullLogger());
+        if (os.platform() === 'win32')
+            expect(path).to.endsWith('\\arduino-language-server.exe');
+        else
+            expect(path).to.endsWith('/arduino-language-server');
+    });
+
+    it('should resolve clangd', async () => {
+        const path = await getExecPath('clangd', new NullLogger(), '--version', os.platform() !== 'win32');
+        if (os.platform() === 'win32')
+            expect(path).to.endsWith('\\clangd.exe');
+        else
+            expect(path).to.endsWith('/clangd');
+    });
+});

arduino-ide-extension/test/node/logger.ts

@@ -0,0 +1,89 @@
+import { ILogger, Loggable, LogLevel } from '@theia/core';
+
+export class NullLogger implements ILogger {
+    logLevel = 0;
+
+    setLogLevel(logLevel: number): Promise<void> {
+        this.logLevel = logLevel;
+        return Promise.resolve();
+    }
+    getLogLevel(): Promise<number> {
+        return Promise.resolve(this.logLevel);
+    }
+    isEnabled(logLevel: number): Promise<boolean> {
+        return Promise.resolve(logLevel >= this.logLevel);
+    }
+    ifEnabled(logLevel: number): Promise<void> {
+        if (logLevel >= this.logLevel)
+            return Promise.resolve();
+        else
+            return Promise.reject();
+    }
+    log(logLevel: any, loggable: any, ...rest: any[]) {
+        return Promise.resolve();
+    }
+
+    isTrace(): Promise<boolean> {
+        return this.isEnabled(LogLevel.TRACE);
+    }
+    ifTrace(): Promise<void> {
+        return this.ifEnabled(LogLevel.TRACE);
+    }
+    trace(arg: any | Loggable, ...params: any[]): Promise<void> {
+        return this.log(LogLevel.TRACE, arg, ...params);
+    }
+
+    isDebug(): Promise<boolean> {
+        return this.isEnabled(LogLevel.DEBUG);
+    }
+    ifDebug(): Promise<void> {
+        return this.ifEnabled(LogLevel.DEBUG);
+    }
+    debug(arg: any | Loggable, ...params: any[]): Promise<void> {
+        return this.log(LogLevel.DEBUG, arg, ...params);
+    }
+
+    isInfo(): Promise<boolean> {
+        return this.isEnabled(LogLevel.INFO);
+    }
+    ifInfo(): Promise<void> {
+        return this.ifEnabled(LogLevel.INFO);
+    }
+    info(arg: any | Loggable, ...params: any[]): Promise<void> {
+        return this.log(LogLevel.INFO, arg, ...params);
+    }
+
+    isWarn(): Promise<boolean> {
+        return this.isEnabled(LogLevel.WARN);
+    }
+    ifWarn(): Promise<void> {
+        return this.ifEnabled(LogLevel.WARN);
+    }
+    warn(arg: any | Loggable, ...params: any[]): Promise<void> {
+        return this.log(LogLevel.WARN, arg, ...params);
+    }
+
+    isError(): Promise<boolean> {
+        return this.isEnabled(LogLevel.ERROR);
+    }
+    ifError(): Promise<void> {
+        return this.ifEnabled(LogLevel.ERROR);
+    }
+    error(arg: any | Loggable, ...params: any[]): Promise<void> {
+        return this.log(LogLevel.ERROR, arg, ...params);
+    }
+
+    isFatal(): Promise<boolean> {
+        return this.isEnabled(LogLevel.FATAL);
+    }
+    ifFatal(): Promise<void> {
+        return this.ifEnabled(LogLevel.FATAL);
+    }
+    fatal(arg: any | Loggable, ...params: any[]): Promise<void> {
+        return this.log(LogLevel.FATAL, arg, ...params);
+    }
+
+    child(name: string): ILogger {
+        return this;
+    }
+}

azure-pipelines.yml

@@ -34,6 +34,8 @@ jobs:
       GITHUB_TOKEN: $(Personal.GitHub.Token)
       THEIA_ELECTRON_SKIP_REPLACE_FFMPEG: 1
     displayName: Build
+  - script: yarn test
+    displayName: Test
   - bash: |
       ./electron/packager/conf-node-gyp.sh
       yarn --cwd ./electron/packager/