microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.29k stars 29.31k forks source link

API: allow an extension to change the environment variables #152806

Open benmcmorran opened 2 years ago

benmcmorran commented 2 years ago

I'm currently investigating options to simplify the usage of vcpkg artifacts in VS Code. While the example below is specific to vcpkg, similar workflows exist in other languages, like Python's virtual environments.

  1. The user has a vcpkg-configuration.json file checked into their repository that describes the artifacts required by the project. In an embedded development context, this is typically things like ARM compiler toolchains, on-chip debuggers, and associated utilities like serial monitors.
  2. From a shell, the user runs vcpkg activate. This downloads the artifacts as needed and then changes the PATH environment variable in the current shell session to include the artifacts. Other environment variables like CC or CXX may also be set depending on the artifact.
  3. From the same shell session, the user runs code . to launch VS Code in the vcpkg environment.

What I'd like to do is create a VS Code extension that can run the activation steps without requiring the user to manually use a shell. Assuming the extension has a way to retrieve the desired environment variables from vcpkg, I need a way to apply these environment variables to the current VS Code instance. The simplest way I can think of to achieve this by adding a new built-in command that accepts a set of environment variables, then restarts the current VS Code process with the environment variables applied to the new process.

benmcmorran commented 2 years ago

I opened #153019 as a potential implementation. It adds a new vscode.window.reload() API, although I'm also open to implementing this as a built-in command if reviewers think that's a better fit.

bpasero commented 2 years ago

I wonder if it would be sufficient to just restart the extension host with these new environment variables? Not super keen on allowing extensions to reload windows tbh.

And would this persist when the user manually opens new windows or restarts? Or would it be only for the duration of the session?

Since this is new API, it needs to be presented in our API calls and discussed. Can you ask @mjbvz and @jrieken for details how to join the meeting and let me know when you plan to join so that I can also be there? It is typically Tuesday morning.

//cc @Tyriar for thoughts since I think you had a similar issue where extensions could change the environment variables for a terminal specifically.

Tyriar commented 2 years ago

On my end we have:

@benmcmorran it sounds like environmentVariableCollection may be exactly what you're after? It depends where you want the environment variables.

Not super keen on allowing extensions to reload windows tbh.

👍. If it was required in the extension host, the window could have a similar dialog to terminal tabs if the extension host wanted to listen for environmentVariableCollection changes:

image

Such a dialog would probably cause a bunch of disruptive messages for the user though. Plus since the usage is mainly targeted at the terminal, we would probably want to add a flag for whether it's important to the extension host so changes to something like GIT_EDITOR doesn't ask to restart the extension host.

benmcmorran commented 2 years ago

it sounds like environmentVariableCollection may be exactly what you're after?

Thanks, I missed this earlier, but it looks like almost exactly what we need! In addition to the terminal support that already exists, we'll also need the environment to apply to the extension host process. This is to support use cases like this:

The scenarios above are narrowly scoped to C/C++ development, and indeed that is the main use case for vcpkg today. Ideally, we could apply the environment variables to all extensions via the extension host process to closely match what would have happened when launching VS Code from a vcpkg activated command line, but if you think that's too extreme it would also work for us to have some way for specific extensions to opt-in to receiving environment variable changes. However, I'm not sure if that approach is technically feasible given that multiple extensions may be running in the same extension host process.

we would probably want to add a flag for whether it's important to the extension host

That makes sense to me.

And would this persist when the user manually opens new windows or restarts? Or would it be only for the duration of the session?

For our purposes this doesn't need to be persistent across all sessions, but it would be convenient if it persisted on a per-workspace basis because the vcpkg-configuration.json file is generally tied to a workspace. It looks like ExtensionContext.environmentVariableCollection already has a flag for this.

Can you ask @mjbvz and @jrieken for details how to join the meeting and let me know when you plan to join so that I can also be there?

Will do. I started a thread internally as well in case that's an easier way to send the meeting invite.

Tyriar commented 2 years ago

@benmcmorran the change to EnvironmentVariableCollection is relevant to that meeting as well. An API change for that would look something like below, I'm not sure who would own the restarting of ext host part of that work and the UX around it though as they would need to be on board with the idea. Maybe also @jrieken?

export interface EnvironmentVariableCollection {
    // Defaults to terminal
    // Not sure if should apply to one or both, vs just warning for the target
    target?: 'exthost' | 'terminal' | 'both';
}
benmcmorran commented 2 years ago

Sorry about my delay responding here -- I'm back with a more concrete proposal and some UX options for discussion before seeking more formal approval for the API changes. @bpasero @alexdima @Tyriar let me know if you what you think of this.

Proposed API changes

This proposal adds a new extHostEnvironment proposed API with these changes:

declare module 'vscode' {
    /**
     * A target process type for an environment variable collection.
     */
    export enum EnvironmentVariableCollectionTarget {
        /**
         * Environment variables apply to all target processes types.
         */
        All = 1,

        /**
         * Environment variables only apply to terminal processes.
         */
        Terminal = 2,

        /**
         * Environment variables only apply to extension host processes.
         */
        ExtensionHost = 3,
    }

    export interface EnvironmentVariableCollection {
        /**
         * The type of process that these environment variable changes should
         * apply to. Defaults to `EnvironmentVariableCollectionTarget.Terminal`.
         */
        target: EnvironmentVariableCollectionTarget;
    }
}

Persistence of the environment variable collection will follow the same rules it follows today. If EnvironmentVariableCollection.persistent is true, then the environment variable collection will be cached and applied to the terminal, extension host process, or both across window reloads.

Changes to EnvironmentVariableCollection.target will not immediately take effect in the target processes. If target changes to include or exclude terminal processes (i.e. everything except transitions between All and Terminal) then the same UI that exists today will be shown to prompt the user to relaunch the terminal process.

image

If target changes to include or exclude the extension host process (i.e. everything except transitions between All and ExtensionHost), then new UI will be shown prompting the user to reload the extension host process. This UX still needs discussion, so I outlined a few options below that I'd like to get feedback on.

UX for extension host reload

First, a warning notification appears prompting the user to reload the extension host process. There currently isn't enough control in the VS Code API to make this notification as full fidelity as the terminal pop-up shown above by using a code block to display which environment variables will change.

image

A persistent warning will also be added to the status bar, similar to the "Restricted Mode" badging. Clicking on this warning will show a pop-up similar to the pop-up shown for terminal changes, which shows which environment variables will change and provides a link to reload the extension host process.

image

As alternatives to the status bar, a warning badge could be added to the extensions view icon, or an infobar like the bar used for restricted mode could be added to the window. I don't like these approaches as much because the view icon might be hidden, and the infobar feels a bit too intrusive.

image

bpasero commented 2 years ago

I suggest a call with the people you pinged in August to drill into this.

benmcmorran commented 2 years ago

Sent an invite. Availability was sparse over the next few weeks, so the earliest slot I was able to find is August 8.

benmcmorran commented 2 years ago

Summarizing results of our discussion:

mohsenari commented 2 years ago

Wondering if there are any updates on this. I'm in the process of developing an extension for our CLI tool which creates isolated nix shell environments. Being able to change env variables means we can simulate the same isolated environments in vscode.

benmcmorran commented 2 years ago

No updates on my side. Based on the discussion summarized above with the VS Code core team (I work on C++ extensions, not core), we're currently implementing this purely through targeted updates to well-known VS Code extensions for C++ development, which mostly meets our specific needs for vcpkg but is definitely not a general solution.

If there's enough community interest, I'm sure we could restart discussions about a VS Code API. Right now it sounds like it's just vcpkg and nix.

Also note that because VS Code extenions all run in the same extension host process, there's nothing technically stopping you from just changing process.env in your extension and having the change apply to all other extensions. Obviously, it's a little risky and won't flush any internal caches extensions may be keeping, but for certain scenarios it may be enough.

Strum355 commented 1 year ago

If there's enough community interest, I'm sure we could restart discussions about a VS Code API. Right now it sounds like it's just vcpkg and nix.

Unless Im misunderstanding, theres a lot of interest outside of vcpkg and nix, including the direnv community. See:

and some other issues I'm not going to dig through my browser history for.

The proposed API changes of yours above looked promising, so its a bit disappointing to see its progress paused in favour of vcpkg specific solutions 😕 Is there a process for getting momentum again?

benmcmorran commented 1 year ago

@Strum355 thanks for those pointers!

@bpasero @Tyriar @alexdima looks like there are at least three groups (vcpkg, nix, and direnv) interested in an API for extensions to set environment variables. Is that sufficient to restart the discussion we paused earlier about adding the proposed extHostEnvironment VS Code API? Rereading the earlier discussion, the remaining open issues are lifecycle concerns around reading persisted environment variable state before launching the extension host process, and formal review of the UI changes.

If this seems like enough community interest, just let me know what next steps I should take (e.g. schedule a meeting, open a PR, continue discussion here, etc.).

cc @qarni

tanshihaj commented 1 year ago

Maybe as a workaround we can create some API to tell VS Code to load all other extensions after extension that want to change process.env fully activates?

opeik commented 1 year ago

Making a bespoke vcpkg solution feels like a misstep. A lot of workflows could benefit from the proposed API changes. Personally, I can't recommend VSCode+Nix to people due to how flaky (ha) the integration is. Ideally, direnv should be able to do its magic without requiring a window reload, and without worrying about extension load order.

Strum355 commented 1 year ago

Personally I wouldnt even mind a window reload, just my 2c. I understand it may be a bigger concern on less powerful machines though

offlinehacker commented 1 year ago

Now that https://github.com/microsoft/vscode/issues/57481 is closed, does that mean that this issue is also out of scope or is there any other plan to be able to change vscode running environment during runtime? It looks like this functionality would be beneficial whenever you want to set environment variables dynamically, where nix and direnv are just two of such examples. API like this could also beneficial for other extensions down the chain, for programming languages like python, where you are dealing with environment loading from virtualenv and conda.

cc @isidorn

thegecko commented 1 year ago

The ability to update environment settings for the extension host would be a really useful addition for our embedded tool workflows at Arm.

Development tools (especially embedded ones) can be very difficult to set up and creating isolated, reproducible environments with ease using tools such as vcpkg is one way to approach this. Another could be python venv.

The ability to activate such environments from within VSCode and proliferate the session configuration brings the ease of use of the tool into the GUI and remove any need for context switching.

JPHutchins commented 1 year ago

I develop a tool for setting environment variables in Linux/Windows/MacOS. Motivation was largely unifying development environment setup across all OS, but I thought an added benefit would be the ability to use these environment variables (or PATH additions) in launch.json or other VSCode configs.

I ran into the same problem as VCPKG wherein I must instruct users to setup their environment before starting VSCode from that terminal, . ./envr.ps1 && code .. It's not that big of a deal, but I feel like embedded developers are finally getting some modern support, and it would be a nice bonus to be able to share OS/toolchain-agnostic launch.json configs across communities.

LMK how I can help support this needed VSCode enhancement.

opeik commented 1 year ago

I'm also happy to assist.

mohsenari commented 1 year ago

For anyone who stumbles upon this thread in the future, I managed to get around this issue by spawning a sub process that sets the env variables I want and re-opens vscode. But this only works for macOS and Linux. I wrote this blog post that explains the details.

zimbatm commented 1 year ago

Another approach that could be interesting would be to add a facility to wrap process invocation. This would be a workspace-level setting that could be configured by the user or extensions.

With that in place, it becomes possible to for example replace a bash invocation with a direnv exec . bash one. Where direnv exec . is the value of the setting.

elupus commented 1 year ago

Is there any progress on this. It is a rather annoying problem when trying to use virtual environments for vscode and the cmake integration.

stasberkov commented 2 weeks ago

I have asp.net core project with configuration via files and environment variables. I have several environments to switch between: sqa1, sqa2, sqa3, sqa4. Environment defines what servers my application use for development. Having some env switch (that changes environment variables) within VS Code would be terrific.