Skip to content

Commit 57ae166

Browse files
vinistockvinistock
committed
Remove JSON requirement from activation script
1 parent 0ef695a commit 57ae166

File tree

13 files changed

+173
-180
lines changed

13 files changed

+173
-180
lines changed

‎project-words

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ dont
2626
eglot
2727
Eglot
2828
eruby
29+
EUTF
2930
EXTGLOB
3031
fakehome
3132
FIXEDENCODING

‎vscode/activation.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
1-
env = { env: ENV.to_h, yjit: !!defined?(RubyVM::YJIT), version: RUBY_VERSION, gemPath: Gem.path }.to_json
2-
STDERR.print("RUBY_LSP_ACTIVATION_SEPARATOR#{env}RUBY_LSP_ACTIVATION_SEPARATOR")
1+
env = ENV.map { |k, v| "#{k}RUBY_LSP_VS#{v}" }
2+
env.unshift(RUBY_VERSION, Gem.path.join(","), !!defined?(RubyVM::YJIT))
3+
STDERR.print("RUBY_LSP_ACTIVATION_SEPARATOR#{env.join("RUBY_LSP_FS")}RUBY_LSP_ACTIVATION_SEPARATOR")

‎vscode/src/debugger.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,6 @@ export class Debugger
258258

259259
this.logDebuggerMessage(`Spawning debugger in directory ${cwd}`);
260260
this.logDebuggerMessage(` Command bundle ${args.join(" ")}`);
261-
this.logDebuggerMessage(
262-
` Environment ${JSON.stringify(configuration.env)}`,
263-
);
264261

265262
this.debugProcess = spawn("bundle", args, {
266263
shell: true,

‎vscode/src/ruby/chruby.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ export class Chruby extends VersionManager {
474474
].join(";");
475475

476476
const result = await this.runScript(
477-
`${rubyExecutableUri.fsPath} -W0 -e '${script}'`,
477+
`${rubyExecutableUri.fsPath} -EUTF-8:UTF-8 -e '${script}'`,
478478
);
479479

480480
const [defaultGems, gemHome, yjit, version] =

‎vscode/src/ruby/versionManager.ts

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ export interface ActivationResult {
1414
gemPath: string[];
1515
}
1616

17+
// Changes to either one of these values have to be synchronized with a corresponding update in `activation.rb`
1718
export const ACTIVATION_SEPARATOR = "RUBY_LSP_ACTIVATION_SEPARATOR";
19+
export const VALUE_SEPARATOR = "RUBY_LSP_VS";
20+
export const FIELD_SEPARATOR = "RUBY_LSP_FS";
1821

1922
export abstract class VersionManager {
2023
protected readonly outputChannel: WorkspaceChannel;
@@ -56,32 +59,33 @@ export abstract class VersionManager {
5659
// language server
5760
abstract activate(): Promise<ActivationResult>;
5861

59-
protected async runEnvActivationScript(activatedRuby: string) {
62+
protected async runEnvActivationScript(
63+
activatedRuby: string,
64+
): Promise<ActivationResult> {
6065
const activationUri = vscode.Uri.joinPath(
6166
this.context.extensionUri,
6267
"activation.rb",
6368
);
69+
6470
const result = await this.runScript(
65-
`${activatedRuby} -W0 -rjson '${activationUri.fsPath}'`,
71+
`${activatedRuby} -EUTF-8:UTF-8 '${activationUri.fsPath}'`,
6672
);
6773

6874
const activationContent = new RegExp(
69-
`${ACTIVATION_SEPARATOR}(.*)${ACTIVATION_SEPARATOR}`,
75+
`${ACTIVATION_SEPARATOR}([^]*)${ACTIVATION_SEPARATOR}`,
7076
).exec(result.stderr);
7177

72-
return this.parseWithErrorHandling(activationContent![1]);
73-
}
74-
75-
protected parseWithErrorHandling(json: string) {
76-
try {
77-
return JSON.parse(json);
78-
} catch (error: any) {
79-
this.outputChannel.error(
80-
`Tried parsing invalid JSON environment: ${json}`,
81-
);
82-
83-
throw error;
84-
}
78+
const [version, gemPath, yjit, ...envEntries] =
79+
activationContent![1].split(FIELD_SEPARATOR);
80+
81+
return {
82+
version,
83+
gemPath: gemPath.split(","),
84+
yjit: yjit === "true",
85+
env: Object.fromEntries(
86+
envEntries.map((entry) => entry.split(VALUE_SEPARATOR)),
87+
),
88+
};
8589
}
8690

8791
// Runs the given command in the directory for the Bundle, using the user's preferred shell and inheriting the current
@@ -99,14 +103,17 @@ export abstract class VersionManager {
99103
this.outputChannel.info(
100104
`Running command: \`${command}\` in ${this.bundleUri.fsPath} using shell: ${shell}`,
101105
);
102-
this.outputChannel.debug(
103-
`Environment used for command: ${JSON.stringify(process.env)}`,
104-
);
106+
107+
// Ensure that LANG and LC_ALL are set to UTF-8. If the user has it set to `C`, then the encoding used for
108+
// environment variables may be ASCII-8BIT. If at the same time the user has a shell prompt (PS1) that contains
109+
// certain characters, the encoding conversion will fail and the activation script will not work.
110+
const env = { ...process.env, LANG: "C.UTF-8", LC_ALL: "C.UTF-8" };
105111

106112
return asyncExec(command, {
107113
cwd: this.bundleUri.fsPath,
108114
shell,
109-
env: process.env,
115+
env,
116+
encoding: "utf-8",
110117
});
111118
}
112119

‎vscode/src/test/suite/debugger.test.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,16 +192,14 @@ suite("Debugger", () => {
192192
'source "https://rubygems.org"\ngem "debug"',
193193
);
194194

195-
const extensionPath = path.dirname(
196-
path.dirname(path.dirname(path.dirname(__dirname))),
197-
);
195+
const extensionPath = path.dirname(path.dirname(path.dirname(__dirname)));
198196
const context = {
199197
subscriptions: [],
200198
workspaceState: {
201199
get: () => undefined,
202200
update: () => undefined,
203201
},
204-
extensionUri: vscode.Uri.file(path.join(extensionPath, "vscode")),
202+
extensionUri: vscode.Uri.file(extensionPath),
205203
} as unknown as vscode.ExtensionContext;
206204
const outputChannel = new WorkspaceChannel("fake", LOG_CHANNEL);
207205
const workspaceFolder: vscode.WorkspaceFolder = {

‎vscode/src/test/suite/ruby.test.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ import { Ruby, ManagerIdentifier } from "../../ruby";
1010
import { WorkspaceChannel } from "../../workspaceChannel";
1111
import { LOG_CHANNEL } from "../../common";
1212
import * as common from "../../common";
13-
import { ACTIVATION_SEPARATOR } from "../../ruby/versionManager";
13+
import {
14+
ACTIVATION_SEPARATOR,
15+
FIELD_SEPARATOR,
16+
VALUE_SEPARATOR,
17+
} from "../../ruby/versionManager";
1418

1519
import { FAKE_TELEMETRY } from "./fakeTelemetry";
1620

@@ -125,16 +129,16 @@ suite("Ruby environment activation", () => {
125129
},
126130
} as unknown as vscode.WorkspaceConfiguration);
127131

128-
const envStub = {
129-
env: { ANY: "true" },
130-
yjit: true,
131-
version: "3.3.5",
132-
gemPath: ["~/.gem/ruby/3.3.5", "/opt/rubies/3.3.5/lib/ruby/gems/3.3.0"],
133-
};
132+
const envStub = [
133+
"3.3.5",
134+
"~/.gem/ruby/3.3.5,/opt/rubies/3.3.5/lib/ruby/gems/3.3.0",
135+
"true",
136+
`ANY${VALUE_SEPARATOR}true`,
137+
].join(FIELD_SEPARATOR);
134138

135139
const execStub = sinon.stub(common, "asyncExec").resolves({
136140
stdout: "",
137-
stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`,
141+
stderr: `${ACTIVATION_SEPARATOR}${envStub}${ACTIVATION_SEPARATOR}`,
138142
});
139143

140144
const ruby = new Ruby(

‎vscode/src/test/suite/ruby/asdf.test.ts

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ import sinon from "sinon";
88
import { Asdf } from "../../../ruby/asdf";
99
import { WorkspaceChannel } from "../../../workspaceChannel";
1010
import * as common from "../../../common";
11-
import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager";
11+
import {
12+
ACTIVATION_SEPARATOR,
13+
FIELD_SEPARATOR,
14+
VALUE_SEPARATOR,
15+
} from "../../../ruby/versionManager";
1216

1317
suite("Asdf", () => {
1418
if (os.platform() === "win32") {
@@ -41,15 +45,16 @@ suite("Asdf", () => {
4145
context,
4246
async () => {},
4347
);
44-
const envStub = {
45-
env: { ANY: "true" },
46-
yjit: true,
47-
version: "3.0.0",
48-
};
48+
const envStub = [
49+
"3.0.0",
50+
"/path/to/gems",
51+
"true",
52+
`ANY${VALUE_SEPARATOR}true`,
53+
].join(FIELD_SEPARATOR);
4954

5055
const execStub = sinon.stub(common, "asyncExec").resolves({
5156
stdout: "",
52-
stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`,
57+
stderr: `${ACTIVATION_SEPARATOR}${envStub}${ACTIVATION_SEPARATOR}`,
5358
});
5459

5560
const findInstallationStub = sinon
@@ -61,12 +66,13 @@ suite("Asdf", () => {
6166

6267
assert.ok(
6368
execStub.calledOnceWithExactly(
64-
`. ${os.homedir()}/.asdf/asdf.sh && asdf exec ruby -W0 -rjson '/fake/activation.rb'`,
69+
`. ${os.homedir()}/.asdf/asdf.sh && asdf exec ruby -EUTF-8:UTF-8 '/fake/activation.rb'`,
6570
{
6671
cwd: workspacePath,
6772
shell: "/bin/bash",
6873
// eslint-disable-next-line no-process-env
69-
env: process.env,
74+
env: { ...process.env, LANG: "C.UTF-8", LC_ALL: "C.UTF-8" },
75+
encoding: "utf-8",
7076
},
7177
),
7278
);
@@ -95,15 +101,16 @@ suite("Asdf", () => {
95101
context,
96102
async () => {},
97103
);
98-
const envStub = {
99-
env: { ANY: "true" },
100-
yjit: true,
101-
version: "3.0.0",
102-
};
103104

105+
const envStub = [
106+
"3.0.0",
107+
"/path/to/gems",
108+
"true",
109+
`ANY${VALUE_SEPARATOR}true`,
110+
].join(FIELD_SEPARATOR);
104111
const execStub = sinon.stub(common, "asyncExec").resolves({
105112
stdout: "",
106-
stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`,
113+
stderr: `${ACTIVATION_SEPARATOR}${envStub}${ACTIVATION_SEPARATOR}`,
107114
});
108115

109116
const findInstallationStub = sinon
@@ -117,12 +124,13 @@ suite("Asdf", () => {
117124

118125
assert.ok(
119126
execStub.calledOnceWithExactly(
120-
`. ${os.homedir()}/.asdf/asdf.fish && asdf exec ruby -W0 -rjson '/fake/activation.rb'`,
127+
`. ${os.homedir()}/.asdf/asdf.fish && asdf exec ruby -EUTF-8:UTF-8 '/fake/activation.rb'`,
121128
{
122129
cwd: workspacePath,
123130
shell: "/opt/homebrew/bin/fish",
124131
// eslint-disable-next-line no-process-env
125-
env: process.env,
132+
env: { ...process.env, LANG: "C.UTF-8", LC_ALL: "C.UTF-8" },
133+
encoding: "utf-8",
126134
},
127135
),
128136
);

‎vscode/src/test/suite/ruby/custom.test.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@ import sinon from "sinon";
99
import { Custom } from "../../../ruby/custom";
1010
import { WorkspaceChannel } from "../../../workspaceChannel";
1111
import * as common from "../../../common";
12-
import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager";
12+
import {
13+
ACTIVATION_SEPARATOR,
14+
FIELD_SEPARATOR,
15+
VALUE_SEPARATOR,
16+
} from "../../../ruby/versionManager";
1317

1418
suite("Custom", () => {
1519
const context = {
@@ -40,15 +44,16 @@ suite("Custom", () => {
4044
async () => {},
4145
);
4246

43-
const envStub = {
44-
env: { ANY: "true" },
45-
yjit: true,
46-
version: "3.0.0",
47-
};
47+
const envStub = [
48+
"3.0.0",
49+
"/path/to/gems",
50+
"true",
51+
`ANY${VALUE_SEPARATOR}true`,
52+
].join(FIELD_SEPARATOR);
4853

4954
const execStub = sinon.stub(common, "asyncExec").resolves({
5055
stdout: "",
51-
stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`,
56+
stderr: `${ACTIVATION_SEPARATOR}${envStub}${ACTIVATION_SEPARATOR}`,
5257
});
5358

5459
const commandStub = sinon
@@ -65,16 +70,13 @@ suite("Custom", () => {
6570

6671
assert.ok(
6772
execStub.calledOnceWithExactly(
68-
<<<<<<< Updated upstream
69-
`my_version_manager activate_env && ruby -W0 -rjson '/fake/activation.rb'`,
70-
=======
71-
`my_version_manager activate_env && ruby '${activationUri.fsPath}'`,
72-
>>>>>>> Stashed changes
73+
`my_version_manager activate_env && ruby -EUTF-8:UTF-8 '${activationUri.fsPath}'`,
7374
{
7475
cwd: uri.fsPath,
7576
shell,
7677
// eslint-disable-next-line no-process-env
77-
env: process.env,
78+
env: { ...process.env, LANG: "C.UTF-8", LC_ALL: "C.UTF-8" },
79+
encoding: "utf-8",
7880
},
7981
),
8082
);

0 commit comments

Comments
 (0)