code-server icon indicating copy to clipboard operation
code-server copied to clipboard

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

Open edvincent opened this issue 2 years ago • 3 comments

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 https://github.com/coder/code-server/pull/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 https://github.com/coder/code-server/pull/5040/commits/4ae5e26ce7ed96a31c92211fc3026e1403725e3f, 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 avatar Sep 06 '22 16:09 edvincent

Thanks for bringing this up @edvincent 🌮

My thinking:

  1. Bring back the "hack" that was removed in https://github.com/coder/code-server/commit/4ae5e26ce7ed96a31c92211fc3026e1403725e3f, 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?

jsjoeio avatar Sep 06 '22 16:09 jsjoeio

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?

edvincent avatar Sep 06 '22 17:09 edvincent

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 avatar Sep 06 '22 20:09 jsjoeio