-
Notifications
You must be signed in to change notification settings - Fork 222
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
System calls such as and fail if encrypted #283
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
1 similar comment
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
1 similar comment
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For tests, take a look at:
I suspect some of them will need to be updated by this change.
Run bazel test -c opt ...
to confirm.
Feel free to add more that exercise system function calls, including a few standard ones.
You may mark in your commit message:
|
@googlebot I signed it! |
@googlebot I fixed it. |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
1 similar comment
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting a fix so quickly!
1. This is a breaking change for the following servers, which will have to be reinstalled: - ltex - clangd 2. This is a breaking change for users who reach into the default options (for example via server:get_default_options()) to access the `cmd` property. nvim-lsp-installer no longer provides the `cmd` (except in a few instances), but instead provides an amended PATH which allows neovim's LSP client to locate the locally installed executable. To access the `cmd`, simply access it via lspconfig instead, for example like so: local default_config = require("lspconfig.server_configurations.rust_analyzer").default_config print("I can now access the cmd governed by lspconfig:", default_config.cmd) 3. This is a breaking change for 3rd party use cases that makes use of the `executable()` APIs (e.g., `npm.executable(root_dir, "tsserver")`). The recommended usage is to instead to use the canonical name of the command ("tsserver"), while providing an amended PATH, for example: local npm = require("nvim-lsp-installer.installers.npm") local server = server.Server:new { ..., root_dir = root_dir, installer = npm.packages { "tsserver" }, default_options = { cmd = { "tsserver" }, cmd_env = npm.env(root_dir), } }
This patch recognizes a SystemTFIdentifier and turns off encryption for that token. Otherwise the obfuscated Verilog cannot be compiled if it contains such calls.
I await further guidance on testcase integration.