<div class="gmail_quote">On Tue, Jan 31, 2012 at 9:34 AM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I noticed a small regression when using --with-cxx-include-dir (/32<br>
was not being added to the lib search dir).</blockquote><div><br></div><div>Ouch!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> The good news is that the<br>

new logic for detecting gcc installations progressed so much that we<br>
should be able to remove most of the special cases.<br></blockquote><div><br></div><div>And yay! ;]</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The attached patches replace the existing options with a single<br>

--with-cxx-install-dir and the only thing the new option does is point<br>
the existing detection logic to the directory it is given.<br>
<br>
Are the patches OK?</blockquote><div><br></div><div> I have a few issues with the patches as-is, and a higher-level request which may make sense as part of this patch, or as a follow up patch.</div><div><br></div><div>My issue with the current patch is not at all to do with the *functionality* but with the naming and documentation of the functionality. Especially as this is getting away from a pile of FIXMEs and toward a really sustainable pattern.</div>
<div><br></div><div>'with-cxx-install-root' is the flag, and the docs then talk about 'libstdc++'. This made me want to rename the flag 'with-libstdcxx-install-root' at first, but then I looked at how you used it. You're just providing a root to look for the GCC installation in, not a root for libstdc++. This root will impact what linker is invoke, what assembler (if one is needed), etc. That makes me think it should be 'with-gcc-install-root'.</div>
<div><br></div><div>However, what the user should place in this is also a bit confusing, because it's not really the root of the GCC installation, its the "prefix" (in configure terminology) of the GCC installation, such as "/usr", "/usr/local", etc. I'm not sure what the best way to clarify this is... 'with-gcc-install-prefix' is close, and maybe with some examples in the docs for the configure flag?</div>
<div><br></div><div>Finally, can you update CMake as well? If not, I can, I'm just being a bit lazy. ;]</div><div><br></div><div><br></div><div>Now for the high-level issue: I really like this logic, but I note that no tests get updated, and no tests broke when I refactored things. I think we need to shift the way this is implemented to something we can test going forward. In the process I think we can also make the entire thing more flexible: let's make it a run-time flag, not (just) a configure-time flag. Then the configure-time flag can setup a default for the runtime flag, and we can test the actual behavior of the runtime flag in the regression test suite. Thoughts?</div>
<div><br></div><div>-Chandler</div></div>