Community
Participate
Working Groups
*** Cloned from bug 323011 comment 12 *** I cloned the task to follow up with the suggested patch in future release. Created attachment 177313 [details] A better pach (against 6.0.2+ Stream) I did some more experiments (on the 6.0.2 branch), and found that files behind linked resources were still not properly associated. Part of this was due to bug 323498, but the other part was a problem with this fix. Attached is an alternative fix that works perfectly fine for me, and seems to run about 5 times faster than your fix in terms of Performance (I tried on an actual 1MB build log). I assume that the performance improvement is because with the fix, include paths that are already known need to be searched much less frequently. Note that this fix alone solves the problem for me in 6.0.2, so your other two changes don't seem to be needed. Could this be considered for all CDT Streams including 6.0.x ?
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
