Bug 296021 - Relative paths in include_directories ignored.
Summary: Relative paths in include_directories ignored.
Status: RESOLVED FIXED
Alias: None
Product: kdevelop
Classification: Applications
Component: Build tools: CMake (show other bugs)
Version: 4.2.90
Platform: Ubuntu Linux
: NOR normal
Target Milestone: 4.2.3
Assignee: kdevelop-bugs-null
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-14 21:16 UTC by bungeman
Modified: 2012-03-19 12:55 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
Mock revert which restores old behavior. (6.59 KB, patch)
2012-03-16 08:22 UTC, bungeman
Details
A minimal project which demonstrates the issue. (1.17 KB, application/octet-stream)
2012-03-17 02:06 UTC, bungeman
Details
A possible fix. (624 bytes, patch)
2012-03-17 22:01 UTC, bungeman
Details

Note You need to log in before you can comment on or make changes to this bug.
Description bungeman 2012-03-14 21:16:56 UTC
I have a CMakeLists.txt which looks something like

add_executable(targetName "../include/header.h" "../src/source.cpp")
set_target_properties(targetName PROPERTIES COMPILE_DEFINITIONS "a;b")
include_directories("../include/config" "../include/core")

The intent is to somewhat abuse CMake and place my (generated) CMakeLists.txt into an unversioned output directory. CMake itself appears to be fine with this. This also all seems to work fine in KDevelop 4.0.0 (using KDevPlatform 1.0.0) on KDE 4.4.5 (kdevelop package 4:4.0.0-0ubuntu1 on Lucid).

However, on 4.2.90 (kdevelop package 4:4.2.90-0ubuntu1 on Precise, using kde 4:4.8.1ubuntu1) none of the include directories listed appear to be used. Furthermore this happens silently, so it is difficult to determine the issue. When I hover over an unresolved include in the code, the searched include path does not mention any of the include directories in the CMakeLists.txt, just the default system directories. Again, this use case used to work in 4.0.0; if I transfer the file to my Lucid machine with 4.0.0 with the exact same setup, the includes all resolve.

If I take this CMakeLists.txt and move it to the root of the project and remove all of the '../' then everything works. Furthermore, and rather oddly, if I then close the new project, delete this new CMakeLists.txt, then open the original CMakeLists.txt file, the includes now resolve! Apparently the parser has a cache and is mapping the includes from both projects to a single entry (since they are the same files on disk).

Note that the only issue is with include_directories. Using '../' for file names in add_executable works as expected in all cases.

As complete speculation, this could possibly be due to the KUrl changes ( https://bugs.kde.org/show_bug.cgi?id=285028 ).
Comment 1 bungeman 2012-03-14 22:04:58 UTC
I was able to localy build kdevelop 4.3.60 and reproduce.
Comment 2 bungeman 2012-03-15 12:58:46 UTC
I managed to debug KDevelop with KDevelop (which would be much easier with a fix for bug 285210). I first started with the cmake project manager side of things. When I get to cmakeparserutils.cpp line 180

data->includeDirectories=v.includeDirectories();

the include directories here are all correct. They are all absolute file names as strings, for example my project setup looks something like

/home/user/project
    /include
        /config
            config.h
        /core
            source.h
    /src
        source.cpp
    /out
        CMakeLists.txt

with CMakeLists.txt looking something like

add_executable(targetName "../src/source.cpp")
include_directories("../include/config" "../include/core")

At cmakeparserutils.cpp line 180 I'm seeing

print includeDirectories
$19 = QStringList<QString> = {[0] = "/home/user/project/out", [1] = "/home/user/project/include/config", [2] = "/home/user/project/include/core"}

and everything looks like it should work.

So I've turned my attention to includepathcomputer.cpp on the language/cpp side, but this has much more indirection involved. However, the end result is that it doesn't appear that includepathcomputer is providing these paths.

Also, I mentioned previously that this situation worked in 4.0.0. It also appears to work in 4.2.3 (4:4.2.3-0ubuntu1, kde4:4.7.4-0ubuntu0.1).
Comment 3 bungeman 2012-03-15 17:13:24 UTC
Seems I got confused about where I saw what when I copied out of the gdb output. After compiling KDevelop in Debug configuration it was much easier to see what is going on. The output

print directories
$19 = QStringList<QString> = {[0] = "/home/user/project/out", [1] = "/home/user/project/include/config", [2] = "/home/user/project/include/core"}

I mentioned above was actually seen in cmakemanager.cpp line 797

folder->setIncludeDirectories(directories);

The folder at this point is 

print folder->d_ptr->m_url
$12 = {<QUrl> = file:///home/usr/project/out/, d = 0x0}

so, so far so good.
Comment 4 bungeman 2012-03-15 20:15:46 UTC
There appears to be only one way for the buildManager (in this case cmake) to influence the include directories, and that appears to be at includepathcomputer.cpp line 96

KUrl::List dirs = buildManager->includeDirectories(file);

However, it seems this is never being executed. The line

QList<KDevelop::ProjectFileItem*> files = project->filesForUrl(m_source);

always seems to return an empty list. Also, a great number of times m_source is the empty url.
Comment 5 bungeman 2012-03-16 08:22:57 UTC
Created attachment 69658 [details]
Mock revert which restores old behavior.

After a great deal of git bisect I have come to the conclusion that this is caused by the following commit. (I really wish this thread http://barney.cs.uni-potsdam.de/pipermail/kdevelop-devel/2010-May/037255.html hadn't gotten sidetracked, bisecting across both kdevelop and kdevplatform is a real pain.) 

acabbf5b22030266968bb60a3ae50109c92c00f6

in kdevplatform. If I take tip of tree kdevplatform and kdevelop and manually revert this patch, the old behavior returns. See attached patch for what I locally reverted. This isn't a serious patch, it's mostly for clarification. I have not yet taken a look into why this is the case.
Comment 6 bungeman 2012-03-17 02:06:16 UTC
Created attachment 69684 [details]
A minimal project which demonstrates the issue.

If this project is imported using the CMake importer with the revert everything works as expected. Without the revert, the include is not resolved.
Comment 7 bungeman 2012-03-17 21:19:10 UTC
Stepping through the code with the minimal testcase but without the patch (at master) in ProjectModel::itemsForUrl I see that the ProjectBaseItem in the d->urlLookupTable has a url like file:///home/user/test/out/../src/code.cpp while the url being searched for is file:///home/user/test/src/code.cpp . As a result, no items are returned. A possible fix here is to simplify the url on the ProjectBaseItem.
Comment 8 bungeman 2012-03-17 22:01:35 UTC
Created attachment 69699 [details]
A possible fix.

Simply canonicalizing the url seems to fix the issue. I believe this should be done with all urls (as in the patch) and not just on relative urls so that if someone perversely writes "/home/user/project/out/../src/code.cpp" it will still work. This is also in keeping with the other call to cleanPath for include directories.

Note that all of this used to worked in the old code path because QUrl operator== 'normalizes' the urls before comparing them.
Comment 9 bungeman 2012-03-18 22:17:07 UTC
Created review https://git.reviewboard.kde.org/r/104339/ .
Comment 10 Milian Wolff 2012-03-19 12:55:00 UTC
Git commit fec7aa1a5364673277b851045e5245a96d1e7fd7 by Milian Wolff.
Committed on 19/03/2012 at 13:53.
Pushed by mwolff into branch '4.3'.

clean paths of target files

this is required for proper functionality of itemsForUrl
since it uses an optimized index-based lookup which
breaks for un-cleaned urls.

Thanks to Ben Wagner for investigating this.
REVIEW: 104339

M  +1    -1    projectmanagers/cmake/cmakemanager.cpp
M  +45   -3    projectmanagers/cmake/tests/cmakemanagertest.cpp
M  +1    -0    projectmanagers/cmake/tests/cmakemanagertest.h

http://commits.kde.org/kdevelop/fec7aa1a5364673277b851045e5245a96d1e7fd7