<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jul 17, 2015, at 2:03 AM, Daniel Sanders <<a href="mailto:Daniel.Sanders@imgtec.com" class="">Daniel.Sanders@imgtec.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div style="" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">-----Original Message-----<br class="">From: Chris Bieneman [<a href="mailto:beanz@apple.com" class="">mailto:beanz@apple.com</a>]<br class="">Sent: 16 July 2015 17:54<br class="">To: Daniel Sanders<br class="">Cc:<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">Subject: Re: [llvm] r242102 - [CMake] We shouldn't be storing values in the<br class="">cache unless they actually need CMake cache behavior.<br class=""><br class=""><br class=""><blockquote type="cite" class="">On Jul 16, 2015, at 8:35 AM, Daniel Sanders <<a href="mailto:Daniel.Sanders@imgtec.com" class="">Daniel.Sanders@imgtec.com</a>><br class=""></blockquote>wrote:<br class=""><blockquote type="cite" class=""><br class="">Hi Chris,<br class=""><br class="">This isn't a problem but I thought I should mention it. One side effect of this<br class=""></blockquote>change is that these variables are no longer present in ccmake's interface<br class="">unless you have already set them manually. I'm in the habit of running cmake<br class="">to get an initial config then ccmake to edit these variables so I was confused<br class="">by their sudden disappearance :-).<br class=""><blockquote type="cite" class=""><br class=""><blockquote type="cite" class="">There is also a temporary change to remove non-default values from the<br class="">cache if they are already present. This should have the impact of cleaning<br class=""></blockquote></blockquote>out<br class=""><blockquote type="cite" class=""><blockquote type="cite" class="">unncecissary values from the caches on the buildbots and people's local<br class=""></blockquote></blockquote>build<br class=""><blockquote type="cite" class=""><blockquote type="cite" class="">directories.<br class=""></blockquote><br class="">Did you mean "non-default" there?<br class=""></blockquote><br class="">That is basically what I meant. If they are non-default then caching them has<br class="">no value other than populating the UIs and causing issues because CMake<br class="">doesn’t implement any form of cache invalidation.<br class=""><br class="">I generally thing these options are advanced user options that are not<br class="">commonly used, so I really want to avoid having them in the cache unless<br class="">they are specified by the user explicitly.<br class=""><br class="">-Chris<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">My reading of the patch is that it removes default-valued variables from the cache and continues to cache non-default values. This makes sense to me since we can reconstruct default values (by using the default) but not non-default values (we'd need to ask the user for it again). However, it's the opposite way around to my reading of the commit message.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div><div>That’s exactly what it does. I am apparently suffering from severe befuddling of words.</div><div><br class=""></div><div>The point of this patch is to remove values from the cache for default, in-tree paths to tools. After running CMake with this patch the only LLVM_EXTERNAL_*_SOURCE_DIR values that will remain in the cache are non-default values.</div><div><br class=""></div><div>I needed to land this change so that I could re-land r242059. As per Takumi’s feedback that patch changes the behavior of LLVM_EXTERNAL_*_SOURCE_DIR slightly so that it is ignored if the default in-tree path exists. The change in behavior and how the value is used in CMake caused failures across the board because the default values were all previously cached.</div><div><br class=""></div><div>Most of this patch (specifically the unset bit) I’m going to remove today when I re-land r242059 because the caches on all the builders (and most developer machines) should be sufficiently cleaned up.</div><div><br class=""></div><div>I’m sorry for the poor wording in my commit message and my email responses.</div><div><br class=""></div><div>-Chris</div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" class=""><blockquote type="cite" class="">-----Original Message-----<br class="">From: <a href="mailto:llvm-commits-bounces@cs.uiuc.edu" class="">llvm-commits-bounces@cs.uiuc.edu</a> [mailto:llvm-commits-<br class=""><a href="mailto:bounces@cs.uiuc.edu" class="">bounces@cs.uiuc.edu</a>] On Behalf Of Chris Bieneman<br class="">Sent: 14 July 2015 02:18<br class="">To: <a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">Subject: [llvm] r242102 - [CMake] We shouldn't be storing values in the<br class=""></blockquote></blockquote>cache<br class=""><blockquote type="cite" class=""><blockquote type="cite" class="">unless they actually need CMake cache behavior.<br class=""><br class="">Author: cbieneman<br class="">Date: Mon Jul 13 20:17:43 2015<br class="">New Revision: 242102<br class=""><br class="">URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D242102-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=C849IHO1jm0P03uZUQavqNN6XO06FBQzEV6dFvvKE_o&s=Vgq7_6v4KxfxttVmPNZoxUoiMPzH1hOgSXxnItDOyz8&e=" class="">http://llvm.org/viewvc/llvm-project?rev=242102&view=rev</a><br class="">Log:<br class="">[CMake] We shouldn't be storing values in the cache unless they actually<br class="">need CMake cache behavior.<br class=""><br class="">add_llvm_external_project puts<br class="">LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR into the cache even if it is<br class="">just the in-tree default path. This causes all sorts of oddness, and makes it<br class=""></blockquote></blockquote>so<br class=""><blockquote type="cite" class=""><blockquote type="cite" class="">that I can't change the behavior of this variable.<br class=""><br class="">This patch never puts LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR<br class=""></blockquote></blockquote>into<br class=""><blockquote type="cite" class=""><blockquote type="cite" class="">the cache. It will only end up in the cache if it is specified on the command<br class="">line, which is the correct behavior.<br class=""><br class="">There is also a temporary change to remove non-default values from the<br class="">cache if they are already present. This should have the impact of cleaning<br class=""></blockquote></blockquote>out<br class=""><blockquote type="cite" class=""><blockquote type="cite" class="">unncecissary values from the caches on the buildbots and people's local<br class=""></blockquote></blockquote>build<br class=""><blockquote type="cite" class=""><blockquote type="cite" class="">directories. This part of the change is marked with a TODO and can be<br class="">removed in a few days.<br class=""><br class="">Modified:<br class=""> llvm/trunk/cmake/modules/AddLLVM.cmake<br class=""><br class="">Modified: llvm/trunk/cmake/modules/AddLLVM.cmake<br class="">URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2D&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=C849IHO1jm0P03uZUQavqNN6XO06FBQzEV6dFvvKE_o&s=lZxMFhELvqME23VE8A9PzNBxszjBC08WnKGUwgiM4xk&e=" class="">http://llvm.org/viewvc/llvm-</a><br class=""><br class=""></blockquote></blockquote>project/llvm/trunk/cmake/modules/AddLLVM.cmake?rev=242102&r1=24210<br class=""><blockquote type="cite" class=""><blockquote type="cite" class="">1&r2=242102&view=diff<br class=""><br class=""></blockquote></blockquote>==========================================================<br class=""><blockquote type="cite" class=""><blockquote type="cite" class="">====================<br class="">--- llvm/trunk/cmake/modules/AddLLVM.cmake (original)<br class="">+++ llvm/trunk/cmake/modules/AddLLVM.cmake Mon Jul 13 20:17:43<br class=""></blockquote></blockquote>2015<br class=""><blockquote type="cite" class=""><blockquote type="cite" class="">@@ -689,10 +689,15 @@ macro(add_llvm_external_project name)<br class=""> list(APPEND LLVM_IMPLICIT_PROJECT_IGNORE<br class="">"${CMAKE_CURRENT_SOURCE_DIR}/${add_llvm_external_dir}")<br class=""> string(REPLACE "-" "_" nameUNDERSCORE ${name})<br class=""> string(TOUPPER ${nameUNDERSCORE} nameUPPER)<br class="">- set(LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR<br class="">"${CMAKE_CURRENT_SOURCE_DIR}/${add_llvm_external_dir}"<br class="">- CACHE PATH "Path to ${name} source directory")<br class="">- if (NOT ${LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR} STREQUAL ""<br class="">- AND EXISTS<br class="">${LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR}/CMakeLists.txt)<br class="">+ #TODO: Remove this check in a few days once it has circulated through<br class="">+ # buildbots and people's checkouts (cbieneman - July 14, 2015)<br class="">+ if(${LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR} STREQUAL<br class="">"${CMAKE_CURRENT_SOURCE_DIR}/${add_llvm_external_dir}")<br class="">+ unset(LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR CACHE)<br class="">+ endif()<br class="">+ if(NOT LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR)<br class="">+ set(LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR<br class="">"${CMAKE_CURRENT_SOURCE_DIR}/${add_llvm_external_dir}")<br class="">+ endif()<br class="">+ if (EXISTS<br class="">${LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR}/CMakeLists.txt)<br class=""> option(LLVM_EXTERNAL_${nameUPPER}_BUILD<br class=""> "Whether to build ${name} as part of LLVM" ON)<br class=""> if (LLVM_EXTERNAL_${nameUPPER}_BUILD)<br class=""><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</blockquote></blockquote></blockquote></div></div></blockquote></div><br class=""></body></html>