Install dbconnect in a child process (#1216)

This avoids problems with command escaping when runnin in a shell.

Author: ilia-db

packages/databricks-vscode/src/language/EnvironmentDependenciesInstaller.ts

@@ -1,4 +1,4 @@
-import {EventEmitter, window} from "vscode";
+import {EventEmitter, OutputChannel, window} from "vscode";
 
 import {Disposable} from "vscode";
 import {MsPythonExtensionWrapper} from "./MsPythonExtensionWrapper";
@@ -9,29 +9,46 @@ export class EnvironmentDependenciesInstaller implements Disposable {
     private onDidInstallAttemptEmitter = new EventEmitter<void>();
     public onDidTryInstallation = this.onDidInstallAttemptEmitter.event;
 
+    private _outputChannel?: OutputChannel;
+
     constructor(private readonly pythonExtension: MsPythonExtensionWrapper) {}
 
+    get outputChannel() {
+        if (!this._outputChannel) {
+            this._outputChannel =
+                window.createOutputChannel("Databricks Connect");
+            this.disposables.push(this._outputChannel);
+        }
+        return this._outputChannel;
+    }
+
     dispose() {
         this.disposables.forEach((i) => i.dispose());
     }
 
     async install(version?: string) {
         version = version ?? DATABRICKS_CONNECT_VERSION;
         try {
+            this.outputChannel.clear();
+            this.outputChannel.show();
             await this.pythonExtension.uninstallPackageFromEnvironment(
-                "pyspark"
+                "pyspark",
+                this.outputChannel
             );
             await this.pythonExtension.uninstallPackageFromEnvironment(
-                "databricks-connect"
+                "databricks-connect",
+                this.outputChannel
             );
             await this.pythonExtension.installPackageInEnvironment(
                 "databricks-connect",
-                version
+                version,
+                this.outputChannel
             );
         } catch (e: unknown) {
             if (e instanceof Error) {
                 window.showErrorMessage(e.message);
             }
+            this.outputChannel.show();
         }
         this.onDidInstallAttemptEmitter.fire();
     }

packages/databricks-vscode/src/language/MsPythonExtensionWrapper.ts

@@ -4,16 +4,12 @@ import {
     Event,
     window,
     Disposable,
-    workspace,
-    RelativePattern,
     Terminal,
     commands,
+    OutputChannel,
 } from "vscode";
 import {StateStorage} from "../vscode-objs/StateStorage";
 import {IExtensionApi as MsPythonExtensionApi} from "./MsPythonExtensionApi";
-import * as os from "node:os";
-import * as path from "node:path";
-import {mkdtemp, readFile} from "fs/promises";
 import {Mutex} from "../locking";
 import * as childProcess from "node:child_process";
 import {promisify} from "node:util";
@@ -88,40 +84,24 @@ export class MsPythonExtensionWrapper implements Disposable {
         );
     }
 
-    private async executeInTerminalE(command: string) {
-        const dir = await mkdtemp(path.join(os.tmpdir(), "databricks-vscode-"));
-        const filePath = path.join(dir, "python-terminal-output.txt");
-
-        const disposables: Disposable[] = [];
-        const exitCodePromise = new Promise<number | undefined>((resolve) => {
-            const fsWatcher = workspace.createFileSystemWatcher(
-                new RelativePattern(dir, path.basename(filePath))
-            );
-            const handleFileChange = async () => {
-                try {
-                    const fileData = await readFile(filePath, "utf-8");
-                    resolve(parseInt(fileData));
-                } catch (e: unknown) {
-                    resolve(undefined);
+    async runWithOutput(
+        command: string,
+        args: string[],
+        outputChannel?: OutputChannel
+    ) {
+        const cp = childProcess.execFile(command, args);
+        cp.stdout?.on("data", (data) => outputChannel?.append(data));
+        cp.stderr?.on("data", (data) => outputChannel?.append(data));
+        return new Promise<void>((resolve, reject) => {
+            cp.on("exit", (code) => {
+                if (code === 0) {
+                    resolve();
+                } else {
+                    reject(new Error(`Command exited with code ${code}`));
                 }
-            };
-            disposables.push(
-                fsWatcher,
-                fsWatcher.onDidCreate(handleFileChange),
-                fsWatcher.onDidChange(handleFileChange)
-            );
+            });
+            cp.on("error", reject);
         });
-
-        try {
-            await this.terminalMutex.wait();
-            this.terminal.show();
-            this.terminal.sendText(`${command}; echo $? > ${filePath}`);
-            const exitCode = await exitCodePromise;
-            return exitCode;
-        } finally {
-            disposables.forEach((i) => i.dispose());
-            this.terminalMutex.signal();
-        }
     }
 
     async getLatestPackageVersion(name: string) {
@@ -190,41 +170,50 @@ export class MsPythonExtensionWrapper implements Disposable {
         );
     }
 
-    async installPackageInEnvironment(name: string, version?: string | RegExp) {
+    async installPackageInEnvironment(
+        name: string,
+        version?: string | RegExp,
+        outputChannel?: OutputChannel
+    ) {
         const executable = await this.getPythonExecutable();
         if (!executable) {
             throw Error("No python executable found");
         }
         if (version === "latest") {
             version = await this.getLatestPackageVersion(name);
         }
-        const execCommand = `'${executable}' -m pip install '${name}${
-            version ? `==${version}` : ""
-        }' --disable-pip-version-check --no-python-version-warning`;
-
-        const exitCode = await this.executeInTerminalE(execCommand);
-        if (exitCode) {
-            throw new Error(
-                `Error while installing ${name} package in the current python environment.`
-            );
-        }
+        const args = [
+            "-m",
+            "pip",
+            "install",
+            `${name}${version ? `==${version}` : ""}`,
+            "--disable-pip-version-check",
+            "--no-python-version-warning",
+        ];
+        outputChannel?.appendLine(`Running: ${executable} ${args.join(" ")}`);
+        await this.runWithOutput(executable, args, outputChannel);
     }
 
-    async uninstallPackageFromEnvironment(name: string) {
+    async uninstallPackageFromEnvironment(
+        name: string,
+        outputChannel?: OutputChannel
+    ) {
         const exists = await this.findPackageInEnvironment(name);
         const executable = await this.getPythonExecutable();
-
         if (!exists || !executable) {
             return;
         }
-
-        const execCommand = `'${executable}' -m pip uninstall '${name}' --disable-pip-version-check --no-python-version-warning -y`;
-        const exitCode = await this.executeInTerminalE(execCommand);
-        if (exitCode) {
-            throw new Error(
-                `Error while un-installing ${name} package from the current python environment.`
-            );
-        }
+        const args = [
+            "-m",
+            "pip",
+            "uninstall",
+            name,
+            "--disable-pip-version-check",
+            "--no-python-version-warning",
+            "-y",
+        ];
+        outputChannel?.appendLine(`Running: ${executable} ${args.join(" ")}`);
+        await this.runWithOutput(executable, args, outputChannel);
     }
 
     async selectPythonInterpreter() {