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

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

Open
benmcmorran opened this issue Jun 21, 2022 · 23 comments
Open

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

benmcmorran opened this issue Jun 21, 2022 · 23 comments
Labels
api api-proposal feature-request Request for new features or functionality under-discussion Issue is under discussion for relevance, priority, approach workbench-os-integration Native OS integration issues
Milestone

Comments

@benmcmorran
Copy link
Member

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.

  • Does this sound like the right approach to solving this problem? I'm willing to create the PR if the approach is sound.
  • Would it be better to restart the entire process, or instead take the more limited approach of the existing "Developer: Reload Window" command which restarts most child processes but leaves the main VS Code process intact?
@benmcmorran
Copy link
Member Author

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.

@weinand weinand assigned bpasero and unassigned weinand Jun 23, 2022
@bpasero bpasero added api under-discussion Issue is under discussion for relevance, priority, approach workbench-os-integration Native OS integration issues labels Jun 24, 2022
@bpasero bpasero added this to the July 2022 milestone Jun 24, 2022
@bpasero
Copy link
Member

bpasero commented Jun 24, 2022

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.

@bpasero bpasero changed the title Feature request: command to restart in custom environment API: allow to reload window with different process environment Jun 24, 2022
@Tyriar
Copy link
Member

Tyriar commented Jun 24, 2022

On my end we have:

  • ExtensionContext.environmentVariableCollection which lets extensions contribute to VS Code's environment, this currently only applies to the terminal but the API was made such that it could be applied elsewhere if we wanted. These environment changes are persisted for the workspace unless the extension opts out.
  • Allow creating a new terminal to pull in a new environment block on Windows Use fresh environment block on new terminals in Windows #47816, this has a decent amount of votes as currently the only way to do this is to restart the main process.

@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
Copy link
Member Author

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:

  • vcpkg adds the arm-none-eabi-gdb debugger to PATH, which the C/C++ extension can then resolve based on seeing arm-none-eabi-gdb as the miDebuggerPath in launch.json.
  • vcpkg adds cmake to PATH which the CMake extension then uses for build system generation.
  • vcpkg sets CC to arm-none-eabi-gcc, which the CMake extension (and perhaps Makefile tools as well) then indirectly uses during build.

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
Copy link
Member

Tyriar commented Jun 24, 2022

@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';
}

@bpasero bpasero changed the title API: allow to reload window with different process environment API: allow an extension to change the environment variables Jun 25, 2022
@jrieken jrieken removed their assignment Jul 5, 2022
@bpasero bpasero modified the milestones: July 2022, Backlog Jul 11, 2022
@bpasero bpasero added the feature-request Request for new features or functionality label Jul 18, 2022
@bpasero bpasero removed their assignment Jul 18, 2022
@benmcmorran
Copy link
Member Author

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
Copy link
Member

bpasero commented Jul 26, 2022

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

@benmcmorran
Copy link
Member Author

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

@Tyriar Tyriar removed their assignment Aug 8, 2022
@benmcmorran
Copy link
Member Author

Summarizing results of our discussion:

  • Instead of adding a new VS Code API, we'll start by using an extension API to allow other extensions to opt-in to awareness of vcpkg environment changes. This is a less generic solution that restarting the extension host process, but it's also less invasive.
  • There may be some overlap between this issue and Use new vsc API to activate terminal without running any commands in terminal vscode-python#11039
  • The VS Code API proposed above would require a reload on first use, but after first use environment variable state would be saved using the persistent flag on the EnvironmentVariableCollection, so hopefully the environment variables changes requested by vcpkg would be a no-op and the user would have a quick, seamless folder opening experience. There may be lifecycle concerns here for an implementation because the persisted environment variable state needs to be read before launching the extension host process.
  • From a UX perspective, it's difficult to know what to communicate to the user during the potentially long period between the folder opening and environment variables being available from vcpkg. vcpkg may need to download large binaries during this time. The user shouldn't be completely blocked, but they'll have a degraded experience during this time and may encounter squiggles in the editor and failing builds.
  • We could also consider an API that changes the extension host environment on-the-fly (like editing process.env) and provides an event when changes are made so that other extensions can flush internal caches of environment variables. However, this API likely wouldn't see broad adoption because some extensions already write directly to process.env.

@mohsenari
Copy link

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
Copy link
Member Author

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
Copy link

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
Copy link
Member Author

@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
Copy link

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
Copy link

opeik commented Nov 24, 2022

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
Copy link

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
Copy link

Now that #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
Copy link
Contributor

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
Copy link

JPHutchins commented Apr 11, 2023

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
Copy link

opeik commented Apr 19, 2023

I'm also happy to assist.

@mohsenari
Copy link

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
Copy link

zimbatm commented Jul 26, 2023

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
Copy link

elupus commented Oct 30, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api api-proposal feature-request Request for new features or functionality under-discussion Issue is under discussion for relevance, priority, approach workbench-os-integration Native OS integration issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.