diff --git a/src/envVarSync.ts b/src/envVarSync.ts new file mode 100644 index 00000000..6514afda --- /dev/null +++ b/src/envVarSync.ts @@ -0,0 +1,95 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. + +import * as vscode from "vscode"; + +/** + * Helpers that update a {@link vscode.EnvironmentVariableCollection} only when + * the resulting mutation would actually change the collection. + * + * Background: VS Code shows a "Restart terminal to apply environment variable + * changes" prompt whenever an extension's collection differs from what has + * already been applied to running terminals. Calling `replace()` / `append()` + * with the same value, or calling `clear()` followed by re-adding identical + * entries, still counts as a change and re-triggers the prompt on every + * window reload. See issue #1647. + * + * These helpers compare the existing mutator (type + value + options) against + * the desired one and skip the write entirely when they match. + */ + +const DEFAULT_OPTIONS: Required = { + applyAtProcessCreation: true, + applyAtShellIntegration: false, +}; + +function normalizeOptions( + options?: vscode.EnvironmentVariableMutatorOptions, +): Required { + return { ...DEFAULT_OPTIONS, ...options }; +} + +function sameOptions( + existing: vscode.EnvironmentVariableMutator, + desired?: vscode.EnvironmentVariableMutatorOptions, +): boolean { + const e = normalizeOptions(existing.options); + const d = normalizeOptions(desired); + return e.applyAtProcessCreation === d.applyAtProcessCreation + && e.applyAtShellIntegration === d.applyAtShellIntegration; +} + +/** + * Calls `collection.replace(variable, value, options)` only when the existing + * mutator (if any) does not already match. + * + * @returns `true` if the collection was actually written to. + */ +export function applyReplaceIfChanged( + collection: vscode.EnvironmentVariableCollection, + variable: string, + value: string, + options?: vscode.EnvironmentVariableMutatorOptions, +): boolean { + const existing = collection.get(variable); + if (existing + && existing.type === vscode.EnvironmentVariableMutatorType.Replace + && existing.value === value + && sameOptions(existing, options)) { + return false; + } + if (options) { + collection.replace(variable, value, options); + } else { + collection.replace(variable, value); + } + return true; +} + +/** + * Calls `collection.append(variable, value, options)` only when the existing + * mutator (if any) does not already match. + * + * @returns `true` if the collection was actually written to. + */ +export function applyAppendIfChanged( + collection: vscode.EnvironmentVariableCollection, + variable: string, + value: string, + options?: vscode.EnvironmentVariableMutatorOptions, +): boolean { + const existing = collection.get(variable); + if (existing + && existing.type === vscode.EnvironmentVariableMutatorType.Append + && existing.value === value + && sameOptions(existing, options)) { + return false; + } + if (options) { + collection.append(variable, value, options); + } else { + collection.append(variable, value); + } + return true; +} + diff --git a/src/noConfigDebugInit.ts b/src/noConfigDebugInit.ts index 2f84ee6b..ea4e3606 100644 --- a/src/noConfigDebugInit.ts +++ b/src/noConfigDebugInit.ts @@ -9,6 +9,9 @@ import * as vscode from 'vscode'; import { sendInfo, sendError } from "vscode-extension-telemetry-wrapper"; import { getJavaHome } from "./utility"; import { buildNoConfigPathAppendValue } from "./pathUtil"; +import { applyAppendIfChanged, applyReplaceIfChanged } from "./envVarSync"; + +const ENV_VAR_COLLECTION_DESCRIPTION = "Java No-Config Debug"; /** * Registers the configuration-less debugging setup for the extension. @@ -21,7 +24,7 @@ import { buildNoConfigPathAppendValue } from "./pathUtil"; * * Environment Variables: * - `VSCODE_JDWP_ADAPTER_ENDPOINTS`: Path to the file containing the debugger adapter endpoint. - * - `JAVA_TOOL_OPTIONS`: JDWP configuration for automatic debugging. + * - `VSCODE_JAVA_EXEC`: Path to the java executable from the Java Language Server (when available). * - `PATH`: Appends the path to the noConfigScripts directory. */ export async function registerNoConfigDebug( @@ -69,22 +72,31 @@ export async function registerNoConfigDebug( } } - // clear the env var collection to remove any existing env vars - collection.clear(); + // Surface a description in VS Code's environment variable UI so users can + // see which extension is contributing these variables. + if (collection.description !== ENV_VAR_COLLECTION_DESCRIPTION) { + collection.description = ENV_VAR_COLLECTION_DESCRIPTION; + } - // Add env var for VSCODE_JDWP_ADAPTER_ENDPOINTS + // Apply our managed variables using diff-aware helpers. On a typical + // window reload the values are unchanged and these calls are no-ops, so + // VS Code does not prompt the user to restart their existing terminals. + // See issue #1647. + // // Note: We do NOT set JAVA_TOOL_OPTIONS globally to avoid affecting all Java processes // (javac, maven, gradle, language server, etc.). Instead, JAVA_TOOL_OPTIONS is set // only in the debugjava wrapper scripts (debugjava.ps1, debugjava.bat, debugjava) - collection.replace('VSCODE_JDWP_ADAPTER_ENDPOINTS', tempFilePath); + applyReplaceIfChanged(collection, 'VSCODE_JDWP_ADAPTER_ENDPOINTS', tempFilePath); // Try to get Java executable from Java Language Server - // This ensures we use the same Java version as the project is compiled with + // This ensures we use the same Java version as the project is compiled with. + // If detection fails or returns nothing, we deliberately keep any previously + // set VSCODE_JAVA_EXEC to avoid churn from transient startup failures. try { const javaHome = await getJavaHome(); if (javaHome) { const javaExec = path.join(javaHome, 'bin', 'java'); - collection.replace('VSCODE_JAVA_EXEC', javaExec); + applyReplaceIfChanged(collection, 'VSCODE_JAVA_EXEC', javaExec); } } catch (error) { // If we can't get Java from Language Server, that's okay @@ -92,7 +104,7 @@ export async function registerNoConfigDebug( } const noConfigScriptsDir = path.join(extPath, 'bundled', 'scripts', 'noConfigScripts'); - collection.append('PATH', buildNoConfigPathAppendValue(noConfigScriptsDir)); + applyAppendIfChanged(collection, 'PATH', buildNoConfigPathAppendValue(noConfigScriptsDir)); // create file system watcher for the debuggerAdapterEndpointFolder for when the communication port is written const fileSystemWatcher = vscode.workspace.createFileSystemWatcher( diff --git a/test/envVarSync.test.ts b/test/envVarSync.test.ts new file mode 100644 index 00000000..f549b92b --- /dev/null +++ b/test/envVarSync.test.ts @@ -0,0 +1,213 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. + +import * as assert from "assert"; +import * as vscode from "vscode"; + +import { + applyAppendIfChanged, + applyReplaceIfChanged, +} from "../src/envVarSync"; + +interface FakeMutator { + type: vscode.EnvironmentVariableMutatorType; + value: string; + options: vscode.EnvironmentVariableMutatorOptions; +} + +interface FakeCollection extends vscode.EnvironmentVariableCollection { + __calls: { replace: number; append: number; delete: number }; +} + +function createFakeCollection(): FakeCollection { + const store = new Map(); + const calls = { replace: 0, append: 0, delete: 0 }; + + const collection = { + persistent: true, + description: undefined as string | vscode.MarkdownString | undefined, + get(name: string): FakeMutator | undefined { + return store.get(name); + }, + replace(name: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions): void { + calls.replace += 1; + store.set(name, { + type: vscode.EnvironmentVariableMutatorType.Replace, + value, + options: { applyAtProcessCreation: true, applyAtShellIntegration: false, ...options }, + }); + }, + append(name: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions): void { + calls.append += 1; + store.set(name, { + type: vscode.EnvironmentVariableMutatorType.Append, + value, + options: { applyAtProcessCreation: true, applyAtShellIntegration: false, ...options }, + }); + }, + prepend(name: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions): void { + store.set(name, { + type: vscode.EnvironmentVariableMutatorType.Prepend, + value, + options: { applyAtProcessCreation: true, applyAtShellIntegration: false, ...options }, + }); + }, + delete(name: string): void { + calls.delete += 1; + store.delete(name); + }, + clear(): void { + store.clear(); + }, + forEach(callback: (variable: string, mutator: FakeMutator, collection: any) => void): void { + store.forEach((mutator, variable) => callback(variable, mutator, collection)); + }, + getScoped(): vscode.EnvironmentVariableCollection { + return collection as unknown as vscode.EnvironmentVariableCollection; + }, + *[Symbol.iterator](): IterableIterator<[string, FakeMutator]> { + yield* store.entries(); + }, + __calls: calls, + }; + + return collection as unknown as FakeCollection; +} + +suite("envVarSync", () => { + suite("applyReplaceIfChanged", () => { + test("writes when variable is missing", () => { + const c = createFakeCollection(); + const changed = applyReplaceIfChanged(c, "FOO", "bar"); + assert.strictEqual(changed, true); + assert.strictEqual(c.__calls.replace, 1); + assert.strictEqual(c.get("FOO")!.value, "bar"); + assert.strictEqual(c.get("FOO")!.type, vscode.EnvironmentVariableMutatorType.Replace); + }); + + test("is a no-op when value and type already match (default options)", () => { + const c = createFakeCollection(); + applyReplaceIfChanged(c, "FOO", "bar"); + c.__calls.replace = 0; + + const changed = applyReplaceIfChanged(c, "FOO", "bar"); + + assert.strictEqual(changed, false); + assert.strictEqual(c.__calls.replace, 0); + }); + + test("writes when value differs", () => { + const c = createFakeCollection(); + applyReplaceIfChanged(c, "FOO", "bar"); + c.__calls.replace = 0; + + const changed = applyReplaceIfChanged(c, "FOO", "baz"); + + assert.strictEqual(changed, true); + assert.strictEqual(c.__calls.replace, 1); + assert.strictEqual(c.get("FOO")!.value, "baz"); + }); + + test("overrides an existing Append mutator", () => { + const c = createFakeCollection(); + applyAppendIfChanged(c, "FOO", "bar"); + c.__calls.replace = 0; + + const changed = applyReplaceIfChanged(c, "FOO", "bar"); + + assert.strictEqual(changed, true); + assert.strictEqual(c.__calls.replace, 1); + assert.strictEqual(c.get("FOO")!.type, vscode.EnvironmentVariableMutatorType.Replace); + }); + + test("writes when options differ from existing mutator", () => { + const c = createFakeCollection(); + applyReplaceIfChanged(c, "FOO", "bar", { applyAtProcessCreation: true, applyAtShellIntegration: false }); + c.__calls.replace = 0; + + const changed = applyReplaceIfChanged(c, "FOO", "bar", { applyAtProcessCreation: false, applyAtShellIntegration: false }); + + assert.strictEqual(changed, true); + assert.strictEqual(c.__calls.replace, 1); + }); + + test("treats omitted options as the documented defaults", () => { + const c = createFakeCollection(); + applyReplaceIfChanged(c, "FOO", "bar", { applyAtProcessCreation: true, applyAtShellIntegration: false }); + c.__calls.replace = 0; + + const changed = applyReplaceIfChanged(c, "FOO", "bar"); + + assert.strictEqual(changed, false); + assert.strictEqual(c.__calls.replace, 0); + }); + }); + + suite("applyAppendIfChanged", () => { + test("writes when variable is missing", () => { + const c = createFakeCollection(); + const changed = applyAppendIfChanged(c, "PATH", ";C:\\extra"); + assert.strictEqual(changed, true); + assert.strictEqual(c.__calls.append, 1); + assert.strictEqual(c.get("PATH")!.value, ";C:\\extra"); + assert.strictEqual(c.get("PATH")!.type, vscode.EnvironmentVariableMutatorType.Append); + }); + + test("is a no-op when value already matches", () => { + const c = createFakeCollection(); + applyAppendIfChanged(c, "PATH", ";C:\\extra"); + c.__calls.append = 0; + + const changed = applyAppendIfChanged(c, "PATH", ";C:\\extra"); + + assert.strictEqual(changed, false); + assert.strictEqual(c.__calls.append, 0); + }); + + test("writes when value differs", () => { + const c = createFakeCollection(); + applyAppendIfChanged(c, "PATH", ";C:\\extra"); + c.__calls.append = 0; + + const changed = applyAppendIfChanged(c, "PATH", ";C:\\other"); + + assert.strictEqual(changed, true); + assert.strictEqual(c.__calls.append, 1); + assert.strictEqual(c.get("PATH")!.value, ";C:\\other"); + }); + + test("overrides an existing Replace mutator on the same variable", () => { + const c = createFakeCollection(); + applyReplaceIfChanged(c, "PATH", ";C:\\extra"); + c.__calls.append = 0; + + const changed = applyAppendIfChanged(c, "PATH", ";C:\\extra"); + + assert.strictEqual(changed, true); + assert.strictEqual(c.__calls.append, 1); + assert.strictEqual(c.get("PATH")!.type, vscode.EnvironmentVariableMutatorType.Append); + }); + }); + + suite("regression: issue #1647 - repeated activations stay quiet", () => { + test("re-applying the same set of variables does not touch the collection", () => { + const c = createFakeCollection(); + + // Simulate first activation. + applyReplaceIfChanged(c, "VSCODE_JDWP_ADAPTER_ENDPOINTS", "/tmp/endpoint-abc.txt"); + applyReplaceIfChanged(c, "VSCODE_JAVA_EXEC", "/opt/jdk/bin/java"); + applyAppendIfChanged(c, "PATH", ":/ext/bundled/scripts/noConfigScripts"); + + const callsAfterFirst = { ...c.__calls }; + assert.deepStrictEqual(callsAfterFirst, { replace: 2, append: 1, delete: 0 }); + + // Simulate a window reload: same values, same order. + applyReplaceIfChanged(c, "VSCODE_JDWP_ADAPTER_ENDPOINTS", "/tmp/endpoint-abc.txt"); + applyReplaceIfChanged(c, "VSCODE_JAVA_EXEC", "/opt/jdk/bin/java"); + applyAppendIfChanged(c, "PATH", ":/ext/bundled/scripts/noConfigScripts"); + + // Nothing should have been written on the second pass. + assert.deepStrictEqual(c.__calls, callsAfterFirst); + }); + }); +});