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

ycmd: 2016-01-12 -> 2017-02-03 #22463

Closed
wants to merge 1 commit into from
Closed

ycmd: 2016-01-12 -> 2017-02-03 #22463

wants to merge 1 commit into from

Conversation

d-xo
Copy link
Contributor

@d-xo d-xo commented Feb 5, 2017

Motivation for this change

Updates to latest version. Fixes #14349

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@LnL7
Copy link
Member

LnL7 commented Feb 5, 2017

I didn't know we have this, I feel like should vimPlugins.youcompleteme use it.

@rasendubi
Copy link
Member

This PR makes ycmd copy all its third party dependencies to the output. This includes a couple of python packages as well as source code for racerd, gocode, godef, and OmniSharp. (This adds additional 42M to the package size.) Is there any good reason for doing so?

I'm also not sure how this fixes #14349, because python scripts are still wrapped in bash scripts.

@cstrahan
Copy link
Contributor

cstrahan commented Feb 6, 2017

The majority of the third_party disk util is OmniSharpServer:

   26.2 MiB [##########] /OmniSharpServer
    3.9 MiB [#         ] /JediHTTP
    3.8 MiB [#         ] /python-future
    1.9 MiB [          ] /requests
    1.0 MiB [          ] /bottle
  844.0 KiB [          ] /gocode
  564.0 KiB [          ] /godef
  564.0 KiB [          ] /waitress
  380.0 KiB [          ] /argparse
  184.0 KiB [          ] /racerd
   28.0 KiB [          ] /frozendict
    4.0 KiB [          ] /tern_runtime

I almost wouldn't be opposed to vendoring some of the other deps though. In the PR I opened above, I think I'll probably just vendor JediHTTP, since it's not a legitimate python package (doesn't have a setup.py, and they're not likely to keep up to date with breaking changes in their deps, since they too vendor stuff...). With all that said, might make sense to use ycmd's vendored python deps too, but I've decided to stick with using our packaged versions for now.

@d-xo
Copy link
Contributor Author

d-xo commented Feb 6, 2017

Yeah this was rushed. Apologies.

Declining in favor of the PR from @cstrahan.

As a side note, @rasendubi the __main__.py output from this expression is definitely not wrapped in a bash script (or at least the one installed into my ~/.nix-profile/bin), but the path is also no longer modified.

@d-xo d-xo closed this Feb 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants