Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Don't pass ELECTRON_RUN_AS_NODE to subprocess #305

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

RichardDRJ
Copy link
Contributor

This change addresses #303, which prevents commit messages from being written in remote hosts.

In VSCode's server-cli.js, it checks for the ELECTRON_RUN_AS_NODE environment variable, and if it's present it maps the fs module to point to the original-fs module:

(process.env.ELECTRON_RUN_AS_NODE || process.versions.electron) &&
  c.define("fs", ["original-fs"], function (r) {
    return r;
  }),

This module is present in Electron, but not in base NodeJS; base NodeJS is what's used by the VSCode remote server, so having this environment variable present when code is called from a subprocess will cause failure.

An alternative, but more involved, fix here might be to move to using ShellExecution or ProcessExecution rather than child_process; I expect that they will support calling code from a nested process, but they have a fairly different API.

This change addresses kahole#303, preventing commit messages from being written in remote hosts.

In VSCode's `server-cli.js`, it checks for the `ELECTRON_RUN_AS_NODE` environment variable, and if it's present it maps the `fs` module to point to the `original-fs` module:

```javascript
(process.env.ELECTRON_RUN_AS_NODE || process.versions.electron) &&
  c.define("fs", ["original-fs"], function (r) {
    return r;
  }),
```

This module is present in Electron, but not in base NodeJS; base NodeJS is what's used by the VSCode remote server, so having this environment variable present when `code` is called from a subprocess will cause failure.

An alternative, but more involved, fix here might be to move to using `ShellExecution` or `ProcessExecution` rather than `child_process`; I expect that they will support calling `code` from a nested process, but they have a fairly different API.
@TomasEkeli
Copy link

i just tested this on windows working in wsl - and this fixes issue #303 for me. 👍

@TomasEkeli
Copy link

can we get this merged, @kahole? current version is broken

@kahole kahole merged commit 75597b9 into kahole:develop Sep 9, 2024
@kahole
Copy link
Owner

kahole commented Sep 9, 2024

Will push new version this week

@kahole
Copy link
Owner

kahole commented Sep 12, 2024

v0.6.62 published

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants