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

core: enable portable-mode #12690

Merged
merged 8 commits into from
Nov 17, 2023
Merged

Conversation

vladarama
Copy link
Contributor

Portable Mode in Theia

Creating a data folder at the root of a Theia application will make it portable. So all of the app data and user data will get written to the data folder instead of their default locations.

The .theiafolder is moved from the user’s home directory (%userprofile% or ~/) to a folder called user-data inside the data folder. user-data contains all of the settings, preferences and extensions.

In addition, all of the application data which is stored by default inside a Theia App Name (Theia Electron Example)folder in (%appdata% or ~/.config ) is moved to a folder called app-data inside the data folder. app-data contains all of the application’s cache and layout information.

Please refer to the following PR to further understand how the Portable mode is implemented in Theia Blueprint:
eclipse-theia/theia-blueprint#276

How to test

The following changes should make the user-data portable (settings, preferences, recent workspace ...).
It also makes the app-data portable so all of the electron cache and layout information will be stored in data/app-data instead of the default location.

You can test that the portable mode is functional using the Electron Example Application.

  • Run yarn and yarn electron build
  • Create a data folder inside /theia/examples/electron.
  • Run yarn electron start.

All of the user-data which is stored by default inside the user's directory will be stored inside of /theia/examples/electron/data/user-data. In addition, all of the app-data which is stored by default in ~/.config/Theia Electron Example or %appdata%/Theia Electron Example will be stored inside of /theia/examples/electron/data/app-data.

Review checklist

Reminder for reviewers

Copy link
Contributor Author

@vladarama vladarama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@msujew
Thank you for the review. I have changed all of the fs methods to their asynchronous version which makes more sense. However, I am running into a few issues I would like your thoughts on. Even after calling await this.makePortable() at the beginning of the start method in electron-main-application.ts, the Local State file gets written to the appdata under Theia Electron Example instead of being written to the data folder. Everything else seems to work fine, so I assume we are running into a race condition where Theia Electron Example gets created and written to before making the electron application portable. I will keep digging into it, but do you have any idea why this might be happening ?

@msujew
Copy link
Member

msujew commented Sep 25, 2023

@vladarama I see, I wasn't aware of that. I assume that the electron.app fires it's ready event while we await the fs.pathExists results, which then automatically starts writing stuff. Using sync calls of course prevents this event from firing until we're done.

I'm fine with changing the isPortable stuff back to sync functions. I was mostly concerned about the non-cached sync calls in env-variables-server.ts. This could've possibly led to performance regressions.

@JonasHelming
Copy link
Contributor

@vladarama This is ready for a re-review, correct?

@vladarama
Copy link
Contributor Author

@JonasHelming
Yes, this PR should be ready for another review.

This commit aims to address the following review comments:
- Switching `fs` methods from synchronous to asynchronous
- Caching the `configDirUri`
- Avoid using process.cwd() as the application path

Issues: switching from sync to async fs methods possibly leads to a race
condition where some files get written to the `appdata` before we have
the chance to make that folder portable. TO FIX !!!

Signed-off-by: Vlad Arama <[email protected]>
This commit reverts the changes from the previous commit to avoid
running into a race condition where the electron app fires its `ready`
event before we can make the application portable.

Signed-off-by: Vlad Arama <[email protected]>
This commit caches the pathExistence of the data folder, the user
folder.
This commit caches the pathExistence of the data folder and user data
folder in order to avoid accessing the disk on every call of
`createConfigDirUri`. This should prevent possible performance
regressions.

Signed-off-by: Vlad Arama <[email protected]>
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks looks good to me 👍

@msujew msujew merged commit bf93b29 into eclipse-theia:master Nov 17, 2023
13 checks passed
@vince-fugnitto vince-fugnitto added this to the 1.44.0 milestone Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants