Feature/qt clang integration #61348

deadloko · 2019-05-12T08:05:16Z

Motivation for this change

Starting with version 4.7 clang code model is the default method for supporting QOL services as is:
code completion, syntactic and semantic highlighting, diagnostics with fixits and integrated Clang-Tidy and Clazy checks, follow symbol, outline of symbols, tooltips, renaming of local symbols.

For future releases of qtc clang code model support is esssential need.

Things done

How clang-tools and clang-code-model functionallity was tested

Create shell.nix:

with import <nixpkgs> {};

{  
    environment = stdenvNoCC.mkDerivation {
    name = "dev_environment";
    buildInputs = [ qt5.full qtcreator cmake gcc8 clang_8 gdb boost libpqxx xterm bashInteractive 
llvmPackages.libcxx];
  };
}

clang_8 is optional as qtcreator will still find it.

Clone test project
Launch nix-shell: nix-shell -I nixpkgs=../nixpkgs --pure --keep LD_LIBRARY_PATH
nixpkgs path is path to my nixpkgs fork with this changes.
Launch qtcreator: qtcreator, if debug of clang-model is needed: QT_LOGGING_RULES=qtc.clang*=true LIBCLANG_TIMING=1 qtcreator
In settings create 2 kits: 1 for clang and 1 for gcc,
Test that project is successfully building with this kits. And that after opening c++ source file there is no popups with errors.
Launch clang tools analysis (In top menu Analysis->Clang-Tidy and Clazy...->Analyze) and check that there is warnings from diagnostic and no errors in application output tab 11:40:47: Clang-Tidy and Clazy finished: Processed 3 files successfully, 0 failed..

Update:

Rebased and tested on 03 june 2019 - reason is qtcreator was updated to 4.9.1 in master.

Update2:

As @abbradar adviced, i removed all changes that was made to system clang, added qt vendor clang version as build dependency. After testing all looks good: clang code model working with both clang and gcc compilers set in project kit. Clang-tidy and clazy checks also works like they should.

deadloko · 2019-05-12T08:17:24Z

As i am still newbie to nix and nixos, i have some doubts:

Is it ok to depend on specific version of llvm libraries?
Is it ok to add depends for compiler or should i resolve qtc need for clang plugins in some other way?
What is preferred method to change package sources? Instead of substituteInPlace i may create yet another .patch file.
Is there a name convention for .patch files?

matthewbauer · 2019-05-12T17:02:00Z

pkgs/development/compilers/llvm/8/clang/clang_qtcreator_support.patch

@@ -0,0 +1,334 @@
+diff --git a/tools/driver/CMakeLists.txt b/tools/driver/CMakeLists.txt


Is there an upstream version of this patch? If possible it would be preferred to use fetchpatch to avoid checking this in.

Unfortunately no, qt maintain separate version of clang in their own git, and afaik deliver it through their installer package (binary .run file). It's is possible, however, to fetch patch directly from their tracker by link. Please let me know if i should do it this way.

Yeah that works

I tried using fetchpatch, but that fails (that diff is against older version of clang). I suggest leaving patch file as it is now.

matthewbauer · 2019-05-12T17:35:04Z

Is it ok to depend on specific version of llvm libraries?

For something like qtcreator it is probably okay. Usually there are tight version constraints on apps like this anyway.

Is it ok to add depends for compiler or should i resolve qtc need for clang plugins in some other way?

This should be fine. Sometimes we can add runtime search paths for this kind of thing but llvm should be okay.

What is preferred method to change package sources? Instead of substituteInPlace i may create yet another .patch file.

.patch should definitely be used if you have something that is upstreamable. substituteInPlace is probably ok for cases like this where you are putting output paths in the source code. Ideally, packages would have configure options to set things like this (for instance, location of llvm), but it's not always worth maintaining.

Is there a name convention for .patch files?

No, but maybe there should be. If there's an upstream version, try to keep that name though.

deadloko · 2019-05-12T18:44:38Z

Is it ok to depend on specific version of llvm libraries?

For something like qtcreator it is probably okay. Usually there are tight version constraints on apps like this anyway.

Is it ok to add depends for compiler or should i resolve qtc need for clang plugins in some other way?

This should be fine. Sometimes we can add runtime search paths for this kind of thing but llvm should be okay.

What is preferred method to change package sources? Instead of substituteInPlace i may create yet another .patch file.

.patch should definitely be used if you have something that is upstreamable. substituteInPlace is probably ok for cases like this where you are putting output paths in the source code. Ideally, packages would have configure options to set things like this (for instance, location of llvm), but it's not always worth maintaining.

Is there a name convention for .patch files?

No, but maybe there should be. If there's an upstream version, try to keep that name though.

Thank you for your answers.

danieldk · 2019-05-14T17:44:38Z

pkgs/development/tools/qtcreator/default.nix

@@ -16,14 +16,15 @@ stdenv.mkDerivation rec {

  src = fetchurl {
    url = "http://download.qt-project.org/official_releases/qtcreator/${baseVersion}/${version}/qt-creator-opensource-src-${version}.tar.xz";
-    sha256 = "1k23i1qsw6d06sy7g0vd699rbvwv6vbw211fy0nn0705a5zndbxv";
+    sha256 = "1a76jv1nl8fqgifk9pq5sdyrsq0ba9iwz5myz1898xhvaf91kvj6";


The updated hash should go into the commit that bumps the qtcreator version.

You're right. Fixed.

deadloko · 2019-05-15T06:03:19Z

Removed patch for botan, it's deprecated.

deadloko · 2019-05-18T18:58:52Z

ping @matthewbauer

nixos-discourse · 2019-05-30T06:44:18Z

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/5

abbradar

Awesome work! I have several questions regarding changes, see review.

pkgs/development/tools/qtcreator/default.nix

abbradar · 2019-06-03T12:01:01Z

pkgs/development/compilers/llvm/8/clang/clang_qtcreator_support.patch

+diff --git a/tools/extra/clazy/CMakeLists.txt b/tools/extra/clazy/CMakeLists.txt
+index d61714d0..f563976f 100644
+--- a/tools/extra/clazy/CMakeLists.txt
+++ b/tools/extra/clazy/CMakeLists.txt


You almost completely replace clazys original CMake file with this. Where does it come from and why is it needed?

We need to build clazy as clang plugin, not stand alone version, to be later invoked from clang executable.

Okay, but where does the modified file come from? Qt Creator's fork? It might be a good idea to explain it as a comment in expression along with a link to the original patch (which doesn't change this CMakeLists.txt).

I wrote it myself (actually just deleted all calls that is not needed for plugin build)

abbradar · 2019-06-03T17:08:06Z

After thinking some more about this: @deadloko maybe we should just build Qt's Clang fork separately? I feel that all this patching is quite flaky and more problems may possibly come later (maybe Qt Creator would expect additional information from clazy or something else). OTOH the fork is guaranteed to be good and doesn't require all this patching. One downside is additional compile time and a separate Clang (but I feel closure sizes aren't a problem for Qt Creator).

deadloko · 2019-06-03T17:35:22Z

@abbradar This is one of possible solutions for this. I'll invest some more time to see if it's possible, cause i've never tried to build qtc with qt version of llvm. I'm still have little experience with nix expressions, but, hopefully, i won't need to to write them from scratch. I'd like to have some advice on:

Where should .nix files for this fork reside (pkgs/development/compilers/ is the location that i have in mind)? And how should i name it?
Is it ok to override llvm,clang, etc... for this fork (I think it should be able to build just fine with just src attribute override) or should i use some different approach?

abbradar · 2019-06-03T17:41:53Z

If just an src override to clang would be enough (try it!) I'd put it into a let in Qt Creator's expression. BTW do they have a customized version of LLVM too? I'd try to build just Clang fork first, using system LLVM.

deadloko · 2019-06-03T18:06:32Z

They have whole llvm project (llvm, clang, clang-tools-extra) forked, but i think system LLVM should do just fine.

deadloko · 2019-06-05T16:52:13Z

Little update:
I managed to build clang as qtcreator runtime dependency for clazy and clang-tidy analysis, but i need some time to test it.
Unfortunately clang-lazy -> clazy patch still needed. They fixed it in qtcreator development tree, but it didn't make it into 4.9.1 release. So i would just add comment about it.

deadloko · 2019-06-06T14:07:53Z

@abbradar I would appreciate if you review new changes.

abbradar

Small fixes and we're good to go. Now we can merge it without fearing that clang breaks somehow, thanks!

pkgs/development/tools/qtcreator/default.nix

abbradar · 2019-06-06T18:34:56Z

@deadloko I tried to build it but got invalid checksum errors for patches, which is a bad sign:

hash mismatch in fixed-output derivation '/nix/store/6az7n3nyaz6nks71a7z0jhv9mld8lj10-clangtidyclazyrunner.cpp?id=53c407bc0c87e0b65b537bf26836ddd8e00ead82':
  wanted: sha256:1nqpwc6b421rvwv64wbpfaykv7fxrn9r1qrw1xf6cl5hqhnw7y93
  got:    sha256:1rl0rc2l297lpfhhawvkkmj77zb081hhp0bbi7nnykf3q9ch0clh

Can you try redownloading them? I think there was a better way than collecting garbage and building it again recently but not sure.

Clang code model provides user with services: code completion, syntactic and semantic highlighting, diagnostics, outline of symbols, tooltips, renaming of local symbols. It's naturally depends on llvm and clang. To make it work with nixos: 1. Add qt version of clang as build dependency. 2. Added patch for clang libc++ regexp trunk regexp not including path like "libc++-version". 3. Fixed paths to llvm/clang libraries and includes. 4. Fixed name of clazy clang plugin.

deadloko · 2019-06-07T02:01:52Z

@abbradar I fixed hashes for patches, but it's strange behavior.

deadloko · 2019-06-07T08:49:20Z

@abbradar I've tested build on another machine (NixOS too). Build still succeedes.

abbradar · 2019-06-07T14:39:54Z

Sadly I lost ability to build this because I needed to RMA my CPU :D If someone could just build it and verify it starts at least I'd merge.

…

On June 7, 2019 11:49:29 AM GMT+03:00, Andrew Newman ***@***.***> wrote: @abbradar I've tested build on another machine (NixOS too). Build still succeedes. -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #61348 (comment)

-- Nikolay.

deadloko · 2019-06-07T16:58:58Z

@abbradar Well, that's unfortunate, hope your cpu replacement soon. =) Also can't you ask borg bot to build this one?
@danieldk @matthewbauer @Ericson2314 may i ask you to provide aid on this one?

abbradar · 2019-06-07T18:02:05Z

I'd just like to run it too but as I can't and you tested it - let's just check that it builds

@GrahamcOfBorg build qtcreator

abbradar · 2019-06-07T21:31:48Z

Bot's timeout kicked in ;_; Can someone try to build this?

abbradar · 2019-06-10T09:35:01Z

Let's not keep this stalled - you tested it on two machines so this should be enough. Merging, thanks!

SRGOM · 2019-09-29T13:14:07Z

@deadloko I'm trying to update qtc to 4.10.0 but I'm not sure what all needs to change here. Could you list some expected changes and tests (I see one detailed testing procedure you've mentioned in the original comment) that would help me change with enough confidence to create a PR?

Some info if it helps- currently just downloading 4.10.0 gives me a patch already applied error for the accompanying patch, and without it a linking error after an hour of compilation.

deadloko · 2019-09-29T15:39:15Z

@SRGOM There was 3 patches (and some substitutions in nix expressions):

In-tree 0001-Fix-clang-libcpp-regexp.patch - needed for correct clang stdlib support, i assume this is still needed.
Two patches from qtc team to properly compile clazy - most definitely not needed anymore, cause 4.10 should include them.

More info on linking error (console output) would help to solve problem.

deadloko requested a review from matthewbauer as a code owner May 12, 2019 08:05

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels May 12, 2019

matthewbauer reviewed May 12, 2019

View reviewed changes

danieldk reviewed May 14, 2019

View reviewed changes

deadloko force-pushed the feature/QtClangIntegration branch from d097c00 to fb741fe Compare May 15, 2019 04:11

deadloko mentioned this pull request May 15, 2019

qtcreator: 4.6.2 -> 4.9.0 #61453

Closed

10 tasks

deadloko force-pushed the feature/QtClangIntegration branch from fb741fe to f7603a9 Compare May 15, 2019 06:03

deadloko force-pushed the feature/QtClangIntegration branch from f7603a9 to a241ce6 Compare May 15, 2019 06:31

matthewbauer requested review from Ericson2314 and abbradar June 3, 2019 01:13

abbradar reviewed Jun 3, 2019

View reviewed changes

deadloko force-pushed the feature/QtClangIntegration branch from a241ce6 to b8596bb Compare June 3, 2019 16:16

deadloko force-pushed the feature/QtClangIntegration branch from b8596bb to 668831c Compare June 6, 2019 13:50

ofborg bot added 10.rebuild-darwin: 0 and removed 10.rebuild-darwin: 1-10 labels Jun 6, 2019

abbradar requested changes Jun 6, 2019

View reviewed changes

pkgs/development/tools/qtcreator/default.nix Outdated Show resolved Hide resolved

pkgs/development/tools/qtcreator/default.nix Show resolved Hide resolved

deadloko force-pushed the feature/QtClangIntegration branch from 668831c to 4a0a8f4 Compare June 6, 2019 14:46

abbradar approved these changes Jun 6, 2019

View reviewed changes

deadloko force-pushed the feature/QtClangIntegration branch from 4a0a8f4 to f3a7f76 Compare June 7, 2019 00:45

deadloko force-pushed the feature/QtClangIntegration branch from f3a7f76 to 3d80880 Compare June 7, 2019 00:53

abbradar merged commit 6365fa1 into NixOS:master Jun 10, 2019

deadloko mentioned this pull request Oct 7, 2019

qtcreator 4.9.1 -> 4.10.0 #70573

Merged

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/qt clang integration #61348

Feature/qt clang integration #61348

deadloko commented May 12, 2019 •

edited

deadloko commented May 12, 2019 •

edited

matthewbauer May 12, 2019

deadloko May 12, 2019

matthewbauer May 12, 2019

deadloko May 13, 2019

matthewbauer commented May 12, 2019

deadloko commented May 12, 2019

danieldk May 14, 2019

deadloko May 15, 2019

deadloko commented May 15, 2019

deadloko commented May 18, 2019

nixos-discourse commented May 30, 2019

abbradar left a comment

abbradar Jun 3, 2019

deadloko Jun 3, 2019

abbradar Jun 3, 2019 •

edited

deadloko Jun 3, 2019

abbradar commented Jun 3, 2019

deadloko commented Jun 3, 2019

abbradar commented Jun 3, 2019

deadloko commented Jun 3, 2019

deadloko commented Jun 5, 2019

deadloko commented Jun 6, 2019

abbradar left a comment

abbradar commented Jun 6, 2019

deadloko commented Jun 7, 2019

deadloko commented Jun 7, 2019

abbradar commented Jun 7, 2019 via email

deadloko commented Jun 7, 2019

abbradar commented Jun 7, 2019 •

edited

abbradar commented Jun 7, 2019

abbradar commented Jun 10, 2019

SRGOM commented Sep 29, 2019

deadloko commented Sep 29, 2019

		@@ -0,0 +1,334 @@
		diff --git a/tools/driver/CMakeLists.txt b/tools/driver/CMakeLists.txt

Feature/qt clang integration #61348

Feature/qt clang integration #61348

Conversation

deadloko commented May 12, 2019 • edited

Motivation for this change

Things done

How clang-tools and clang-code-model functionallity was tested

Update:

Update2:

deadloko commented May 12, 2019 • 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

matthewbauer commented May 12, 2019

deadloko commented May 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deadloko commented May 15, 2019

deadloko commented May 18, 2019

nixos-discourse commented May 30, 2019

abbradar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abbradar Jun 3, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abbradar commented Jun 3, 2019

deadloko commented Jun 3, 2019

abbradar commented Jun 3, 2019

deadloko commented Jun 3, 2019

deadloko commented Jun 5, 2019

deadloko commented Jun 6, 2019

abbradar left a comment

Choose a reason for hiding this comment

abbradar commented Jun 6, 2019

deadloko commented Jun 7, 2019

deadloko commented Jun 7, 2019

abbradar commented Jun 7, 2019 via email

deadloko commented Jun 7, 2019

abbradar commented Jun 7, 2019 • edited

abbradar commented Jun 7, 2019

abbradar commented Jun 10, 2019

SRGOM commented Sep 29, 2019

deadloko commented Sep 29, 2019

deadloko commented May 12, 2019 •

edited

deadloko commented May 12, 2019 •

edited

abbradar Jun 3, 2019 •

edited

abbradar commented Jun 7, 2019 •

edited