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

Pipe commands have issues in devcontainers #321

Closed
losnappas opened this issue Nov 17, 2023 · 7 comments
Closed

Pipe commands have issues in devcontainers #321

losnappas opened this issue Nov 17, 2023 · 7 comments
Labels
bug Something isn't working

Comments

@losnappas
Copy link

I'm on the pre-release version and seeing some issues w/ in dev containers especially:

  1. Start up a new dev container, w/o linking to a specific folder on host (Dev Containers: New Dev Container... command), I picked Alpine w/ defaults
  2. Run a pipe command on some file in the container, I have not defined any "automationProfile"
  3. Error: error executing command "dance.selections.pipe.replace": Invalid host defined options

I didn't look into how to debug/test the issue, devcontainers like to install extensions from the store.

@71 71 added the bug Something isn't working label Nov 18, 2023
@71
Copy link
Owner

71 commented Nov 18, 2023

Thanks for the report! The bug is also on the release version. I investigated a bit and it's quite strange:

  1. !#echo hi outside of debugging (both inside and outside of a devcontainer) gives me "Invalid host defined options".
  2. If I launch Dance and debug it !#echo hi works outside a devcontainer and leads to spawn /bin/sh ENOENTinside a devcontainer.

Since commands need child_process to execute, I'm guessing VS Code limits access to it, but maybe not when debugging is enabled. Does piping work locally?

@losnappas
Copy link
Author

losnappas commented Nov 18, 2023

Piping does indeed work locally.

I tried version 0.5.12001 (the pre-release from 1 year ago), and !#echo 123 works inside the devcontainer with that. However, the command is run on host rather than inside the devcontainer (which may be convenient).

From that, I figured out that piping works (on latest) if I do

  "remote.extensionKind": {
    "gregoire.dance": ["workspace"]
  }

So I'm guessing running child_process as UI extension w/ devcontainers is buggy. !#echo $USER gives a blank string as UI extension, but gives username as workspace extension.

@71
Copy link
Owner

71 commented Nov 18, 2023

I experimented a bit more with ways to use built-in VS Code APIs rather than child_process to execute commands.

In both cases, I execute a vscode.Task, which allows me to run an arbitrary command.

const task = new vscode.Task(
  { type: "process" },
  vscode.TaskScope.Workspace,
  "Run Dance command",
  "Dance",
  new vscode.ProcessExecution(
    "/bin/sh",
    [
      "-c",
      `...`,
    ],
    {
      // VS Code will fail to compute `cwd` if there is no workspace folder. Help it.
      cwd: cwd ?? process?.cwd() ?? "/",
      env: {
        DANCE_COMMAND: command,
        DANCE_INPUT: input ?? "",
        ...givenEnv,
      },
    },
  ),
);
task.presentationOptions = {
  reveal: vscode.TaskRevealKind.Silent,
  echo: true,
  focus: false,
  panel: vscode.TaskPanelKind.New,
  showReuseMessage: false,
  clear: true,
  // @ts-expect-error: only recognized by "process" tasks
  close: true,
};
const execution = await vscode.tasks.executeTask(task);

The problem is that there is no API to get the output of the command. I tried two things to get around this problem, both of which work in local scenarios (replace ... with the following text in the example above):

  1. Register a URI handler in VS Code and execute it from the command line:

    code --open-url ${vscode.env.uriScheme}://${extensionId}/provide-command-output?id=${id}&output=`echo "${input}" | ${command}`&exitCode=$?

    Unfortunately, there doesn't appear to be a way to get the path to code consistently above. code and "$_" work locally (on macOS), but not remotely. There doesn't seem to be an API to get the proper path of the code binary in a remote environment.

    Outside of that problem, two other issues would have to be fixed:

    1. We only extract stdout above, we also need stderr (that can probably be done by using pipes within the shell script, and passing two arguments to the command).
    2. The output of the command is not escaped, so handling in the URL may not be great.
  2. Write the outputs to temporary files, and read them from VS Code:

    const storageUri = extensionContext.storageUri ?? extensionContext.globalStorageUri;
    const stdoutUri = vscode.Uri.joinPath(storageUri, executionId + ".stdout");
    const stderrUri = vscode.Uri.joinPath(storageUri, executionId + ".stderr");
    echo "${input}" | ${command} >${stdoutUri} 2>${stderrUri}

    Now, the problem is that storageUri is always local to Dance; even extensionContext.storageUri (which is scoped to the current workspace, if any) points to the local file system, so the remote endpoint cannot write to these paths.

Other problems

Executing commands using vscode.Task appears to be very slow as it sets up a new terminal, a new task, and then runs the command. It does integrate quite nicely with VS Code, and does not suffer from the whole "Invalid host defined options" thing.

Also, the above solution works with /bin/sh only. Adapting it to work with cmd is probably possible, but it would be best to respect vscode.env.shell. That's not possible though as shells may not support redirections and/or process substitution as used above.

Way forward

For remote scenarios, I believe the only way to make things work is to create a new extension (potentially named "Dance - Remote command execution") which is installed in the remote (unlike Dance, which is installed in the client), with which Dance can communicate to run commands locally (either with child_process or with vscode.Task -- note that the former is used in Microsoft's extensions as well).

The proposed API vscode.window.onDidExecuteTerminalCommand may also help solve this issue, but it is not yet stable (so Dance cannot be published in the marketplace with it).

@71
Copy link
Owner

71 commented Nov 18, 2023

I tried a bit more to get it working in a devcontainer by using ps -o command= -p $PPID to get the path of the binary running in the server. Unfortunately, it appears to be node, which is not what we want. code is also available in the devcontainer, but that seems like a shim to access the "client" VS Code, and it does not support --open-url.

Perhaps using automatic VS Code port forwarding and setting up a node server which listens for command requests and executes them could work.

@losnappas
Copy link
Author

Separating the piping into its own extension isn't a bad idea, it's already solid functionality in and of itself. There's similar alternatives but I really like the js/shell pattern going on there (with #).

But, on the old version, 0.5.12001, pipes work, they just run on the host instead of inside the container. Any idea what's different there?

@71
Copy link
Owner

71 commented Nov 19, 2023

Oh, that's right, they did work before... It looks like if I remove cwd from the call to cp.spawn(), things work fine again.

@71
Copy link
Owner

71 commented Nov 19, 2023

This should be fixed by e9cd2ca.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants