[llvm] r242102 - [CMake] We shouldn't be storing values in the cache unless they actually need CMake cache behavior.

[Daniel Sanders]

> -----Original Message-----
> From: Chris Bieneman [mailto:beanz at apple.com]
> Sent: 16 July 2015 17:54
> To: Daniel Sanders
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm] r242102 - [CMake] We shouldn't be storing values in the
> cache unless they actually need CMake cache behavior.
> 
> 
> > On Jul 16, 2015, at 8:35 AM, Daniel Sanders <Daniel.Sanders at imgtec.com>
> wrote:
> >
> > Hi Chris,
> >
> > This isn't a problem but I thought I should mention it. One side effect of this
> change is that these variables are no longer present in ccmake's interface
> unless you have already set them manually. I'm in the habit of running cmake
> to get an initial config then ccmake to edit these variables so I was confused
> by their sudden disappearance :-).
> >
> >> There is also a temporary change to remove non-default values from the
> >> cache if they are already present. This should have the impact of cleaning
> out
> >> unncecissary values from the caches on the buildbots and people's local
> build
> >> directories.
> >
> > Did you mean "non-default" there?
> 
> That is basically what I meant. If they are non-default then caching them has
> no value other than populating the UIs and causing issues because CMake
> doesn’t implement any form of cache invalidation.
> 
> I generally thing these options are advanced user options that are not
> commonly used, so I really want to avoid having them in the cache unless
> they are specified by the user explicitly.
> 
> -Chris

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.

> >> -----Original Message-----
> >> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> >> bounces at cs.uiuc.edu] On Behalf Of Chris Bieneman
> >> Sent: 14 July 2015 02:18
> >> To: llvm-commits at cs.uiuc.edu
> >> Subject: [llvm] r242102 - [CMake] We shouldn't be storing values in the
> cache
> >> unless they actually need CMake cache behavior.
> >>
> >> Author: cbieneman
> >> Date: Mon Jul 13 20:17:43 2015
> >> New Revision: 242102
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=242102&view=rev
> >> Log:
> >> [CMake] We shouldn't be storing values in the cache unless they actually
> >> need CMake cache behavior.
> >>
> >> add_llvm_external_project puts
> >> LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR into the cache even if it is
> >> just the in-tree default path. This causes all sorts of oddness, and makes it
> so
> >> that I can't change the behavior of this variable.
> >>
> >> This patch never puts LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR
> into
> >> the cache. It will only end up in the cache if it is specified on the command
> >> line, which is the correct behavior.
> >>
> >> There is also a temporary change to remove non-default values from the
> >> cache if they are already present. This should have the impact of cleaning
> out
> >> unncecissary values from the caches on the buildbots and people's local
> build
> >> directories. This part of the change is marked with a TODO and can be
> >> removed in a few days.
> >>
> >> Modified:
> >>    llvm/trunk/cmake/modules/AddLLVM.cmake
> >>
> >> Modified: llvm/trunk/cmake/modules/AddLLVM.cmake
> >> URL: http://llvm.org/viewvc/llvm-
> >>
> project/llvm/trunk/cmake/modules/AddLLVM.cmake?rev=242102&r1=24210
> >> 1&r2=242102&view=diff
> >>
> ==========================================================
> >> ====================
> >> --- llvm/trunk/cmake/modules/AddLLVM.cmake (original)
> >> +++ llvm/trunk/cmake/modules/AddLLVM.cmake Mon Jul 13 20:17:43
> 2015
> >> @@ -689,10 +689,15 @@ macro(add_llvm_external_project name)
> >>   list(APPEND LLVM_IMPLICIT_PROJECT_IGNORE
> >> "${CMAKE_CURRENT_SOURCE_DIR}/${add_llvm_external_dir}")
> >>   string(REPLACE "-" "_" nameUNDERSCORE ${name})
> >>   string(TOUPPER ${nameUNDERSCORE} nameUPPER)
> >> -  set(LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR
> >> "${CMAKE_CURRENT_SOURCE_DIR}/${add_llvm_external_dir}"
> >> -      CACHE PATH "Path to ${name} source directory")
> >> -  if (NOT ${LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR} STREQUAL ""
> >> -      AND EXISTS
> >> ${LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR}/CMakeLists.txt)
> >> +  #TODO: Remove this check in a few days once it has circulated through
> >> +  # buildbots and people's checkouts (cbieneman - July 14, 2015)
> >> +  if(${LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR} STREQUAL
> >> "${CMAKE_CURRENT_SOURCE_DIR}/${add_llvm_external_dir}")
> >> +    unset(LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR CACHE)
> >> +  endif()
> >> +  if(NOT LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR)
> >> +    set(LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR
> >> "${CMAKE_CURRENT_SOURCE_DIR}/${add_llvm_external_dir}")
> >> +  endif()
> >> +  if (EXISTS
> >> ${LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR}/CMakeLists.txt)
> >>     option(LLVM_EXTERNAL_${nameUPPER}_BUILD
> >>            "Whether to build ${name} as part of LLVM" ON)
> >>     if (LLVM_EXTERNAL_${nameUPPER}_BUILD)
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits