<table><tr><td style="">mwolff requested changes to this revision.<br />mwolff added a comment.<br />This revision now requires changes to proceed.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D11136">View Revision</a></tr></table><br /><div><div><p>cool! but needs some work :)</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11136#inline-54055">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ibuildsystemmanager.h:137</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> */</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">virtual</span> <span class="n">Path</span> <span style="color: #004012">findCompiler</span><span class="p">(</span><span class="n">KDevelop</span><span style="color: #aa2211">::</span><span class="n">ProjectTargetItem</span><span style="color: #aa2211">*</span> <span class="n">p</span><span class="p">)</span> <span style="color: #aa4000">const</span> <span class="p">{</span> <span class="n">Q_UNUSED</span><span class="p">(</span><span class="n">p</span><span class="p">);</span> <span style="color: #aa4000">return</span> <span class="p">{};</span> <span class="p">};</span>
</div><div style="padding: 0 8px; margin: 0 4px; "><span class="p">};</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">make it pure virtual like the rest of this interface</p>
<p style="padding: 0; margin: 8px;">rename to <tt style="background: #ebebeb; font-size: 13px;">compiler</tt></p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11136#inline-54056">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakemanager.cpp:1002</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">targetInfo</span><span class="p">.</span><span class="n">sources</span><span class="p">.</span><span class="n">isEmpty</span><span class="p">())</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">qDebug</span><span class="p">()</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"could not find target"</span> <span style="color: #aa2211"><<</span> <span class="n">item</span><span style="color: #aa2211">-></span><span class="n">text</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">return</span> <span class="p">{};</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">category-based logging, also below</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11136#inline-54057">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakemanager.cpp:1009</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">lang</span><span class="p">.</span><span class="n">isEmpty</span><span class="p">())</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">qDebug</span><span class="p">()</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"no lang for..."</span> <span style="color: #aa2211"><<</span> <span class="n">item</span> <span style="color: #aa2211"><<</span> <span class="n">item</span><span style="color: #aa2211">-></span><span class="n">text</span><span class="p">()</span> <span style="color: #aa2211"><<</span> <span class="n">info</span><span class="p">.</span><span class="n">defines</span> <span style="color: #aa2211"><<</span> <span class="n">targetInfo</span><span class="p">.</span><span class="n">sources</span><span class="p">.</span><span class="n">constFirst</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">return</span> <span class="p">{};</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">remove ...</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11136#inline-54058">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakemanager.cpp:1012</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">const</span> <span class="n">QString</span> <span class="n">var</span> <span style="color: #aa2211">=</span> <span class="n">QStringLiteral</span><span class="p">(</span><span style="color: #766510">"CMAKE_"</span><span class="p">)</span> <span style="color: #aa2211">+</span> <span class="n">lang</span> <span style="color: #aa2211">+</span> <span class="n">QStringLiteral</span><span class="p">(</span><span style="color: #766510">"_COMPILER"</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">const</span> <span style="color: #aa4000">auto</span> <span class="n">ret</span> <span style="color: #aa2211">=</span> <span class="n">CMake</span><span style="color: #aa2211">::</span><span class="n">readCacheValues</span><span class="p">(</span><span class="n">KDevelop</span><span style="color: #aa2211">::</span><span class="n">Path</span><span class="p">(</span><span class="n">buildDirectory</span><span class="p">(</span><span class="n">item</span><span class="p">),</span> <span class="n">QStringLiteral</span><span class="p">(</span><span style="color: #766510">"CMakeCache.txt"</span><span class="p">)),</span> <span class="p">{</span><span class="n">var</span><span class="p">});</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">use QLatin1String since you are concatenating</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11136#inline-54060">View Inline</a><span style="color: #4b4d51; font-weight: bold;">compilerprovider.cpp:39</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "><span style="color: #304a96">#include</span> <span class="cpf"><QDir></span><span style="color: #304a96"></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#include</span> <span class="cpf"><iprojectcontroller.h></span><span style="color: #304a96"></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">#include</span> <span class="cpf"><interfaces/ibuildsystemmanager.h></span><span style="color: #304a96"></span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">move up, add interfaces/</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11136#inline-54061">View Inline</a><span style="color: #4b4d51; font-weight: bold;">compilerprovider.cpp:157</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">qCDebug</span><span class="p">(</span><span class="n">DEFINESANDINCLUDES</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"found compiler"</span> <span style="color: #aa2211"><<</span> <span class="n">path</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">path</span><span class="p">.</span><span class="n">isEmpty</span><span class="p">()</span> <span style="color: #aa2211">||</span> <span style="color: #aa2211">!</span><span class="n">QDir</span><span style="color: #aa2211">::</span><span class="n">isAbsolutePath</span><span class="p">(</span><span class="n">path</span><span class="p">.</span><span class="n">toLocalFile</span><span class="p">()))</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">continue</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">this isAbsolutePath check - shouldn't it be an assertion? the BSM should sanitize and produce absolute paths, I'd say (like, based on the build/source dir)</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11136#inline-54062">View Inline</a><span style="color: #4b4d51; font-weight: bold;">compilerprovider.cpp:162</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">bool</span> <span class="n">alreadyPresent</span> <span style="color: #aa2211">=</span> <span style="color: #304a96">false</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">for</span> <span class="p">(</span> <span style="color: #aa4000">const</span> <span class="n">CompilerPointer</span><span style="color: #aa2211">&</span> <span style="color: #a0a000">compiler</span> <span class="p">:</span> <span class="n">m_compilers</span> <span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">compiler</span><span style="color: #aa2211">-></span><span class="n">path</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span class="n">pathString</span><span class="p">)</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;"><tt style="background: #ebebeb; font-size: 13px;">for (const auto& compiler : m_compilers)</tt></p>
<p style="padding: 0; margin: 8px;">or better yet:</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">if (std::any_of(m_compilers.begin(), m_compilers.end(),
[pathString](const auto& compiler) { compiler->path() == pathString; })
{
continue;
}</pre></div></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D11136#inline-54064">View Inline</a><span style="color: #4b4d51; font-weight: bold;">compilerprovider.cpp:171</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #74777d">//we need to search, sdk compiler names are weird: arm-linux-androideabi-g++</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">int</span> <span class="n">factory</span> <span style="color: #aa2211">=</span> <span class="n">path</span><span class="p">.</span><span class="n">lastPathSegment</span><span class="p">().</span><span class="n">contains</span><span class="p">(</span><span class="n">QLatin1String</span><span class="p">(</span><span style="color: #766510">"clang"</span><span class="p">))</span> <span style="color: #aa2211">?</span> <span style="color: #601200">1</span> <span style="color: #aa2211">:</span> <span style="color: #601200">0</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">auto</span> <span class="n">compiler</span> <span style="color: #aa2211">=</span> <span class="n">m_factories</span><span class="p">[</span><span class="n">factory</span><span class="p">]</span><span style="color: #aa2211">-></span><span class="n">createCompiler</span><span class="p">(</span><span class="n">path</span><span class="p">.</span><span class="n">lastPathSegment</span><span class="p">(),</span> <span class="n">pathString</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">this looks weird to me - this shouldn't be hardcoded like this, no? it should go through all factories and find a match. I.e. on windows we may need to use the msvc factory?</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R32 KDevelop</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D11136">https://phabricator.kde.org/D11136</a></div></div><br /><div><strong>To: </strong>apol, KDevelop, mwolff<br /><strong>Cc: </strong>mwolff, kdevelop-devel<br /></div>