search

eclipse-theia / theia

Eclipse Theia is a cloud & desktop IDE framework implemented in TypeScript.
http://theia-ide.org
Eclipse Public License 2.0
20.05k stars 2.5k forks source link

Theia apps: option to respect each operating system standards when chosing their user-level `config folder` path #1518

Open simark opened 6 years ago

simark commented 6 years ago

Update:

The XDG Base Directory Specification is the closest thing Linux has to a standard of where to put files. We should try to respect it as much as possible. Currently, we just put things in $HOME/.theia. For example, the user should be able to override where the local config is with the XDG_CONFIG_HOME env var. If it is not set, it should fallback to $HOME/.config.

This NPM module should make it easy to get the directory paths (not sure it works cross-platform though):

https://www.npmjs.com/package/xdg-basedir

epatpol commented 6 years ago

I just tested and it seems to work fine with Windows, even changing XDG_CONFIG_HOME outputs the correct new location. Now we just need someone on mac OS to test the library. @MarkZ3 if you have a bit of time?

npm install xdg-basedir

Launch node shell with node or put the following in a test.js file and call node test.js

const xdgBasedir = require('xdg-basedir');

console.log(xdgBasedir.data);
console.log(xdgBasedir.config);
console.log(xdgBasedir.dataDirs)

and then do the same with changing the env variable XDG_CONFIG_HOME just before the calls like so

process.env.XDG_CONFIG_HOME=/random/path

console.log...

And check if it outputs the path relative to the new variable now.

MarkZ3 commented 6 years ago

The output I get is:

/Users/myuser/.local/share
/Users/myuser/.config
[ '/Users/myuser/.local/share',
  '/usr/local/share/',
  '/usr/share/' ]

However, I have almost nothing in .local/share and .config, it seems "~/Library/Application Support" is a more common place. There's some info here: https://developer.apple.com/library/content/documentation/General/Conceptual/MOSXAppProgrammingGuide/AppRuntime/AppRuntime.html#//apple_ref/doc/uid/TP40010543-CH2-SW9

epatpol commented 6 years ago

So would it be a good idea to support XDG for Linux/Windows as the paths seem to correspond with what most applications save their data/config stuff and do something else for Mac OS (Use ~/Library/Application Support)?

simark commented 6 years ago

We checked again on Windows, and it's the same. I returns .config and .local/share, even though it's not really the standard thing to do on Windows. Things should probably go in %USER%/AppData/something/something. There's probably an npm package that does all this already, I just didn't find it yet :)

Michael2MacDonald commented 1 year ago

This issue has not had an update in 5 years. Is it even still on the devs' radar and do they plan to implement this feature or not? This feature is important to me. I understand that sometimes devs are busy or GitHub issues are lost in time. Because of that, I just wanted to bring attention back to this issue in case it was forgotten and ask what is preventing it from being finished (not enough dev time, indecision on how it should be implemented, etc?).

Thanks in advance!

vince-fugnitto commented 1 year ago

@Michael2MacDonald I definitely think it is still relevant but hasn't had the highest priority and the fact that it is customizable by products built with the framework. We also at the time we followed the convention set forth by vscode. If interested please don't hesitate to contribute a fix, in vscode for instance they have a long standing issue related to it as well https://github.com/microsoft/vscode/issues/3884.

wc7086 commented 1 year ago

If interested please don't hesitate to contribute a fix.

The biggest challenge was coming up with a solution that would convince the development team.

Michael2MacDonald commented 1 year ago

Sorry in advance, and this is not targeted towards the Theia team. You guys have done nothing wrong. This is more towards the Adruino devs that have outright refuse to solve the issue in the past, VS Code, Electron, and Chromium.

It kind of looks to me like every is playing the blame game. The Arduino IDE team is saying that it is an upstream issue with theia, theia is saying that they follow VS-Code and VS-Code should fix it, VS-Code is saying that it is really an issue inherited from electron, elecron has the issue because of chromium, and the chromium devs just don't seem to care at all about it as far as I can tell.

And nearly everyone is claiming it would be a breaking change, despite the fact that all it would take is to check if the old folder exists, using it if it does, and using the XDG folder if it doesn't. Or just add an update script to move files to the new location.

It is clear to me that the issue is not that it is too difficult to implement, or that it would break peoples systems, or, or, or... The issue is that no one really wants to fix it.

People have made pull requests in many of these projects with the working code to solve the issue. It is clear that the project maintainers just don't care about this issue and don't want to merge the fix.

I am sorry. I don't want to be rude, but it is clear that this is a feature everyone wants, it is a standard that is set in place by all three major operating system, and it is a function that is required for nearly every app that has configuration or needs cache. So I don't undersatnd why there is not a standard implementation or library and why every app does not just put files in the correct location to begin with.

Again, I am sorry for being rude. It just seems to me like someone, anyone, needs to make the fix. Then we can work on merging it upstream/downstream.

I have followed issue after issue and pull-request after pull-request from many projects, and they all just link to another project's pull request for the same issue from 4-6 years ago. Non of them merged.

I am going to refrain from any more comments from now on unless I have something useful or productive to say.

Thanks for all you guy's hard work and I hope you can solve this issue soon. --Michael MacDonald

msujew commented 1 year ago

@Michael2MacDonald Saying that Vince plays the blame game isn't really fair - he only mentioned that vscode (a project with a much larger contributor base) hasn't introduced this feature yet either. In the end, Theia is free for devs to use and effectively paid for by the contributor companies (mainly Ericsson, TypeFox, EclipseSource and ARM, among others), with each individual contributor somehow being obliged to represent the requirements of their company (at least as long as they work on the companies dime). This issue in particular just isn't a high priority for anyone of these contributors right now.

However, as Vince mentioned, PRs on this topic are still appreciated and will be reviewed by the team. I've added the appropriate labels to this issue.

MarkZ3 commented 1 year ago

@msujew you tagged the wrong person :)

marcdumais-work commented 1 year ago

Hi @Michael2MacDonald,

Thank you for providing context.

I do not see at first look that we depend on where vscode finds its user-level settings folder - we do not use that folder AFAIK. I agree with the Arduino team, that this is probably something that should be fixed in the Theia framework.

(update: I am taking only about where the default settings folder is assumed to be. The issue with Electron/Chromium allegedly not using the correct folders for some things, we really have no control over)

This issue's description is quite old - we should update it and also provide some pointers about how we think this should be done, the important related places in the code, etc. We need to also consider how to migrate settings and other files from the old Theia application user's folder, to the new one.

I think doing the above will increase the chances that someone will be willing to pick it up soon, and that the person will know what we expect in an eventual contribution.

I also want to mention that we have started to use reactions to issues. So, when an issue is of interest to you, you should up-vote it. We will regularly run a search to check for "forgotten" issues, that have several up-votes, and use this info to give them visibility and consider to prioritize them.

(this is part of our "community pulse" initiative, discussed last public dev-meeting ).

Maybe this issue here could become the first one to get the label "community pulse"? cc: @JonasHelming

Michael2MacDonald commented 1 year ago

Thank you for everyone's responses and for taking steps to boost the issue and make it more approachable to fix.

I don't disagree that the issue should be fixed in theia, not the arduino IDE, but I guess my biggest issue with them is that before they moved to IDE version 2.0 they outright refused to solve the issue in v1.8.

I think I must have misunderstood what @vince-fugnitto said about following conventions set by VS-Code. I believe I missed the part where that was the case when the issue was first opened, but is no longer the case.

You can probably tell how frustrated I am because it seems like such a simple issue but it is so widespread. I don't blame theia, but the issue in the VS Code repo has been getting constant comments so it is mind boggling that it has taken such a large project multiple years to make no progress. Theia is smaller and has not gotten any new attention on the issue until now, so I understand why no progress has been made.

I have not contributed to projects before, but I would like to learn and get better at communicating in a helpful and contributing why.

I have drafted an "Update" that I am going to comment here soon. It contains the current config directory behavior, the proposed new behavior, and some pseudo-code I wrote after peeking at the source code. I don't expect it to be anywhere near perfect or even the final solution, but hopefully it sparks more action and we can build off of it.

Michael2MacDonald commented 1 year ago

Update

This is the current state of the issue as far as I am aware. I have done some browsing of the code and wrote some really crappy pseudo-code to help get people thinking about the solution and to help find potential issues that will have to be considered when developing the final solution.

Current Behavior:

It currently checks for the THEIA_CONFIG_DIR env var, and uses it as the config directory if set. If it is not set, it uses $HOME/.theia (or %USER%/.theia for windows).

Current Implementation:

The code that actually sets the location of config directory (.theia): https://github.com/eclipse-theia/theia/blob/17e52694b18bc25a375801f635555374df41c992/packages/core/src/node/env-variables/env-variables-server.ts#L45-L47

Plugins just reference the config directory: https://github.com/eclipse-theia/theia/blob/17e52694b18bc25a375801f635555374df41c992/packages/plugin-ext/src/main/node/paths/plugin-paths-service.ts#L133-L141

UserStorageUri (eg. "user-storage:/user") also just points to the config directory: https://github.com/eclipse-theia/theia/blob/17e52694b18bc25a375801f635555374df41c992/packages/userstorage/src/browser/user-storage-contribution.ts#L47-L62

Proposed New Behavior:

It first checks for the THEIA_CONFIG_DIR env var and uses it if it is set. If not, it checks if the old locations exist ($HOME/.theia or %USER%/.theia). If not, it uses the following platform dependant rules:

Proposed New Implementation: (Note! This is pseudo-code. I have no idea how the source code works and I don't know JS.)

// File: theia/packages/core/src/node/env-variables/env-variables-server.ts

protected async createConfigDirUri(): Promise<string> {
    // Use user-set location via `$THEIA_CONFIG_DIR` env var
    if (process.env.THEIA_CONFIG_DIR) {
        return FileUri.create(process.env.THEIA_CONFIG_DIR).toString();
    }
    // Then use '$HOME/.theia' if it exists
    if ( FILE('$HOME/.theia').exists() ) {
        return FileUri.create(join(homedir(), '.theia')).toString();
    }
    // Finally, use 
    if (is_linux) {
        if (process.env.XDG_CONFIG_HOME) {
            return FileUri.create('$XDG_CONFIG_HOME/theia').toString();
        } else {
            return FileUri.create(join(homedir(), '.config/theia')).toString();
        }
    } else if (is_windows) {
        //return FileUri.create(join(homedir(), 'AppData/something/something')).toString();
        return FileUri.create(join(process.env.APPDATA, 'AppData/something/something')).toString();
    } else if (is_mac) {
        return FileUri.create(join(homedir(), 'Library/Application Support/something')).toString();
    }
}

It would be great if a dev with knowledge of the workings of Theia could point out any locations within the code that need to be changed, other than createConfigDirUri().

This proposal does not consider other locations such as $HOME/.local/share or $HOME/.cache. I am going to post more information about those later and compile a list of functions responsible for setting the location of other data/logs/etc that need to be changed to respect the proper directories.

ToDo:

marcdumais-work commented 1 year ago

@Michael2MacDonald Thank you so much for sharing your enhancement proposal.

I will note that, as a framework, Theia is neutral wr to where IDE developers chose to store their IDE's configs. It's totally their choice. However, I think Theia should offer the option of using standard folders, for those who want that option.

About your proposal, the approach you propose, to use the legacy config folder if present, is a valid and simple one. Another one would be to have some type of data migration, if the new config folder is enabled and a legacy folder is present. IDE developers could also extend and chose their own mechanism. I'm not disagreeing with your proposal.

@vince-fugnitto pointed-out to me this npm library we could use, that could help with picking the standard OS-specific folders for a few things (configs, caches, ...):

https://github.com/sindresorhus/env-paths

Michael2MacDonald commented 1 year ago

Thanks for your response. I am glad you brought up data migration. I did think about it as an option and I would love to hear from other people if they think that would be the better route to go.

As for allowing projects to maintain control of the config folders location, there is currently a mechanism for projects to override the config directory location. I would like to note that this mechanism overrides the entire filepath so if a project wanted to use the provided paths from theia, but change the name of the config folder (such is the case with the Arduino IDE), this would be impossible. They would have to use a static path or add their own code implementation to generate the correct directory paths.

I propose we add a way for projects to override the directory name separately from the default directory path so that they can use the provided OS specific paths with a custom directory name.

If a project would like to continue to use the current config directory path, we could either add an option to disable OS specific paths, or it could be up to the project to use the current override feature to set the path back to ~/.theia. This is a behavior that needs to be decided on.

I will take a look at that library because it seems promising!

marcdumais-work commented 1 year ago

Hi @Michael2MacDonald,

Thank you for the thoughtful follow-up. I will be responding separately. Would you be okay, with me assigning this issue to you? [*] No obligation for you to provide a PR, just that you continue to "drive the discussion", that will eventually lead to a nice solution. Maintainers would help you, if you want to try to implement the solution, once we narrow-down what's wanted.

[*]: if it's possible - I do not recall if GitHub permits to assign an issue to a non-member

marcdumais-work commented 1 year ago

Hi @Michael2MacDonald ,

I'm, not sure how much you know about making Theia-based applications and customizing them vs Vanilla Theia? I'll provide some info below that i think can help. Feel free to re-use anything that I mention, make it part of your design.

As for allowing projects to maintain control of the config folders location, there is currently a mechanism for projects to override the config directory location. I would like to note that this mechanism overrides the entire filepath

yes, pretty much. e.g. there are a few options, like by overriding createConfigDirUri() or getConfigDirUri(). But using these would leave each extender having to figure-out the standard paths for any supported Operating System. Not very efficient.

The current Theia API, though in-line with vscode, is a bit limited. Here are examples of how real Theia applications currently override the default <userhome>/.theia configuration folder:

Theia Blueprint sets it's own config folder, by overriding getConfigDirUri(): https://github.com/eclipse-theia/theia-blueprint/blob/ccfd46923fd11aa9d9595de1b59d471d99d2d5a5/theia-blueprint-product/src/node/theia-blueprint-variables-server.ts#L23-L32

Arduino IDE does it a bit differently, overriding configDirUri, but same end result. https://github.com/arduino/arduino-ide/blob/4c55807392b3b724e2c5912280c621f9ed8db5e6/arduino-ide-extension/src/node/theia/env-variables/env-variables-server.ts#L10-L13

Interestingly Arduino IDE uses a folder name that's defined in their App's package.json: https://github.com/arduino/arduino-ide/blob/097c92d904ca9b126e54b6a58030b5f4c650f075/electron-app/package.json#L66

For compatibility reason, we want to keep the above ways of customizing the config folder working the same. However we can propose new APIs that will offer more flexibility to Theia application authors, in how they pick their app's config folder, including the option to pick the default standard for a given OS.

Here's a quick example new API proposal, where I attempt to add a more flexible way of picking the config folder. It does not handle yet the "standardization" aspect. The code is mostly untested and this may not even be useful, other than to demonstrate a pattern that we can use to make APIs extensible/overridable: 

    /**
     * Legacy createConfigDirUri(). Will provide a bad default config directory, for anything more than
     * an example application.
     * Extenders can override this with their own implementation
     * @returns Promise<string> config directory for the app
     */
    protected async createConfigDirUri(): Promise<string> {
        return FileUri.create(process.env.THEIA_CONFIG_DIR || join(homedir(), '.theia')).toString();
    }

    /**
     * More customizable version of createConfigDirUri(). Extenders can use this version and
     * then override _doCreateCustomConfigDirUri() and/or _doCreateConfigDirUriBaseFolder()
     * as suits their needs.
     * @since 1.38.0
     * @experimental
     */
    protected async createCustomConfigDirUri(): Promise<string> {
        return FileUri.create(process.env.THEIA_CONFIG_DIR || await this._doCreateCustomConfigDirUri()).toString();
    }

    /**
     * Extenders can override this to specify their own product's config directory
     * The default implementation delegates picking the base folder to _doCreateConfigDirUriBaseFolder()
     * and uses the ".theia" folder under it.
     * @since 1.38.0
     * @experimental
     */
    async _doCreateCustomConfigDirUri(): Promise<string> {
        return join(await this._doCreateConfigDirUriBaseFolder(), '.theia');
        // TODO: Use product name from app's `package.json` if defined and ".theia" only as fall-back?
    }

    /**
     * Extenders can override this to specify a different base folder for the  config directory.
     * The default implementation uses the user's home folder
     * @since 1.38.0
     * @experimental
     */
    async _doCreateConfigDirUriBaseFolder(): Promise<string> {
        return homedir();
    }

diff --git a/packages/core/src/node/env-variables/env-variables-server.ts b/packages/core/src/node/env-variables/env-variables-server.ts
index 37e3aef26e9..61ffea9e6d1 100644
--- a/packages/core/src/node/env-variables/env-variables-server.ts
+++ b/packages/core/src/node/env-variables/env-variables-server.ts
@@ -42,10 +42,49 @@ export class EnvVariablesServerImpl implements EnvVariablesServer {
         });
     }

+    /**
+     * Legacy createConfigDirUri(). Will provide a bad default config directory, for anything more than
+     * an example application.
+     * Extenders can override this with their own implementation
+     * @returns Promise<string> config directory for the app
+     */
     protected async createConfigDirUri(): Promise<string> {
         return FileUri.create(process.env.THEIA_CONFIG_DIR || join(homedir(), '.theia')).toString();
     }

+    /**
+     * More customizable version of createConfigDirUri(). Extenders can use this version and
+     * then override _doCreateCustomConfigDirUri() and/or _doCreateConfigDirUriBaseFolder()
+     * as suits their needs.
+     * @since 1.38.0
+     * @experimental
+     */
+    protected async createCustomConfigDirUri(): Promise<string> {
+        return FileUri.create(process.env.THEIA_CONFIG_DIR || await this._doCreateCustomConfigDirUri()).toString();
+    }
+
+    /**
+     * Extenders can override this to specify their own product's config directory
+     * The default implementation delegates picking the base folder to _doCreateConfigDirUriBaseFolder()
+     * and uses the ".theia" folder under it.
+     * @since 1.38.0
+     * @experimental
+     */
+    async _doCreateCustomConfigDirUri(): Promise<string> {
+        return join(await this._doCreateConfigDirUriBaseFolder(), '.theia');
+        // TODO: Use product name from app's `package.json` if defined and ".theia" only as fall-back?
+    }
+
+    /**
+     * Extenders can override this to specify a different base folder for the  config directory.
+     * The default implementation uses the user's home folder
+     * @since 1.38.0
+     * @experimental
+     */
+    async _doCreateConfigDirUriBaseFolder(): Promise<string> {
+        return homedir();
+    }
+
     async getExecPath(): Promise<string> {
         return process.execPath;
     }
marcdumais-work commented 1 year ago

cc: @kittaakos (Arduino IDE maintainer)

TL;DR: We are currently "spit-balling" about how to offer support in Theia, to help apps such as Arduino IDE and Theia Blueprint, to select a standard, OS-appropriate, configuration folder (if/when they wish - there could be valid migration concerns to address first). We could welcome follow-up contributions, to similarly help with other types of folders (cache, ...). We aim to keep the current way of customizing the config folder working the same, so you and other adopters will not have a surprise folder migration.

We already know it will not be perfect, since Electron-bundled Chromium allegedly does not fully respect the "standard", of splitting "cache" vs "config" content in different folders. Instead it apparently uses the config folder to store cache content too. Allegedly, there is no sign of immediate intention to do differently. One complain about this is that the config folder is often backed-up, and adding cache content will make the backups bigger for no good reason. Using the proper config folder in Theia apps, will likely result in bigger yet backups (but hopefully justifiably - one's IDE configuration can be valuable). The content that could tend to become bigger in that folder is the user-installed vscode extensions, which may not be a concern for Arduino IDE?.

Michael2MacDonald commented 1 year ago

Would you be okay, with me assigning this issue to you? [*] No obligation for you to provide a PR, just that you continue to "drive the discussion", that will eventually lead to a nice solution. Maintainers would help you, if you want to try to implement the solution, once we narrow-down what's wanted.

I am ok with that, but I don't think I can implement the solution so I would just drive discussion and suggest possible implementations.

Michael2MacDonald commented 1 year ago

Here's a quick example new API proposal

That looks good. There is one issue we will have to address when we combine that with the OS specific path generation.

The issue is the dot (.) at the beginning of the directory name. The dot should be there if the config folder is in certain directories but should not be used in others.

If the project sets the directory name but leaves the path to be automatically generated depending on the OS, then we will have to make sure that it also adds/removes the dot depending on that directory.

The opposite is also an issue. If the project sets the directory location but lets theia set the name, then there needs to be a way for the project to add the dot if the directory they specify requires it. In this case, we could provide a flag or function that adds the dot, or we could make sure that the dot can be added directly to the directory path by the project.

kittaakos commented 1 year ago

Here's a quick example new API proposal,

Thank you for the naming and the new API proposal, @marcdumais-work 👍

I would rather see the config folder provider as a standalone service injected into the envVariable server. Downstream, we had issues with the current default envVariable server implementation. Please ping me on the PR when there is a concrete plan with the new API; we can gradually tweak it to support XDG_CONFIG_HOME, etc. fully.

marcdumais-work commented 1 year ago

The issue is the dot (.) at the beginning of the directory name. The dot should be there if the config folder is in certain directories but should not be used in others.

My understanding is that files/folders starting with a dot are considered hidden on Linux/Unix. They will not be shown by default using ls unless one uses the -a option to "see all". In popular file explorer Nautilus, dot-files/folders are not shown by default. I do not know whether the dot means the same on Windows. In any case, the reason for the dot in the default folder /home/<user>/.theia is the same as for .vscode, I think: to partially mitigate the cluttering of the user's home folder. Looking at the content of my ~/.config, I see no dotted-folders entries in it.

Do you see others cases, than the user's home folder, where the dot should be used? Are there OS-specific considerations?

paul-marechal commented 1 year ago

I would rather see the config folder provider as a standalone service injected into the envVariable server. Downstream, we had issues with the current default envVariable server implementation.

I just had a quick look at the EnvVariablesServer interface to see what could cause issues, and I don't get why the environment variables server does stuff with exec path or drives or config directories???

We're most likely missing a proper abstracting that would make use of environment variables, but I feel queezy about having the environment variables server depend on some folder provider service? A quick refactoring to properly split service responsibilities might be in order?

Michael2MacDonald commented 1 year ago

Looking at the content of my ~/.config, I see no dotted-folders entries in it.

That is kind of my point. Sorry if I wasn't clear. The dot marks a folder or file as hidden on linux (and I think all of unix?). It is important to hide files in the home directory to prevent clutter, but if it is in an already hidden directory or a directory not readily exposed to the user, it should not be hidden. For example, .config is already hidden so files in it should not be hidden. Files in a directory like /usr/local should also not be hidden (this does not apply to us though).

So we just need to take that into consideration so that the directory is properly hidden/not-hidden depending on where it is located.

marcdumais-work commented 1 year ago

A quick refactoring to properly split service responsibilities might be in order?

Maybe. It could be a good opportunity to mark the current configuration-folder-related stuff in the environment server as deprecated[*] and propose a new dedicated service that cares only about the app's configuration directory.

[*] if I remember correctly how we are supposed to handle Theia APIs, what we mark as deprecated would remain usable/maintained for the foreseeable future, but perhaps eventually removed in Theia v3.x or later.