cquery: init at 2018-03-25 #37890

tobim · 2018-03-26T20:20:36Z

Motivation for this change

Add the cquery language server

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
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
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.

Mic92 · 2018-03-26T21:47:13Z

pkgs/development/tools/misc/cquery/default.nix

+    args+=", \"-isystem\", \"${llvmPackages.libcxx}/include/c++/v1\""
+
+    wrapProgram $out/bin/cquery \
+      --add-flags "'"'--init={"extraClangArguments": ['"''${args}"']}'"'"


What happens if --init is specified twice?

It gets overridden if it is duplicated, but probably test best we can do without custom patches. NIX_CFLAGS_COMPILE/$NIX_CXXSTDLIB_COMPILE might be also interesting here?

Sorry I did not test for that. I think it would be best to patch that upstream. I could use jd to merge the options in the meantime, but that would be pretty hacky...

NIX_CFLAGS_COMPILE contains thing that are only needed during build time, so I don't think that should be used.
See my other reply regarding NIX_CXXSTDLIB_COMPILE.

Btw, thank you for taking a look.

I mean NIX_CFLAGS_COMPILE could be read at run-time because then developing in a nix shell would just works.

I see. I'd still like it to be usable outside of nix-shell, though.
Also, reading NIX_CFLAGS_COMPILE at runtime would introduce platform dependent behaviour, and I would rather not provide people with an excuse to avoid creating a compile_commands.json file.

The point is that on nix compile_command.json is not enough to find all includes.
To give you an example. If you install zlib1g-dev on ubuntu it will be installed into /usr/include.
gcc/libclang will search there for includes and find it during compilation.
If you do nix-shell -p zlib in a project, cc-wrapper from nixpkgs will add -isystem /nix/store/spxwrxnsf28sq5gdp42xvqac7psvlqya-zlib-1.2.11-dev/include -isystem /nix/store/spxwrxnsf28sq5gdp42xvqac7psvlqya-zlib-1.2.11-dev/include to $NIX_CFLAGS_COMPILE and the compiler wrapper around gcc/clang will pick the same variable up.
In both versions the build system don't need to explicitly pass an include argument and they don't do that. So if cquery does not replicate the same behaviour it will be not very useful except for basic c/c++ project depending on the standard library. Platform specific behavior is fine here. That's the point of packaging.

I would argue that if a compile_commands.json does not contain include paths to zlib if its header is included in your compilation unit, then the compilation database is wrong.
Even though the path to /usr/include is added automatically on other systems does not make it right.
There are better ways to do this, for example cmake solves this through CMAKE_MODULES_PATH and the findXXX.cmake modules.
I must admit that classic ./configure; make; make install style build systems can't easily do that, and getting those dependencies from the NIX_CFLAGS_COMPILE field would be a viable workaround.

But no real-life project does this correct completely. Take a look at neovim, systemd or llvm - these where just the first random three project I checked. Usually most projects just check if a header file can be found, but do not bother to add the full path - this would require either a cmake module or a pkgconfig file. Not every library/distribution provides this, sometimes they are even broken, that's why many projects add fallbacks to their build system.

Mic92 · 2018-03-26T21:50:24Z

pkgs/development/tools/misc/cquery/default.nix

+  src = fetchFromGitHub {
+    owner = "cquery-project";
+    repo = "cquery";
+    rev = "e45a9ebbb6d8bfaf8bf1a3135b6faa910afea37e";


How about a released version? https://github.com/cquery-project/cquery/releases

The latest release version has a few bugs that have since been fixed. This revision works better (at least in the code bases I tested it with).

ok. releases as it seems progress fast here anyway.

Mic92 · 2018-03-26T23:24:46Z

pkgs/development/tools/misc/cquery/default.nix

+    # We need to tell cquery where to find the standard library headers.
+
+    args="\"-isystem\", \"${if (stdenv.hostPlatform.libc == "glibc") then stdenv.cc.libc.dev else stdenv.cc.libc}/include\""
+    args+=", \"-isystem\", \"${llvmPackages.libcxx}/include/c++/v1\""


if llvmPackages.libcxx is in buildInputs, $NIX_CXXSTDLIB_COMPILE` should be used instead.

I tried $NIX_CXXSTDLIB_COMPILE at first, but that starts with the paths to libstdc++ if we don't force libcxxStdenv. In my tests libclang seems to have a few problems with libstdc++, it works flawlessly with libc++.

Use NIX_CFLAGS_COMPILE to find additional include paths.

Mic92 · 2018-03-29T19:50:40Z

Thanks!

Mic92 · 2018-04-26T10:45:30Z

seems to not build on darwin: https://hydra.nixos.org/build/72710712/nixlog/1

A patch has been provided upstream: jacobdufault/cquery@4f773f4
but upgrading to the latest commits seems to break the tests on linux:

running install tests
/nix/store/9zv4i1yv2y14r52y6r4fqzwhjlr1qsan-source /build/source/build
(   0.003s) [stdin        ]            lsp.cc:166   | Failed to read JsonRpc input; exiting
builder for '/nix/store/bfycja4icdhxf74gsmsqfq1fr3cha8pa-cquery-2018-04-24.drv' failed with exit code 1
error: build of '/nix/store/bfycja4icdhxf74gsmsqfq1fr3cha8pa-cquery-2018-04-24.drv' failed

tobim · 2018-05-01T18:53:45Z

opened a new PR: #39825

cquery: init at 2018-03-25

4930caa

GrahamcOfBorg added 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Mar 26, 2018

Mic92 reviewed Mar 26, 2018

View reviewed changes

cquery: extended wrapper

04f3b76

Use NIX_CFLAGS_COMPILE to find additional include paths.

Mic92 merged commit 683a73f into NixOS:master Mar 29, 2018

tobim deleted the cquery branch March 30, 2018 21:10

Janik-Haag added the 12. first-time contribution label Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cquery: init at 2018-03-25 #37890

cquery: init at 2018-03-25 #37890

tobim commented Mar 26, 2018 •

edited

Mic92 Mar 26, 2018

Mic92 Mar 26, 2018 •

edited

tobim Mar 27, 2018

Mic92 Mar 27, 2018

tobim Mar 27, 2018

Mic92 Mar 27, 2018 •

edited

tobim Mar 27, 2018

Mic92 Mar 28, 2018

Mic92 Mar 26, 2018

tobim Mar 27, 2018

Mic92 Mar 27, 2018

Mic92 Mar 26, 2018

tobim Mar 27, 2018

Mic92 Mar 27, 2018

Mic92 commented Mar 29, 2018

Mic92 commented Apr 26, 2018

tobim commented May 1, 2018

cquery: init at 2018-03-25 #37890

cquery: init at 2018-03-25 #37890

Conversation

tobim commented Mar 26, 2018 • edited

Motivation for this change

Things done

Choose a reason for hiding this comment

Mic92 Mar 26, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 Mar 27, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 commented Mar 29, 2018

Mic92 commented Apr 26, 2018

tobim commented May 1, 2018

tobim commented Mar 26, 2018 •

edited

Mic92 Mar 26, 2018 •

edited

Mic92 Mar 27, 2018 •

edited