-
Notifications
You must be signed in to change notification settings - Fork 176
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
chore(compass): explicitly set secret store to gnome-libsecret on linux if not set #5958
base: main
Are you sure you want to change the base?
Conversation
private static setupJavaScriptArguments(): void { | ||
// For Linux users with drivers that are avoided by Chromium we disable the | ||
// GPU check to attempt to bypass the disabled WebGL settings. | ||
app.commandLine.appendSwitch('ignore-gpu-blacklist', 'true'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like it's something we want to set as early as possible, for consistency moved this and the new appendSwitch
call to the setup-* file that is the first thing that runs in the app
* @see {@link https://github.com/microsoft/vscode/issues/185212#issuecomment-1593271415} | ||
*/ | ||
if (app.commandLine.hasSwitch('password-store') === false) { | ||
app.commandLine.appendSwitch('password-store', 'gnome-libsecret'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any other factors we should be taking into account when setting this default, e.g. specific values of $XDG_CURRENT_DESKTOP or whether the gnome-keyring
binary is in PATH/whether a libsecret.so
is available on the current system (I think a relatively easy way to check the latter would be to see which error gets thrown by process.dlopen({exports: {}}, 'libsecret-1.so.0')
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point! I was thinking about checking that XDG_ var is not in the "supported" list, but it seemed like a mess to maintain. I like the idea of checking for libsecret!
One thing I want to do first is to check what exactly happens when you set this in the environment where it's not supported. If it's not different from not setting it at all I think, just from the perspective of having less logic to maintain, I would prefer not to do any extra checks, but please tell me if this doesn't sound right to you 🙂
No idea if this is a bad idea to do so, but as we have gnome-keyring in the dependencies for all linux installers, maybe we can force the password store to work around the autodetection logic that causes issues to the users. Probably would not make it worse?