<div dir="ltr">Yes, definitely so.  <div><br></div><div>Then it must be because the previous code only overrode the CC and CXX variables in the "make configure" step.  Of course, how silly of me...  Your definitions are used by the "make" step, because the previous code did not specify CC and CXX there.  I think the former code was incorrect and that the way it is done now is much more clean, albeit perhaps not optimal in terms of retaining the backwards compatibility that I sought.  In other words, you were sort of lucky that it worked before with ccache - because the getLLDBuildFactory() function did not specify its own environment all the places it should, but only the first place.  Strictly speaking, you could see this scenario with the old code:</div>
<div><br></div><div>   CC=gcc CXX=C++ cmake ...</div><div>   CC=clang CXX=clang++ make ...<br><div><br></div><div>I think the new way is the proper way.  Because previously you could not control CC and CXX in the "make configure" step but only in the "make" step.  Now you can control them in every step of the build factory function.</div>
</div><div><br></div><div>I think the best way to fix this is that you add your desired environment variables to the call to getLLDBuildFactory() in zorg/buildbot/builders/LLDBuilder.py.  Sorry for the inconvenience, but I didn't realize that the former code basically was written so unfortunately that it had to break when somebody came along and tried to change it.</div>
<div><br></div><div>If there's consensus that we should keep the old, slightly broken method, I'll of course change it back, but I cannot see any sense in doing that.</div><div><br></div><div><br></div><div>-- Mikael</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">2013/12/10 Dmitri Gribenko <span dir="ltr"><<a href="mailto:gribozavr@gmail.com" target="_blank">gribozavr@gmail.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Mon, Dec 9, 2013 at 11:34 AM, Mikael Lyngvig <<a href="mailto:mikael@lyngvig.org">mikael@lyngvig.org</a>> wrote:<br>
> The patch has been updated and I am re-submitting everything I've submitted<br>
> to Galina as well so that it goes through a full peer review.<br>
><br>
> Phew, thank you a ton for catching that!<br>
><br>
> As far as the CC and CXX environment variables goes, I did it to retain<br>
> backwards compatibility with the existing three LLD builders:<br>
><br>
> -                               env={<br>
> -                                    'CXX': "clang++",<br>
> -                                    'C':   "clang"},<br>
> +                               env=merged_env,<br>
><br>
> So they already got overriden, I have simply made the override overriddable<br>
> :-)  Now you can specify that you want to use ccache in the env argument,<br>
> something you could not do before.  I am also going to use ccache.  I bet<br>
> your lld builds haven't been using ccache, even if you thought they did.<br>
<br>
I think they are using ccache, please take a look:<br>
<br>
<a href="http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-ubuntu-13.04/builds/10304/steps/cmake-configure/logs/stdio" target="_blank">http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-ubuntu-13.04/builds/10304/steps/cmake-configure/logs/stdio</a><br>

<span class="HOEnZb"><font color="#888888"><br>
Dmitri<br>
<br>
--<br>
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if<br>
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <<a href="mailto:gribozavr@gmail.com">gribozavr@gmail.com</a>>*/<br>
</font></span></blockquote></div><br></div>