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

Spike - Activation script sent to terminal after debugger starts (Use Conda run) #5664

Closed
DonJayamanne opened this issue May 17, 2019 · 6 comments
Assignees
Labels
area-terminal bug Issue identified by VS Code Team member as probable bug important Issue identified as high-priority

Comments

@DonJayamanne
Copy link

We have issues reported where:

  • Debugger starts and the debug scripts are sent to the terminal
  • The activation scripts for the environment are sent to the terminal after the above debugger scripts

We have experienced this today with:

  • pipenv shell being sent after debugger scripts
  • conda activate .... being sent after debugger scripts
@ghost ghost added the triage-needed Needs assignment to the proper sub-team label May 17, 2019
@DonJayamanne DonJayamanne self-assigned this May 17, 2019
@DonJayamanne DonJayamanne added area-terminal needs upstream fix important Issue identified as high-priority bug Issue identified by VS Code Team member as probable bug labels May 17, 2019
@ghost ghost removed triage-needed Needs assignment to the proper sub-team labels May 17, 2019
@DonJayamanne DonJayamanne removed their assignment Jun 24, 2019
@ghost ghost added triage-needed Needs assignment to the proper sub-team labels Jun 24, 2019
@DonJayamanne
Copy link
Author

Solution: Use VS Code debugger api to send the two commands separately:

  • First command - activate the necessary environment
  • Second command - send debugger commands
  • We probably also need to tell our own extension NOT to send activation commands to the terminal when these messages are being sent.

@karrtikr
Copy link

karrtikr commented Jun 26, 2019

Prescribed solution

this.debugSession.runInTerminalRequest(termArgs, 5000, (response) => {
    if (response.success) {
        resolve();
    } else {
        reject(response);
    }
});

@brettcannon brettcannon added this to the 2019 - June Sprint 13 milestone Jun 26, 2019
@ericsnowcurrently ericsnowcurrently self-assigned this Jul 8, 2019
@ericsnowcurrently
Copy link
Member

for upstream: see microsoft/vscode#67692

@ericsnowcurrently
Copy link
Member

To try this out, I added the following to src/client/debugger/debugAdapter/DebugClients/LocalDebugClient.ts (right before the existing const termArgs: ... line in launchExternalTerminal():

                if (!isExternalTerminal) {
                    const termArgs: DebugProtocol.RunInTerminalRequestArguments = {
                        kind: consoleKind,
                        title: 'Python Debug Console',
                        cwd: cwd,
                        args: ['echo', 'spam'],
                        env: env
                    };
                    this.debugSession.runInTerminalRequest(termArgs, 5000, (response) => {
                        if (response.success) {
                            resolve();
                        } else {
                            reject(response);
                        }
                    });
                }

Unfortunately, it opened a separate terminal for each request. We we're back to the drawing board.

The only other realistic solutions (without upstream changes):

  • generate a shell script that contains the two shell invocations (requires quoting the args)
  • add a static script that supports executing multiple "argv"s, e.g. separated by a semicolon

@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented Jul 15, 2019

I ran into the following problems:

A. DebugSession.runInTerminalRequest() does not support re-using a terminal from an earlier call; so we can't just call that API once for activation, followed by once for the debugger
B. runInTerminalRequest() does not support sending multiple commands (sort of)
C. we can't use runInTerminalRequest() directly for activation commands (they do things like "source" and runInTerminalRequest() prepends the env vars to the "command" string that gets executed, which is incompatible with "source")
D. getting the activation commands to the debugger isn't trivial

So we have a few options at this point if we want to activate environments:

  1. make the status quo work (via TerminalAutoActivation)
    • none of the above problems
    • complicated and maybe not even possible
  2. do as much as possible in the debugger
    • keeps the debug adapter code in the debug adapter
    • suboptions dealing with problem D:
      a. pass the activation commands via the launch config
      - requires getting them into the config dynamically
      b. use a custom DAP request to get the activation commands from the extension
      - only custom events are currently supported?
      c. use a custom DAP event to request the activation commands from the extension
      - we already have the machinery in the extension for custom DAP events
      - how to get them back? through a file?
    • suboptions to deal with problems A, B, and C:
      d. wrap activation commands and debugger command in a script to run via runInTerminalRequest()
      - requires a temporary shell script written to disk
      - requires quoting, etc.
      e. add a static script that decodes its args into multiple commands and runs them
      - takes advantage of VSC arg quoting, etc.
      f. (upstream) get runInTerminalRequest() updated to solve the problem
      - support multiple requests against the same terminal (by PID or some other ID)
      - (or support passing multiple commands in a single request)
      - support "source", etc commands (move env vars to end?)
      g. chain the commands (e.g. with && or ;) and wrap the whole thing in e.g. sh -c ""
    • suboptions to deal with problems A, and B:
      h. chain commands, e.g. with && or ;
      - env vars might only apply to the first command (need to check)
    • suboptions to deal with problems C:
      i. chain the activation command with a preceding noop (e.g. true && source ...)
      - env vars might only apply to the first command (need to check)
  3. do as much as possible in the extension
    • not affected by the problems from above
    • solution (in debug adapter):
      • add a way to trigger starting the debugger from the extension (via the debug adapter protocol)
        1. use a custom DAP request
          • only custom events are currently supported?
        2. use a custom DAP event
          • we already have the machinery in the extension for custom DAP events
          • how to indicate ready? through a file?
      • trigger that event/request (send debugger command and env vars)
      • wait for the debugger to start (necessary for some internal VSC debugger functionality)
    • solution (in extension)
      • add DAP event handler
      • create terminal
      • activate environment in terminal (like normal)
      • run debugger command in terminal
      • notify debug adapter

Might be necessary:

  • generate activation commands as multiple argv (string[][]) instead of a single string
    • necessary for use with spawn and
    • conda
    • pipenv
    • pyenv
    • venv, etc.

Other notes:

@karrtikr karrtikr changed the title Activation script sent to terminal after debugger starts Spike - Activation script sent to terminal after debugger starts Jul 31, 2019
@karrtikr
Copy link

karrtikr commented Jul 31, 2019

Spike

  • Will using conda run work in terminal (works, this is a documented solution)
  • Will using conda run in background process work (spawn('conda run xyz.py')), i.e. we're not running in a terminal.
  • Will using <fully qualified path to conda executable> run work` in terminal
  • Will using <fully qualified path to conda executable> run work` in background process.

@karrtikr karrtikr changed the title Spike - Activation script sent to terminal after debugger starts Spike - Activation script sent to terminal after debugger starts (Use Conda run) Jul 31, 2019
@DonJayamanne DonJayamanne self-assigned this Aug 5, 2019
@ghost ghost removed needs PR labels Aug 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-terminal bug Issue identified by VS Code Team member as probable bug important Issue identified as high-priority
Projects
None yet
Development

No branches or pull requests

4 participants