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

code-settings-sync extension cannot install extensions when using linux build #72

Open
ganapativs opened this Issue Jan 3, 2019 · 12 comments

Comments

Projects
None yet
3 participants
@ganapativs
Copy link

ganapativs commented Jan 3, 2019

code-settings-sync extension cannot install extensions when using linux build. Tested on latest Ubuntu.

Steps to reproduce:

  • Install latest linux build(either amd64 or x64)
  • Try to sync extensions using latest code-settings-sync extension
  • Extension installation fails with following error
"bin/code" --install-extension stevencl.addDocComments
Error: Command failed: "bin/code" --install-extension stevencl.addDocComments
/bin/sh: 1: bin/code: not found

Reason:

  • MacOS and Windows(probably) have bin/code file inside installation path, whereas linux builds have bin/vscodium file
  • code-settings-sync extension resolves bin/code binary inside installation folder(source)

Why does it resolves to bin/code?
Looks like issue with this line.

Eg: When debian build of VSCodium is installed, the vscodium binary path is resolved to /usr/share/vscodium/vscodium(initial myExt in above source) and then it resets it to bin/code.

I see that VSCodium support was merged to code-settings-sync here, this works fine. but this PR doesn't cover linux path resolution fix while installing extensions.

Does this issue belongs here or should I raise the issue in code-settings-sync repo?

@ganapativs

This comment has been minimized.

Copy link

ganapativs commented Jan 3, 2019

@paulcarroty Yes!

Looks like there are some differences in the way the VSCodium binary is exposed, like VSCodium build of MacOS has bin/code file inside installation path, whereas linux build has bin/vscodium file. Is this expected?

Also the code-settings-sync needs to be fixed to support the same. but how to detect if extension is running inside VSCodium?

If somebody can help with it, I can create a PR to fix it in code-settings-sync.

@paulcarroty

This comment has been minimized.

Copy link
Contributor

paulcarroty commented Jan 3, 2019

@ganapativs better move this issue to code-settings-sync repo.

I tried to make the bin/code symlink, but it doesn't fix the issue for Fedora 29.

@ganapativs

This comment has been minimized.

Copy link

ganapativs commented Jan 3, 2019

@paulcarroty I'll also raise the issue in code-settings-sync repo, but it feels like it's also relevant to discuss it here.

@stripedpajamas

This comment has been minimized.

Copy link
Contributor

stripedpajamas commented Jan 3, 2019

Hi @ganapativs! Thank you for this really well documented issue, that is very helpful. You might have already said this and I missed it, but does this extension work when using MS VS Code on Linux? Wondering if the extension just doesn't know how to handle the Linux installation of VS Code or if it has to do with something we changed during build.

@ganapativs

This comment has been minimized.

Copy link

ganapativs commented Jan 4, 2019

@stripedpajamas Yes. I'm using VSCode in Ubuntu right now. code-settings-sync just works fine.

Also migrating extension from VSCode to VSCodium should works for now in Linux. But this is a temporary fix.

Also as @paulcarroty suggested, should I raise the same issue in code-settings-sync repo?

@stripedpajamas

This comment has been minimized.

Copy link
Contributor

stripedpajamas commented Jan 4, 2019

I see.

Since that extension doesn't explicitly support VSCodium, a PR would probably go a long way further than an issue.

In MS Visual Studio Code, do you have bin/code inside the installation path?

@ganapativs

This comment has been minimized.

Copy link

ganapativs commented Jan 5, 2019

Yes, VSCode has bin/code.

I'll see if I can create a PR. But in the meantime if someone is familiarity with the code-settings-sync repo, please help :)

@stripedpajamas

This comment has been minimized.

Copy link
Contributor

stripedpajamas commented Jan 5, 2019

@ganapativs I think I found where MS is treating Linux/Windows differently than Mac:

https://github.com/Microsoft/vscode/blob/master/build/gulpfile.vscode.js#L408

It looks like for Linux and Windows, the bin/code.sh (which is the CLI file needed in code-settings-sync) is renamed to bin/${product.applicationName} which we have set to VSCodium. On Mac they've hardcoded the rename to bin/code (see this line).

So I think we could possibly edit the logic of this line pre-build to rename to bin/code in all cases instead of product.applicationName. This should get the extension working (and any other extensions that rely on the CLI being present in the usual place. Thoughts?

@ganapativs

This comment has been minimized.

Copy link

ganapativs commented Jan 6, 2019

@stripedpajamas Thats a great find 👏 But renaming binary to bin/code in all platforms would fix half of the problem.

The other half of the problem is in this line and this line. Which is probably specific to this extension.

myExt variable in source has VSCodium path by default(eg: /usr/share/vscodium or something, which doesn't have code substring in it). so line 324 always resets myExt to bin/code which is invalid command and extension breaks.

I'm not sure if code-settings-sync extension works fine in windows with VSCodium.

@paulcarroty

This comment has been minimized.

Copy link
Contributor

paulcarroty commented Jan 6, 2019

edit the logic of this line pre-build to rename to bin/code in all cases instead of product.applicationName

Agree. Hope we'll avoid side effects this time.

@stripedpajamas

This comment has been minimized.

Copy link
Contributor

stripedpajamas commented Jan 6, 2019

On second thought, we should try to minimize all changes to the vscode repo prebuild when possible. It makes more sense to PR code-settings-sync to look for product.applicationName when applicable.

@ganapativs

This comment has been minimized.

Copy link

ganapativs commented Jan 7, 2019

Hmmm. Either way should work fine. I'll spend some time on code-settings-sync codebase then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment