Change scope of jvmargs configuration

[fvclaus]

Signed-off-by: Frederik Claus [email protected]

I just started working with a Windows machine and used Settings Sync to sync my settings from my Linux machine. On Linux, I used the Lombok extension. The following java.jdt.ls.vmargs was synced:

"java.jdt.ls.vmargs": "-XX:+UseParallelGC -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90 -Dsun.zip.disableMemoryMapping=true -Xmx1G -Xms100m -javaagent:\"/home/dev/.vscode/extensions/gabrielbb.vscode-lombok-1.0.1/server/lombok.jar\"",

On windows, this crashed the language server with

{
  message: 'Error opening zip file or JAR manifest missing : c:\\Users\\COMPUTERFFCL\\.vscode\\extensions\\gabrielbb.vscode-lombok-1.0.1\\server\\lombok.jar\n',
  level: 'info',
  timestamp: '2022-03-21 13:28:11.004'
}

After installing the Lombok extension on the Windows machine. java.jdt.ls.vmargs was overriden and synced as

"-XX:+UseParallelGC -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90 -Dsun.zip.disableMemoryMapping=true -Xmx1G -Xms100m -javaagent:\"c:\\Users\\COMPUTERFFCL\\.vscode\\extensions\\gabrielbb.vscode-lombok-1.0.1\\server\\lombok.jar\"",

I believe, the java.jdt.ls.vmargs should not be synced across machines. Besides the path issues with -javaagent, the configuration might be tailored to a specific hardware configuration.

AFAIK, this will have only one consequence: If settings are synced across machines, the JVM args on the newly synced machine will be empty. Should the default value, currently specified only in package.json, also be used in javaServerStarter.ts?

vscode-java/src/javaServerStarter.ts

Lines 83 to 102 in 7ed0798

	let vmargsCheck = workspace.getConfiguration().inspect('java.jdt.ls.vmargs').workspaceValue;
	if (vmargsCheck !== undefined) {
	const isWorkspaceTrusted = (workspace as any).isTrusted; // keep compatibility for old engines < 1.56.0
	const agentFlag = getJavaagentFlag(vmargsCheck);
	if (agentFlag !== null && (isWorkspaceTrusted === undefined || !isWorkspaceTrusted)) {
	const keyVmargs = getKey(IS_WORKSPACE_VMARGS_ALLOWED, context.storagePath, vmargsCheck);
	const key = context.globalState.get(keyVmargs);
	if (key !== true && (workspace.workspaceFolders && isInWorkspaceFolder(agentFlag, workspace.workspaceFolders))) {
	vmargsCheck = workspace.getConfiguration().inspect('java.jdt.ls.vmargs').globalValue;
	}
	}
	} else {
	vmargsCheck = getJavaConfiguration().get('jdt.ls.vmargs');
	}
	let vmargs;
	if (vmargsCheck !== undefined) {
	vmargs = vmargsCheck + '';
	} else {
	vmargs = '';
	}

In the meantime, this can be used as the workaround:

    "settingsSync.ignoredSettings": [
        "java.jdt.ls.vmargs"
    ]

[testforstephen]

I also see some side-effect for this. when I use VS Code remote development extension to open a folder in docker container, I observed my Windows jvmargs is shared to Linux container automatically, that would break Linux container if my jvmargs contains lombok agent path. I agree a path related setting should be limited to machine scope.