Skip to content

Respect LD_LIBRARY_PATH when forking on Linux #545

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

Merged
merged 1 commit into from
Apr 29, 2019

Conversation

TheMozg
Copy link
Contributor

@TheMozg TheMozg commented Apr 19, 2019

Describe in detail the problem you had and how this PR fixes it

Currently there is no easy way to run code-server if disto's "stock" libstdc++ package is too old because code-server does not respect LD_LIBRARY_PATH on Linux.

Even if I set LD_LIBRARY_PATH I get this error:

...
INFO  Starting shared process [1/5]...
WARN  stderr {"data":"/export/home/fedor.kalugin/cli-linux-x64: /lib64/libstdc++.so.6: version `GLIBCXX_3.4.20' not found (required by /export/home/fedor.kalugin/cli-linux-x64)\n/export/home/fedor.kalugin/cli-linux-x64: /lib64/libstdc++.so.6: version `GLIBCXX_3.4.21' not found (required
by /export/home/fedor.kalugin/cli-linux-x64)\n/export/home/fedor.kalugin/cli-linux-x64: /lib64/libstdc++.so.6: version `CXXABI_1.3.9' not found
(required by /export/home/fedor.kalugin/cli-linux-x64)\n"}
INFO  Starting shared process [2/5]... {"error":"Exited with 1"}
...

In some environments there is newer libstdc++ available that isn't installed to default location. This Pull Request makes it possible to use custom libstdc++.

Is there an open issue you can link to?

#347
#239

Copy link
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, interesting, normally when something forks we preserve the environment: https://github.com/codercom/code-server/blob/5f40ebb8457380aa1e9955302d9e41a6b042ef14/packages/protocol/src/node/modules/child_process.ts#L86

But, since the shared process is forked directly it doesn't get the environment preserved. I think it might make sense to edit: https://github.com/codercom/code-server/blob/5f40ebb8457380aa1e9955302d9e41a6b042ef14/packages/server/src/vscode/sharedProcess.ts#L91

Then import preserveEnv from @coder/protocol and call it there before forking the shared process.

@TheMozg TheMozg force-pushed the respect-ld-library-path branch from a28784b to 79f8fd1 Compare April 28, 2019 13:50
@TheMozg TheMozg force-pushed the respect-ld-library-path branch from 79f8fd1 to 5149ad1 Compare April 28, 2019 13:55
@TheMozg
Copy link
Contributor Author

TheMozg commented Apr 28, 2019

Updated the PR to use preserveEnv, also used it for --install-extension option.

Copy link
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic!

@code-asher code-asher merged commit 1622fd4 into coder:master Apr 29, 2019
andreimc added a commit to devonlineco/code-server that referenced this pull request May 14, 2019
* upstream/master: (68 commits)
  Fix docker oneliner in README.md
  Fix macOS release
  Rename codercom/code-server to cdr/code-server
  Expose Terminal Service in API
  Make preserveEnv return a new object
  Preserve environment when forking shared process (coder#545)
  codercom -> cdr
  Package only on darwin
  Include version with target env
  Add docker service
  Add support for musl and centos
  Remove reveal in finder/explorer option from the context menu (coder#586)
  Fix open dialog crash when there is a broken link
  Only output password if it was generated
  Fix full screen detection for Chromium
  Fix toggling full screen
  Improve size column in dialogs
  Fix typo DigitalOcean (coder#595)
  Fix coder#587 (coder#588)
  Fix protocol fs test
  ...
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