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

[Fix]: Figure out next steps with keytar/libsecret #5536

Open
edvincent opened this issue Sep 6, 2022 · 3 comments
Open

[Fix]: Figure out next steps with keytar/libsecret #5536

edvincent opened this issue Sep 6, 2022 · 3 comments
Labels
enhancement Some improvement that isn't a feature
Milestone

Comments

@edvincent
Copy link
Contributor

Why do you want this feature?

Right now, following the guide from https://coder.com/docs/code-server/latest/npm, it doesn't specify that libsecret needs to be installed - which can cause confusions and issues as seen in #5533

libsecret is needed because keytar is a dependency of vscode - and it's used to store secrets.

HOWEVER, there's a fallback if it can't store creds in the OS store (what libsecret is used for) - that uses memory. It shows a log message in the console, but code-server (and vscode) are fully functional without it, thanks to that fallback built-in into vscode.

See https://github.com/microsoft/vscode/blob/1fb94816f2d40bdc77536eac44eada91a9a9e8ac/src/vs/platform/credentials/node/credentialsMainService.ts#L44-L48 for that catch that means there's a fallback.

What is your suggestion?

Need to figure out/agree what is the state of the need for libsecret when running code-server. Some ideas:

  • Try to get vscode to add keytar as an optionalDependency, as that's what it effectively is
  • Bring back the "hack" that was removed in 4ae5e26, which moves it to an optionalDependency on our side
  • Update the install doc to make libsecret part of the packages installed - https://coder.com/docs/code-server/latest/npm

Are you interested in submitting a PR for this?

Yes

@edvincent edvincent added the enhancement Some improvement that isn't a feature label Sep 6, 2022
@jsjoeio jsjoeio added this to the September 2022 milestone Sep 6, 2022
@jsjoeio
Copy link
Contributor

jsjoeio commented Sep 6, 2022

Thanks for bringing this up @edvincent 🌮

My thinking:

  1. Bring back the "hack" that was removed in 4ae5e26, which moves it to an optionalDependency on our side
  2. Try to get vscode to add keytar as an optionalDependency, as that's what it effectively is

If we did this 3, wouldn't be necessary right?

@code-asher what are your thoughts?

@edvincent
Copy link
Contributor Author

Correct - I'm all for the most lightweight install for the basic features, so not forcing libsecret is a bit better.

Ideally vscode would do the optionalDependency upstream, but I'm not sure we can convince them too... Hence the "hack".

What I can propose is to send them a PR explaining the rationale, and depending on the outcome there, bring the hack or not?

Unless we want to fix it quickly in here (before we bring the next vscode release), and want to bring back the hack in parallel?

@jsjoeio
Copy link
Contributor

jsjoeio commented Sep 6, 2022

I think working in parallel - sending PR upstream and adding our hack back in the meantime - seems like the best approach 👍🏼 Let's go with that!

@jsjoeio jsjoeio modified the milestones: September 2022, October 2022 Sep 30, 2022
@jsjoeio jsjoeio modified the milestones: October 2022, On Deck Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Some improvement that isn't a feature
Projects
None yet
Development

No branches or pull requests

2 participants