Skip to content

Commit

Permalink
Remove JSON requirement from activation script
Browse files Browse the repository at this point in the history
  • Loading branch information
vinistock committed Jan 22, 2025
1 parent 0ef695a commit 57ae166
Show file tree
Hide file tree
Showing 13 changed files with 173 additions and 180 deletions.
1 change: 1 addition & 0 deletions project-words
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ dont
eglot
Eglot
eruby
EUTF
EXTGLOB
fakehome
FIXEDENCODING
Expand Down
5 changes: 3 additions & 2 deletions vscode/activation.rb
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
env = { env: ENV.to_h, yjit: !!defined?(RubyVM::YJIT), version: RUBY_VERSION, gemPath: Gem.path }.to_json
STDERR.print("RUBY_LSP_ACTIVATION_SEPARATOR#{env}RUBY_LSP_ACTIVATION_SEPARATOR")
env = ENV.map { |k, v| "#{k}RUBY_LSP_VS#{v}" }
env.unshift(RUBY_VERSION, Gem.path.join(","), !!defined?(RubyVM::YJIT))
STDERR.print("RUBY_LSP_ACTIVATION_SEPARATOR#{env.join("RUBY_LSP_FS")}RUBY_LSP_ACTIVATION_SEPARATOR")
3 changes: 0 additions & 3 deletions vscode/src/debugger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,6 @@ export class Debugger

this.logDebuggerMessage(`Spawning debugger in directory ${cwd}`);
this.logDebuggerMessage(` Command bundle ${args.join(" ")}`);
this.logDebuggerMessage(
` Environment ${JSON.stringify(configuration.env)}`,
);

this.debugProcess = spawn("bundle", args, {
shell: true,
Expand Down
2 changes: 1 addition & 1 deletion vscode/src/ruby/chruby.ts
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ export class Chruby extends VersionManager {
].join(";");

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

const [defaultGems, gemHome, yjit, version] =
Expand Down
47 changes: 27 additions & 20 deletions vscode/src/ruby/versionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ export interface ActivationResult {
gemPath: string[];
}

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

export abstract class VersionManager {
protected readonly outputChannel: WorkspaceChannel;
Expand Down Expand Up @@ -56,32 +59,33 @@ export abstract class VersionManager {
// language server
abstract activate(): Promise<ActivationResult>;

protected async runEnvActivationScript(activatedRuby: string) {
protected async runEnvActivationScript(
activatedRuby: string,
): Promise<ActivationResult> {
const activationUri = vscode.Uri.joinPath(
this.context.extensionUri,
"activation.rb",
);

const result = await this.runScript(
`${activatedRuby} -W0 -rjson '${activationUri.fsPath}'`,
`${activatedRuby} -EUTF-8:UTF-8 '${activationUri.fsPath}'`,
);

const activationContent = new RegExp(
`${ACTIVATION_SEPARATOR}(.*)${ACTIVATION_SEPARATOR}`,
`${ACTIVATION_SEPARATOR}([^]*)${ACTIVATION_SEPARATOR}`,
).exec(result.stderr);

return this.parseWithErrorHandling(activationContent![1]);
}

protected parseWithErrorHandling(json: string) {
try {
return JSON.parse(json);
} catch (error: any) {
this.outputChannel.error(
`Tried parsing invalid JSON environment: ${json}`,
);

throw error;
}
const [version, gemPath, yjit, ...envEntries] =
activationContent![1].split(FIELD_SEPARATOR);

return {
version,
gemPath: gemPath.split(","),
yjit: yjit === "true",
env: Object.fromEntries(
envEntries.map((entry) => entry.split(VALUE_SEPARATOR)),
),
};
}

// Runs the given command in the directory for the Bundle, using the user's preferred shell and inheriting the current
Expand All @@ -99,14 +103,17 @@ export abstract class VersionManager {
this.outputChannel.info(
`Running command: \`${command}\` in ${this.bundleUri.fsPath} using shell: ${shell}`,
);
this.outputChannel.debug(
`Environment used for command: ${JSON.stringify(process.env)}`,
);

// Ensure that LANG and LC_ALL are set to UTF-8. If the user has it set to `C`, then the encoding used for
// environment variables may be ASCII-8BIT. If at the same time the user has a shell prompt (PS1) that contains
// certain characters, the encoding conversion will fail and the activation script will not work.
const env = { ...process.env, LANG: "C.UTF-8", LC_ALL: "C.UTF-8" };

return asyncExec(command, {
cwd: this.bundleUri.fsPath,
shell,
env: process.env,
env,
encoding: "utf-8",
});
}

Expand Down
6 changes: 2 additions & 4 deletions vscode/src/test/suite/debugger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,16 +192,14 @@ suite("Debugger", () => {
'source "https://rubygems.org"\ngem "debug"',
);

const extensionPath = path.dirname(
path.dirname(path.dirname(path.dirname(__dirname))),
);
const extensionPath = path.dirname(path.dirname(path.dirname(__dirname)));
const context = {
subscriptions: [],
workspaceState: {
get: () => undefined,
update: () => undefined,
},
extensionUri: vscode.Uri.file(path.join(extensionPath, "vscode")),
extensionUri: vscode.Uri.file(extensionPath),
} as unknown as vscode.ExtensionContext;
const outputChannel = new WorkspaceChannel("fake", LOG_CHANNEL);
const workspaceFolder: vscode.WorkspaceFolder = {
Expand Down
20 changes: 12 additions & 8 deletions vscode/src/test/suite/ruby.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ import { Ruby, ManagerIdentifier } from "../../ruby";
import { WorkspaceChannel } from "../../workspaceChannel";
import { LOG_CHANNEL } from "../../common";
import * as common from "../../common";
import { ACTIVATION_SEPARATOR } from "../../ruby/versionManager";
import {
ACTIVATION_SEPARATOR,
FIELD_SEPARATOR,
VALUE_SEPARATOR,
} from "../../ruby/versionManager";

import { FAKE_TELEMETRY } from "./fakeTelemetry";

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

const envStub = {
env: { ANY: "true" },
yjit: true,
version: "3.3.5",
gemPath: ["~/.gem/ruby/3.3.5", "/opt/rubies/3.3.5/lib/ruby/gems/3.3.0"],
};
const envStub = [
"3.3.5",
"~/.gem/ruby/3.3.5,/opt/rubies/3.3.5/lib/ruby/gems/3.3.0",
"true",
`ANY${VALUE_SEPARATOR}true`,
].join(FIELD_SEPARATOR);

const execStub = sinon.stub(common, "asyncExec").resolves({
stdout: "",
stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`,
stderr: `${ACTIVATION_SEPARATOR}${envStub}${ACTIVATION_SEPARATOR}`,
});

const ruby = new Ruby(
Expand Down
42 changes: 25 additions & 17 deletions vscode/src/test/suite/ruby/asdf.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ import sinon from "sinon";
import { Asdf } from "../../../ruby/asdf";
import { WorkspaceChannel } from "../../../workspaceChannel";
import * as common from "../../../common";
import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager";
import {
ACTIVATION_SEPARATOR,
FIELD_SEPARATOR,
VALUE_SEPARATOR,
} from "../../../ruby/versionManager";

suite("Asdf", () => {
if (os.platform() === "win32") {
Expand Down Expand Up @@ -41,15 +45,16 @@ suite("Asdf", () => {
context,
async () => {},
);
const envStub = {
env: { ANY: "true" },
yjit: true,
version: "3.0.0",
};
const envStub = [
"3.0.0",
"/path/to/gems",
"true",
`ANY${VALUE_SEPARATOR}true`,
].join(FIELD_SEPARATOR);

const execStub = sinon.stub(common, "asyncExec").resolves({
stdout: "",
stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`,
stderr: `${ACTIVATION_SEPARATOR}${envStub}${ACTIVATION_SEPARATOR}`,
});

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

assert.ok(
execStub.calledOnceWithExactly(
`. ${os.homedir()}/.asdf/asdf.sh && asdf exec ruby -W0 -rjson '/fake/activation.rb'`,
`. ${os.homedir()}/.asdf/asdf.sh && asdf exec ruby -EUTF-8:UTF-8 '/fake/activation.rb'`,
{
cwd: workspacePath,
shell: "/bin/bash",
// eslint-disable-next-line no-process-env
env: process.env,
env: { ...process.env, LANG: "C.UTF-8", LC_ALL: "C.UTF-8" },
encoding: "utf-8",
},
),
);
Expand Down Expand Up @@ -95,15 +101,16 @@ suite("Asdf", () => {
context,
async () => {},
);
const envStub = {
env: { ANY: "true" },
yjit: true,
version: "3.0.0",
};

const envStub = [
"3.0.0",
"/path/to/gems",
"true",
`ANY${VALUE_SEPARATOR}true`,
].join(FIELD_SEPARATOR);
const execStub = sinon.stub(common, "asyncExec").resolves({
stdout: "",
stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`,
stderr: `${ACTIVATION_SEPARATOR}${envStub}${ACTIVATION_SEPARATOR}`,
});

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

assert.ok(
execStub.calledOnceWithExactly(
`. ${os.homedir()}/.asdf/asdf.fish && asdf exec ruby -W0 -rjson '/fake/activation.rb'`,
`. ${os.homedir()}/.asdf/asdf.fish && asdf exec ruby -EUTF-8:UTF-8 '/fake/activation.rb'`,
{
cwd: workspacePath,
shell: "/opt/homebrew/bin/fish",
// eslint-disable-next-line no-process-env
env: process.env,
env: { ...process.env, LANG: "C.UTF-8", LC_ALL: "C.UTF-8" },
encoding: "utf-8",
},
),
);
Expand Down
28 changes: 15 additions & 13 deletions vscode/src/test/suite/ruby/custom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import sinon from "sinon";
import { Custom } from "../../../ruby/custom";
import { WorkspaceChannel } from "../../../workspaceChannel";
import * as common from "../../../common";
import { ACTIVATION_SEPARATOR } from "../../../ruby/versionManager";
import {
ACTIVATION_SEPARATOR,
FIELD_SEPARATOR,
VALUE_SEPARATOR,
} from "../../../ruby/versionManager";

suite("Custom", () => {
const context = {
Expand Down Expand Up @@ -40,15 +44,16 @@ suite("Custom", () => {
async () => {},
);

const envStub = {
env: { ANY: "true" },
yjit: true,
version: "3.0.0",
};
const envStub = [
"3.0.0",
"/path/to/gems",
"true",
`ANY${VALUE_SEPARATOR}true`,
].join(FIELD_SEPARATOR);

const execStub = sinon.stub(common, "asyncExec").resolves({
stdout: "",
stderr: `${ACTIVATION_SEPARATOR}${JSON.stringify(envStub)}${ACTIVATION_SEPARATOR}`,
stderr: `${ACTIVATION_SEPARATOR}${envStub}${ACTIVATION_SEPARATOR}`,
});

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

assert.ok(
execStub.calledOnceWithExactly(
<<<<<<< Updated upstream
`my_version_manager activate_env && ruby -W0 -rjson '/fake/activation.rb'`,
=======
`my_version_manager activate_env && ruby '${activationUri.fsPath}'`,
>>>>>>> Stashed changes
`my_version_manager activate_env && ruby -EUTF-8:UTF-8 '${activationUri.fsPath}'`,
{
cwd: uri.fsPath,
shell,
// eslint-disable-next-line no-process-env
env: process.env,
env: { ...process.env, LANG: "C.UTF-8", LC_ALL: "C.UTF-8" },
encoding: "utf-8",
},
),
);
Expand Down
Loading

0 comments on commit 57ae166

Please sign in to comment.