Bug 387391 - Random responses from the C/C++ language support when using symbolic links to CMake sub-projects.
Summary: Random responses from the C/C++ language support when using symbolic links to...
Status: CONFIRMED
Alias: None
Product: kdevelop
Classification: Applications
Component: Language Support: CPP (Clang-based) (show other bugs)
Version: 5.2.1
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: kdevelop-bugs-null
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-11-28 09:55 UTC by Venca B Spam
Modified: 2018-07-25 06:59 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
ComplexSampleScenario (412.72 KB, application/zip)
2017-11-28 09:56 UTC, Venca B Spam
Details
How to reproduce bug - video illustration (1.25 MB, video/mp4)
2017-12-01 06:49 UTC, Venca B Spam
Details
Scenario with CMake and symbolic links which is OK (769 bytes, application/gzip)
2017-12-01 15:22 UTC, Venca B Spam
Details
Scenario with CMake and symbolic links which is BAD/FAILS. (1000 bytes, application/gzip)
2017-12-01 15:33 UTC, Venca B Spam
Details
Scenario with CMake and symbolic links INSIDE PROJECT which is BAD/FAILS. (1.38 KB, application/gzip)
2017-12-12 17:14 UTC, Venca B Spam
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Venca B Spam 2017-11-28 09:55:28 UTC
Random responses from the C/C++ language support when using symbolic links to CMake sub-projects.

* How to reproduce? *
1. Open/Import attached CMake project located in ComplexSampleScenario/Projects/SampleMultiProject folder of attached archive.
2. Let KDevelop parse the project.
3. Browse the code specially files ClassA1.cpp, ClassB1.cpp, SubProjectExecutable1.cpp.

* What happens? *
The language support randomly shows error/warning hints in files ClassA1.cpp, ClassB1.cpp.

* What is expected? *
The language support should not show error/warning hints in files ClassA1.cpp, ClassB1.cpp as they are valid and the project definition provides proper include paths to build those files.

* Notes/Observations*

- When SubProjectLibraryA and SubProjectLibraryB is not sym-linked KDevelop language support works well (without random error/warning hints)

- Sometimes files ClassA1.cpp, ClassB1.cpp does not show errors initially, later when user just press space " " keeping the file compilable, KDevelop language support changes its state and shows error/warning hints
Comment 1 Venca B Spam 2017-11-28 09:56:44 UTC
Created attachment 109090 [details]
ComplexSampleScenario
Comment 2 Venca B Spam 2017-11-28 10:03:16 UTC
I did not mentioned that the attached scenario assumes Linux operating system environment as it uses call to 'ln' executable (at this moment does not support Windows mklink).
Comment 3 Venca B Spam 2017-12-01 06:49:46 UTC
Created attachment 109144 [details]
How to reproduce bug - video illustration
Comment 4 Venca B Spam 2017-12-01 06:51:45 UTC
I added short clip/video of how to reproduce the bug. 

If would be cool if anyone has an idea where to look at into the code :-).
Comment 5 Sven Brauch 2017-12-01 09:08:24 UTC
Huh, very strange issue. I have no idea what causes this. You can export KDEV_CLANG_DISPLAY_DIAGS=1 and look if there's anything interesting in the output. Also, look at the "Problems" toolview.
Comment 6 Venca B Spam 2017-12-01 10:12:47 UTC
Thank you for your advice, I will try it.

I know how to change the project to work well. It is as simple as to replace symbolic links by their targets . In other words, if the project does not contain symbolic links and all folders are 'at-the-place' everything works well.

I guess it will be in some kind of evaluation of paths if they fit to some pattern. If it does not, e.g. if the include path is outside the project root, it may be excluded, even it is valid - but it is just guessing.
Comment 7 Venca B Spam 2017-12-01 10:50:01 UTC
Ok, I did the test and it looks even weirdest then before.

I also enabled the KDEV_CLANG_DISPLAY_ARGS=1 to see how the Clang is being invoked.

Observations:
a) 'sometimes' it is invoked without proper -I include definitions which produces the expected issue described here.
b) 'sometimes' it is invoked WITH proper -I which gives proper output just with some linking issues (I assume it does not break the think)
c) however 'sometimes' even it was initially OK (see the bullet b) above), when I press e.g. " " (space) in the editor, the editor changes the hint/parsing to error. What is really weird here is, that I do not see any Clang invocation nor even attempt to run Clang (at the moment I pressed the keyboard " " - could be any character triggering the dirty save flag)

Summary:
- The change to from ok to an error state changes without Clang invocation
- It may start with an error state on the beginning based on Clang output which correctly generate the error as it do not get proper -I includes on argument line.

Is there any debug environment variables I can use to enable logs to observe the parser loop/events?
Comment 8 Sven Brauch 2017-12-01 10:52:20 UTC
Maybe the logic which checks which project a file is part of gets confused by the symlinks, could that be related?
Comment 9 Venca B Spam 2017-12-01 11:02:34 UTC
Yes, I guess that it may be the issue. If the state machine (whatever logic) expects the state has some meaning and it behaves a bit different it may have unpredictable results.

In fact the symlink os API sometimes resolve symlinks sometimes do not. Symlink itself is just inode pointing to another inode. Depends on how you watch files, it may return results which may not be understandable for the parser.

I can imagine following:

string watch_inode_file( string file) {..}

when called:

auto result = watch_inode_file( "/path/to/project/symlinked_dir/file.cpp");

may return in result="/path/somewhere/else/symlinked_dir/file.cpp".
Comment 10 Venca B Spam 2017-12-01 11:06:06 UTC
What I can not explain is the:
a) 'sometimes' it is invoked without proper -I include definitions which produces the expected issue described here.
Comment 11 Sven Brauch 2017-12-01 11:07:38 UTC
I could imagine that this could e.g. depend on whether the parser is invoked for the open document you modified, or for the document which is listed in the list of files which are part of the project. Those are the same of course, but maybe be listed with different paths (once with symlink resolved, once without).
Comment 12 Francis Herne 2017-12-01 11:32:40 UTC
See also https://phabricator.kde.org/D7930 , which claims to improve ..something...? about symbolic links to build dirs but also describes some scenarios about symlinks in include paths.

Also, https://bugs.kde.org/show_bug.cgi?id=217405 probably covers this bug, but from before many changes to the C++ and CMake support.
Comment 13 Venca B Spam 2017-12-01 15:17:54 UTC
Could it be caused by this?
https://github.com/KDE/kdevelop/blob/master/plugins/cmake/cmakemanager.cpp#L246
Comment 14 Venca B Spam 2017-12-01 15:22:13 UTC
Created attachment 109154 [details]
Scenario with CMake and symbolic links which is OK

I added simplified scenario with CMake and symbolic links which does not suffer with the bug.

This test case does not use CMake project-in-project. It uses symbolic link just to put external library to common include directory.

This test pass ok.
Comment 15 Venca B Spam 2017-12-01 15:33:10 UTC
Created attachment 109155 [details]
Scenario with CMake and symbolic links which is BAD/FAILS.

I added another simplified scenario with CMake and symbolic links which shows the bug.

This test case does use CMake project-in-project. It uses symbolic link to put external library to the 'master' project root dir which then add_subdirectory() into it.

The bug shows in minimalistic case, when one browse the Bar.cpp. When one looks at Foo.cpp, it works well.

It is also worth of mention, that the Bar.cpp can be temporarily 'fixed' when one execute following action:
- RMB click on the project in the project tree view
- from the context menu select and LMB click on 'Open Configuration'
- select 'Language Support=>C/C++ Parser'
- do some dummy change which will enable the 'Apply' button.
(e.g. by changing and returning the 'Compiler for path' dropdown list)
- press OK or Apply and then OK

This will make the file Bar.cpp temporarily error free, when one insert/change any character in the editor, it will break aggain.


This test pass ok.
Comment 16 Venca B Spam 2017-12-01 15:36:01 UTC
The last sentence "This test pass ok." in comment before this one is Copy&Paste error. Of course that test case fails.
Comment 17 Venca B Spam 2017-12-04 11:42:35 UTC
I have another observation :-).

When the symbolic link is absolute, the issue does not show.

So summary for now:

The issue occurs *only* when relative symbolic links are employed.
Comment 18 Venca B Spam 2017-12-05 19:37:13 UTC
The previous comment was false positive. It has still problem regardless the link is absolute or relative.

(In reply to Venca B Spam from comment #17)
> I have another observation :-).
> 
> When the symbolic link is absolute, the issue does not show.
> 
> So summary for now:
> 
> The issue occurs *only* when relative symbolic links are employed.

OK, this was false positive.
Comment 19 Venca B Spam 2017-12-05 19:52:52 UTC
As the previous post was false positive, the issue still annoys me.

I did a bit investigation and it seems that changes I put here

https://github.com/vbspam/kdevelop/commit/d1898acee23e8324a718414c5e4ccb0fcae0d460

solved most of my test cases (please skip my debug messages). 

Also please ignore changes in "plugins/custom-definesandincludes/compilerprovider/widget/compilerswidget.cpp" which are not related to this issue. 
Should I propose merge request, I will provide properly formed commits.


I understand that the "canonicalFilePath" is probably native to KDevelop, but it does not work for symbolic links when resolving against CMake build system and also does not return properly "hasBuildSystemInfo" flag.
Comment 20 Venca B Spam 2017-12-12 11:30:27 UTC
Finally I found a solution to all my test cases. For anyone with better knowledge of KDevelop/KF/QT please check it here:
https://github.com/vbspam/kdevelop/commit/550c6e1b6867598f372a68979c1ee7718ee2854c

I put there some debugging messages to easily find out where I see the problem is.

It looks that it is expected that canonicalFilePath is best to deal with symlinks. I see it as good argument, my debug messages however reveals that KDevelop uses absoluteFilePath in some cases.

What is surprise to me is that my previous attempt to force use of "absoluteFilePath" (previous comment commit) sometimes caused more complex test cases did not worked. I guess it may be the IndexedString maid be malformed by my improper use of QT api.

I would like to ask very politely anyone who will review it, please do not comment my use of the KF/QT API as I am absolute novice and just trying to help. The core of the issue is described in my bugreport and illustrated in the https://bugs.kde.org/attachment.cgi?id=109155.

Also please note, the commit is just for discussion and tests. Should I propose merge/pull request, I will of course follow particular project rules.
Comment 21 Sven Brauch 2017-12-12 12:17:31 UTC
Thank you very much for looking into this! Ideally, you could submit your full patch to phabricator.kde.org, which is our usual platform for reviewing changes. I am a bit confused by your github link, because the code you end up with is exactly what I see in our current codebase ...?
Comment 22 Venca B Spam 2017-12-12 12:56:35 UTC
(In reply to Sven Brauch from comment #21)
> Thank you very much for looking into this! Ideally, you could submit your
> full patch to phabricator.kde.org, which is our usual platform for reviewing
> changes. I am a bit confused by your github link, because the code you end
> up with is exactly what I see in our current codebase ...?

Thank you for your attention. The final change is just single line in plugins/cmake/cmakeserverimportjob.cpp which changes canonicalFilePath()
 call to absoluteFilePath().

(https://github.com/vbspam/kdevelop/commit/d1898acee23e8324a718414c5e4ccb0fcae0d460#diff-a96baafe1c9ef34cbcc5cbc05978cfaeR139).


From my point of view, even it pass all my test cases, it is not ready to submit due to the following reasons:
a) needs some cleanup (variable names, remove comments)
b) although I know why it works, I do not know why the original idea of Milian Wolff does not work with my test cases (https://github.com/vbspam/kdevelop/commit/e1500e1f) 

Please note, I put my changes to branch dev-387391, which I hope I properly link in this issue (at least it works on my laptop).
Comment 23 Venca B Spam 2017-12-12 17:14:28 UTC
Created attachment 109341 [details]
Scenario with CMake and symbolic links INSIDE PROJECT which is BAD/FAILS.

Added another test scenario covering symbolic links inside project.
Comment 24 Aaron Puchert 2018-01-06 19:06:54 UTC
I think this is a hard problem. We're running into a fundamental issue with symlinks here.

KDevelop uses paths to identify files. Comparing paths is already hard enough because of "..", and that is used quite heavily. (Try compiling any file with gcc or clang with option -v, and you're probably going to find ".." in the default include paths.) But it gets really problematic with symlinks. (See http://harmful.cat-v.org/software/symlinks for someone ranting about symlinks.)

Right now, the flags passed to the parser depend on the path. (Specifically, files outside the project tree are parsed without project-specific flags, like certain include directories you've set.) Even that is a compromise, because a file could be compiled with different flags for different targets. But for an IDE that compromise is Ok, I guess. However, when using symlinks, the same file could have different paths, and thus different flags. I guess this is what happens here: the file is at the same time inside and outside the source tree, so sometimes you get the include directories, sometimes you don't.

But what do we expect? Should the flags be associated with a path, or some kind of canonical path, or an inode number? Should symlinks be resolved for that path? It seems you want a file to be treated as if it were inside the project, although it technically isn't. What if someone opens the file via the symlink-free path outside of the project? Should it still be parsed as part of the project? This seems like a difficult question that needs to be answered before we can think about writing a patch. Maybe I'm not clever enough, but it doesn't seem obvious to me how to get this working with symlinks without breaking other things.

By the way, small nitpick: symlinks don't point to inodes, as you wrote in #c9, but to (relative or absolute) paths.
Comment 25 Venca B Spam 2018-01-06 20:12:06 UTC
Thank you for your feedback!

While studying the KDevelop architecture I realized all you stated. I compared this with my user level (or call it business requirements) and I see the way how it is implemented in KDevelop as "a bit that the technical solution is exposed to the end user" - but I could be wrong and it may be an intentional effect. I compared how it is solved in QtCreator and I like it more there (they use absolute path as it is defined in the build system). However in general I still like KDevelop more - which keeps my attention to this issue :-).

There are however more consequences of how the symlinks are handled in KDevelop. I reported another bug (https://bugs.kde.org/show_bug.cgi?id=387866#c5) where it is even more obvious that the canonicalized paths exposed to the end user is not perfect.
Comment 26 Victor Lamoine 2018-07-25 06:59:07 UTC
Just trying to add more information, the background parser fails to parse my ROS projects if the ROS packages (a directory containing a CMakeLists.txt file) are symlinks

I'm using KDevelop 5.2.1 on Kubuntu 18.04

Steps to reproduce
- Install ROS: http://wiki.ros.org/melodic/Installation/Ubuntu
- Clone one of my package to your $HOME directory: 

git clone https://gitlab.com/InstitutMaupertuis/topics_rviz_plugin.git $HOME/topics_rviz_plugin

- Create a catkin workspace with the ROS package as a symlink:

mkdir -p $HOME/catkin_workspace/src
cd $HOME/catkin_workspace/src
catkin_init_workspace
ln -s ../../topics_rviz_plugin


- Launch KDevelop from a terminal in which ROS is sourced (usally done in .bashrc, see http://wiki.ros.org/melodic/Installation/Ubuntu#melodic.2BAC8-Installation.2BAC8-DebEnvironment.Environment_setup), you can also use

source /opt/ros/*/setup.bash

- Import the top level CMakeLists.txt ($HOME/catkin_workspace/src/CMakeLists.txt)
- Tweak the import parameters like advised here: https://wiki.ros.org/IDEs#Building_catkin_packages
- Let the background parser finish (shouldn't take more than a few seconds)
- Open topics_rviz_plugin/src/topic_info.cpp for example, it should be correctly coloured, modify the file (add a whitespace for example) and save, the colours goes away and KDE fails to resolve every ROS include.

# Before editing the file
https://phabricator.kde.org/file/data/bfp4lecqj4x2rscdcefx/PHID-FILE-vfndx6hqoqlr5xbihkul/Screenshot_20180724_150241.png

# After editing the file
https://phabricator.kde.org/file/data/c7tgybksobzitmbbjpa7/PHID-FILE-r3tiwi3metsql5bd2z4u/Screenshot_20180724_150248.png

If instead of having a symlink in the catkin workspace ($HOME/catkin_workspace/src/topics_rviz_plugin) you put the directory and import the project again, everything works perfectly.
My conclusion is that the symlink causes the problems but I can be wrong.