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

chore(compass): explicitly set secret store to gnome-libsecret on linux if not set #5958

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gribnoysup
Copy link
Collaborator

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?

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');
Copy link
Collaborator Author

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');
Copy link
Collaborator

@addaleax addaleax Jun 21, 2024

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'))?

Copy link
Collaborator Author

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 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants