Bugzilla – Full Text Bug Listing
| Summary: | [Scanner Discovery] Per File Build output parser misses includes with drive-relative paths when sources are behind a linked resource | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Andrew Gvozdev <angvoz.dev> | ||||||||||
| Component: | cdt-build | Assignee: | Andrew Gvozdev <angvoz.dev> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | Andrew Gvozdev <angvoz.dev> | ||||||||||
| Severity: | normal | ||||||||||||
| Priority: | P3 | CC: | cdtdoug, mober.at+eclipse | ||||||||||
| Version: | 8.0 | ||||||||||||
| Target Milestone: | 8.0.1 | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Andrew Gvozdev
Created attachment 196519 [details] Patch from bug 323011 Attaching my patch from bug 323011 as per Andrew's request. If possible, I'd like to see this improvement made in CDT 8.0.1. Thanks! CQ:WIND00222792 ping ping, can I help moving this forward ? I want to look at this eventually but it is not on the short list. Frankly, I'd rather spend the little time I have for CDT on bug 290631. I still want to understand your case to cover it too. That will likely cause patching the old releases but I can't promise any timing on that. Obviously, just reproducing the problem is not trivial here. I finally got to re-trying this with 8.0 (Indigo) and found that there are still issues when sources are behind a linked resource:
1.) When using the gcc "per file" scanner info profile, only 1 include is
found (8 includes missing!)
2.) When using the gcc "per project" scanner info profile, the
"/kernel/bsp/common/obj/x86" include is missing, which gets included by
-I. while compiling in X:\kernel\bsp\common\obj\x86
Both issues do not occur when the sources are inside the project. I will attach a testcase and instructions to reproduce very easily.
Updated description since this is not only about performance, but about correctness - previous value was:
[Scanner Discovery] Build output parser could run 5 times faster
Created attachment 200810 [details]
Testcase to reproduce
Steps to reproduce:
1. Extract attached ZIP somewhere, D:\tmp
2. subst X: D:\tmp\DiscoveryTest
3. Create an empty project (type: Makefile Project) in Workspace e.g. D:\ws
4. Select Project, File > New > Folder : Advanced : Link to folder in FS
--> Link "x" to "X:\"
5. Project Properties > C/C++ Build > Discovery:
5a) Select "Per file" discovery profile, load X:\build.log
--> only one Include is detected, 8 includes missing
5b) Select "Per project" discovery profile, load X:\build.log
--> 7 includes detected, but "/x/kernel/bsp/common/obj/x86" is missing
--> Should be found due to "-I."
5c) The implicit "X:\" include path is missing, it is needed to resolve
the include relation
commonSysUtils.c --> vsbConfig.h
which uses the computed "/kernel/..." filename from _VSB_CONFIG_FILE
To check include paths, just expand the "includes" node in the project explorer, or open "/x/kernel/bsp/common/src/commonSysUtils.c" and in the editor choose Show In > Include Browser.
I think the important point here is that when the build log reader knows that
"X:/foo/bar" is the current directory, then any file reference like
"/some/folder" should be interpreted as "X:/some/folder".
Created attachment 200819 [details] patch v2 against cdt_8_0 Attached patch is my original contribution from bug 323011, rebased to cdt_8_0. This fixes issue (5b) for me -- as per bug 323011 comment 14 the difference had probably been that you tested with the "per project" profile and I tested with the "per file" profile. Issue (5a) was likely a build or config error, it works OK for me regardless of applying the patch or not. In fact, the "per project" code path is completely different than the "per file" code path and looks OK. Issue (5c) is not fixed with this, and actually I'm not sure how to fix this properly because the drive root (X:\) is not really an include path ... it's just a prefix location for where absolute includes can be found. In the "per file" mode it might be a good idea adding it, but I was not sure. Could my patch be considered for cdt 8.0.1 ? I should also mention that in terms of performance, my patch achieves 6 sec load time on a large build log, where the original cdt_8_0 branch takes 20 sec. (In reply to comment #7) > Created attachment 200810 [details] > Testcase to reproduce Hmm, I am getting different results trying to reproduce. > Steps to reproduce: > > 1. Extract attached ZIP somewhere, D:\tmp > 2. subst X: D:\tmp\DiscoveryTest > 3. Create an empty project (type: Makefile Project) in Workspace e.g. D:\ws > 4. Select Project, File > New > Folder : Advanced : Link to folder in FS > --> Link "x" to "X:\" > 5. Project Properties > C/C++ Build > Discovery: > 5a) Select "Per file" discovery profile, load X:\build.log > --> only one Include is detected, 8 includes missing I don't get any includes here, it does not look like it ran at all. I need to look why. > 5b) Select "Per project" discovery profile, load X:\build.log > --> 7 includes detected, but "/x/kernel/bsp/common/obj/x86" is missing > --> Should be found due to "-I." I am getting 8 includes here including "/x/kernel/bsp/common/obj/x86" > 5c) The implicit "X:\" include path is missing, it is needed to resolve > the include relation > commonSysUtils.c --> vsbConfig.h > which uses the computed "/kernel/..." filename from _VSB_CONFIG_FILE > > To check include paths, just expand the "includes" node in the project explorer, > or open "/x/kernel/bsp/common/src/commonSysUtils.c" and in the editor choose > Show In > Include Browser. > I think the important point here is that when the build log reader knows that > "X:/foo/bar" is the current directory, then any file reference like > "/some/folder" should be interpreted as "X:/some/folder". The good news (well, for me) is that the new scanner discovery already handles that just fine. Except that your case uncovered that it does not understand absolute path to gcc :). After I fixed that it worked like a charm. Thanks, Andrew, for picking this up again. It's interesting that our test results are different... but good to know you've made progress. What I found in my testing was that the results depended on whether the workspace was fresh and the project had just been created - or it had been pre-existing by some way. In the context of this bug, could we focus on the "per file" discovery profile? That's the immediate issue relevant to us, and that's what my attached 1-liner patch fixes. Thanks! Martin Created attachment 201530 [details]
screenshot
Sigh. I am not able to get per-file scanner discovery working with your test case as described at all in stock CDT. After applying your patch I am getting 15 entries. They don't show after loading, to uncover any collected entries I have to close and reopen the project first. Along with adding the entries to file-specific settings it shows that also on the project level. This thing is profoundly broken.
I applied your patch adding cosmetic changes. Also, corrected the issue with non-existing relative paths showing differently in Paths & Symbols. Created bug 355127 for the problem with loading the log I was having. Committed on master and cdt_8_0. Thanks for the patch Martin. *** cdt git genie on behalf of Andrew Gvozdev ***
bug 345750: convert relative paths to absolute also for those that don't
exist
[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=fa5965cef09a515c28a51b1aa44e3b010d2a33e0
*** cdt git genie on behalf of Andrew Gvozdev ***
bug 345750: tidy up cosmetics
[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=d5d717128b47835cfbb5a4623d977bfbb2edd925
*** cdt git genie on behalf of Martin Oberhuber ***
bug 345750: [Scanner Discovery] Per File Build output parser misses
includes with drive-relative paths when sources are behind a linked
resource
[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=950f133f77d2f390e9a069015aa4c64e776a9d03
*** cdt git genie on behalf of Andrew Gvozdev ***
bug 345750: convert relative paths to absolute also for those that don't
exist
[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=88c2992cbe5ba299b6e46a10f46c643ba461e4de
*** cdt git genie on behalf of Andrew Gvozdev ***
bug 345750: tidy up cosmetics
[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=54caae0c7a3af39f7d35785f6afc3a018e159d34
*** cdt git genie on behalf of Martin Oberhuber ***
bug 345750: [Scanner Discovery] Per File Build output parser misses
includes with drive-relative paths when sources are behind a linked
resource
[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=656f825db02388ef78e277570d0af5e225ed31f3
|