Skip to content

Commit 3f3c787

Browse files
committed
Remove JSON requirement from activation script
1 parent 0ef695a commit 3f3c787

File tree

13 files changed

+166
-177
lines changed

13 files changed

+166
-177
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_VALUE_SEPARATOR!#{v}" }
2+
env.unshift(RUBY_VERSION, Gem.path.join(","), !!defined?(RubyVM::YJIT))
3+
STDERR.print("RUBY_LSP_ACTIVATION_SEPARATOR#{env.join("!RUBY_LSP_VALUE_SEPARATOR!")}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 & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ 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_VALUE_SEPARATOR!";
1820

1921
export abstract class VersionManager {
2022
protected readonly outputChannel: WorkspaceChannel;
@@ -56,32 +58,37 @@ export abstract class VersionManager {
5658
// language server
5759
abstract activate(): Promise<ActivationResult>;
5860

59-
protected async runEnvActivationScript(activatedRuby: string) {
61+
protected async runEnvActivationScript(
62+
activatedRuby: string,
63+
): Promise<ActivationResult> {
6064
const activationUri = vscode.Uri.joinPath(
6165
this.context.extensionUri,
6266
"activation.rb",
6367
);
68+
6469
const result = await this.runScript(
65-
`${activatedRuby} -W0 -rjson '${activationUri.fsPath}'`,
70+
`${activatedRuby} -EUTF-8:UTF-8 '${activationUri.fsPath}'`,
6671
);
6772

6873
const activationContent = new RegExp(
69-
`${ACTIVATION_SEPARATOR}(.*)${ACTIVATION_SEPARATOR}`,
74+
`${ACTIVATION_SEPARATOR}([^]*)${ACTIVATION_SEPARATOR}`,
7075
).exec(result.stderr);
7176

72-
return this.parseWithErrorHandling(activationContent![1]);
73-
}
77+
const [version, gemPath, yjit, ...envEntries] =
78+
activationContent![1].split(VALUE_SEPARATOR);
7479

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-
);
80+
const env: Record<string, string> = {};
8281

83-
throw error;
82+
for (let i = 0; i < envEntries.length; i += 2) {
83+
env[envEntries[i]] = envEntries[i + 1];
8484
}
85+
86+
return {
87+
version,
88+
gemPath: gemPath.split(","),
89+
yjit: yjit === "true",
90+
env,
91+
};
8592
}
8693

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

106115
return asyncExec(command, {
107116
cwd: this.bundleUri.fsPath,
108117
shell,
109-
env: process.env,
118+
env,
119+
encoding: "utf-8",
110120
});
111121
}
112122

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: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ 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+
VALUE_SEPARATOR,
16+
} from "../../ruby/versionManager";
1417

1518
import { FAKE_TELEMETRY } from "./fakeTelemetry";
1619

@@ -125,16 +128,16 @@ suite("Ruby environment activation", () => {
125128
},
126129
} as unknown as vscode.WorkspaceConfiguration);
127130

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-
};
131+
const envStub = [
132+
"3.3.5",
133+
"~/.gem/ruby/3.3.5,/opt/rubies/3.3.5/lib/ruby/gems/3.3.0",
134+
"true",
135+
`ANY${VALUE_SEPARATOR}true`,
136+
].join(VALUE_SEPARATOR);
134137

135138
const execStub = sinon.stub(common, "asyncExec").resolves({
136139
stdout: "",
137-
stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`,
140+
stderr: `${ACTIVATION_SEPARATOR}${envStub}${ACTIVATION_SEPARATOR}`,
138141
});
139142

140143
const ruby = new Ruby(

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

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ 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+
VALUE_SEPARATOR,
14+
} from "../../../ruby/versionManager";
1215

1316
suite("Asdf", () => {
1417
if (os.platform() === "win32") {
@@ -41,15 +44,16 @@ suite("Asdf", () => {
4144
context,
4245
async () => {},
4346
);
44-
const envStub = {
45-
env: { ANY: "true" },
46-
yjit: true,
47-
version: "3.0.0",
48-
};
47+
const envStub = [
48+
"3.0.0",
49+
"/path/to/gems",
50+
"true",
51+
`ANY${VALUE_SEPARATOR}true`,
52+
].join(VALUE_SEPARATOR);
4953

5054
const execStub = sinon.stub(common, "asyncExec").resolves({
5155
stdout: "",
52-
stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`,
56+
stderr: `${ACTIVATION_SEPARATOR}${envStub}${ACTIVATION_SEPARATOR}`,
5357
});
5458

5559
const findInstallationStub = sinon
@@ -61,12 +65,13 @@ suite("Asdf", () => {
6165

6266
assert.ok(
6367
execStub.calledOnceWithExactly(
64-
`. ${os.homedir()}/.asdf/asdf.sh && asdf exec ruby -W0 -rjson '/fake/activation.rb'`,
68+
`. ${os.homedir()}/.asdf/asdf.sh && asdf exec ruby -EUTF-8:UTF-8 '/fake/activation.rb'`,
6569
{
6670
cwd: workspacePath,
6771
shell: "/bin/bash",
6872
// eslint-disable-next-line no-process-env
69-
env: process.env,
73+
env: { ...process.env, LANG: "C.UTF-8", LC_ALL: "C.UTF-8" },
74+
encoding: "utf-8",
7075
},
7176
),
7277
);
@@ -95,15 +100,16 @@ suite("Asdf", () => {
95100
context,
96101
async () => {},
97102
);
98-
const envStub = {
99-
env: { ANY: "true" },
100-
yjit: true,
101-
version: "3.0.0",
102-
};
103103

104+
const envStub = [
105+
"3.0.0",
106+
"/path/to/gems",
107+
"true",
108+
`ANY${VALUE_SEPARATOR}true`,
109+
].join(VALUE_SEPARATOR);
104110
const execStub = sinon.stub(common, "asyncExec").resolves({
105111
stdout: "",
106-
stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`,
112+
stderr: `${ACTIVATION_SEPARATOR}${envStub}${ACTIVATION_SEPARATOR}`,
107113
});
108114

109115
const findInstallationStub = sinon
@@ -117,12 +123,13 @@ suite("Asdf", () => {
117123

118124
assert.ok(
119125
execStub.calledOnceWithExactly(
120-
`. ${os.homedir()}/.asdf/asdf.fish && asdf exec ruby -W0 -rjson '/fake/activation.rb'`,
126+
`. ${os.homedir()}/.asdf/asdf.fish && asdf exec ruby -EUTF-8:UTF-8 '/fake/activation.rb'`,
121127
{
122128
cwd: workspacePath,
123129
shell: "/opt/homebrew/bin/fish",
124130
// eslint-disable-next-line no-process-env
125-
env: process.env,
131+
env: { ...process.env, LANG: "C.UTF-8", LC_ALL: "C.UTF-8" },
132+
encoding: "utf-8",
126133
},
127134
),
128135
);

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

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ 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+
VALUE_SEPARATOR,
15+
} from "../../../ruby/versionManager";
1316

1417
suite("Custom", () => {
1518
const context = {
@@ -40,15 +43,16 @@ suite("Custom", () => {
4043
async () => {},
4144
);
4245

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

4953
const execStub = sinon.stub(common, "asyncExec").resolves({
5054
stdout: "",
51-
stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`,
55+
stderr: `${ACTIVATION_SEPARATOR}${envStub}${ACTIVATION_SEPARATOR}`,
5256
});
5357

5458
const commandStub = sinon
@@ -65,16 +69,13 @@ suite("Custom", () => {
6569

6670
assert.ok(
6771
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
72+
`my_version_manager activate_env && ruby -EUTF-8:UTF-8 '${activationUri.fsPath}'`,
7373
{
7474
cwd: uri.fsPath,
7575
shell,
7676
// eslint-disable-next-line no-process-env
77-
env: process.env,
77+
env: { ...process.env, LANG: "C.UTF-8", LC_ALL: "C.UTF-8" },
78+
encoding: "utf-8",
7879
},
7980
),
8081
);

0 commit comments

Comments
 (0)