Portable mode: VSCODE_PORTABLE not available for uninstall script

[JimiC]

• VSCode Version: 1.25.1
• OS Version: Any

Steps to Reproduce:

1. Uninstall any extension that provides a vscode:uninstall script
2. Inspect process.env.
3. process.env.VSCODE_PORTABLE is missing

Does this issue occur when all extensions are disabled?: Yes

When trying to support Portable Mode on vscode-icons, process.env.VSCODE_PORTABLE is very handy to determine if the vscode instance is started in portable mode (thus finding where the user-data directory is). Unfortunately the same can not be achieved for the uninstall script.

[joaomoreno]

@JimiC Isn't it better to actually get the user-data directory?

Why do you need that path, anyway?

[jens1o]

@joaomoreno See:
https://github.com/vscode-icons/vscode-icons/blob/a9a31ac3ccbb26105b4250292709b0c84848d486/src/cleanUp/index.ts#L15-L31

It is needed to delete all configuration files.

[JimiC]

@joaomoreno You see having process.env.VSCODE_PORTABLE simplifies the whole determining logic.

Then the code would look like this:

let userDir: string;
if (process.env.VSCODE_PORTABLE) {
  userDir = path.join(process.env.VSCODE_PORTABLE, 'user-data', 'User');
} else {
  const appDir = /^win/.test(process.platform) 
    ? process.env,APPDATA 
    : process.platform === 'darwin'
        ?`${os.homeDir()}/Library/Application Support` 
        : process.platform === 'linux' 
           ? `${os.homedir()}/.config`
           : '/var/local';
  const baseUserDataDir = /dev/i.test(__dirname)
    ? 'code-oss-dev'
    : /oss/i.test(__dirname);
       ? 'Code - OSS'
       : /insiders/i.test(__dirname)
          ? 'Code - Insiders'
          : 'Code';
  userDir = path.join(appDir, baseUserDataDir, 'User');
}

Now, if we had an env variable for the "user data" directory and one for the "extensions" directory, for the non-portable instances, I would make you a statue.

[joaomoreno]

Now, if we had an env variable for the "user data" directory and one for the "extensions" directory, for the non-portable instances, I would make you a statue.

Get the marble.

[joaomoreno]

Actually, hold the marble.

After discussing with @sandy081, we think you might benefit more from two other options:

• If you had VS Code API in the uninstall hook, you could simply use the configuration API to delete settings: Provide access to VS Code API in extension uninstall lifecycle hook #45474
• Provide a declarative way of marking settings as to be deleted when the extension is uninstalled: Provide access to VS Code API in extension uninstall lifecycle hook #45474 (comment)

This is because our settings files might (and often do) contain comments, which your parseJSON function can't handle. We also should preserve settings file whitespace as much as possible when modifying it, which your saveSettings function won't do.

Closing this for now.

[JimiC]

Having access to vscode API is a plus, and would help in cleaning up the settings.json properly. But what about the scenario that we want to delete a file that was put in the User directory by the extension? And that proposal surely doesn't justify a statue. 😄

[joaomoreno]

But what about the scenario that we want to delete a file that was put in the User directory by the extension?

Well, I sure do hope the hypothetical extension would be using the official ExtensionContext.storagePath API, instead of putting files where it shouldn't. 😉

[JimiC]

Ooops. I'll have to explore that ExtensionContext, which I admit, was not aware of.
I thought that this was only to register the commands. RTFM to the rescue.

[JimiC]

I see it now. Guilty as charged. Just for the record, I've seen other extensions storing files where they shouldn't too. We are going to adjust our code to use the API instead. And let's hope we will get the API for the uninstall script real soon.

[joaomoreno]

@JimiC Sweet!

[JimiC]

@joaomoreno After trying to implement settings storage using ExtensionContext.storagePath I came upon the following cases:

• storagePath is undefined when a workspace is not loaded
• storagePath is different for every workspace loaded

Therefore using ExtensionContext.storagePath to store extension settings is not a viable solution, as a constant path is required.

The offer for the statue still stands.

[joaomoreno]

cc @sandy081 for insights

[sandy081]

@JimiC You are right, storagePath is workspace scoped. Why not using ExtensionContext.extensionPath for global storage of files? These gets cleaned up when extension is removed from the disk

[JimiC]

Because we want this file to be persistent during a new extension version release. If we are to use ExtensionContext.extensionPath then we will have to implement a previous extension version folder detection (note that not everyone updates as soon as a new version gets released), which can cause other issues for us, and transfer that file to the new extension version folder, modified appropriately.

[JimiC]

This #2741 would be the proper solution.

[JimiC]

@sandy081 One other thing is that we support icons customization and we need a persistent location where the user can put their custom icons. These icons should not be removed with every new extension version release. User directory seems the logical place to put those.

[sandy081]

I see, not sure if we expose user data directory as an API.

@jrieken What location do we recommend authors if they want to store files as global cache?

If user data directory is the common usage why not expose it in the extension context?

[jrieken]

There is ExtensionContext#storagePath which can be used to persist things

[JimiC]

@jrieken We have already ruled out that this path does not suffice (#54345 (comment)).
We need a location to persist things across new extension version releases.

[JimiC]

@jrieken Can we get an ExtensionContext#userDataPath ?

[jrieken]

That sounds wrong... The ExtensionContext is private and scoped to each extension and userDataPath sounds pretty global/shared to my ears. Do you want something like ExtensionContext#globalStoragePath that's shared between all windows (empty, single file, folder, workspace) but scoped to an extension?

[JimiC]

Do you want something like ExtensionContext#globalStoragePath that's shared between all windows (empty, single file, folder, workspace) but scoped to an extension?

If this will be persistent across extension updates, yes, please. And we will need this path to be available on uninstall script also. Is there a way to have access to ExtensionContext in that too?

[JimiC]

@jrieken I have solved half my problem by using ExtensionContext#globalState to store the extensions state (the place was obvious but who cares to read the entire f@cking manual). This though requires that ExtensionContext#globalState is accessible in the uninstall script also so that we can clean up the state, although I'm not sure that this task is crucial.

Now I'm up to solve the other half of my problem, which is where to store user files by default (in our case custom icons) and have access to that via the API. Any recommendations?

[joaomoreno]

Do you want something like ExtensionContext#globalStoragePath that's shared between all windows (empty, single file, folder, workspace) but scoped to an extension?

@jrieken 👍