Re: [PATCH] slightly modify cache variable behaviour, Was: Re: option() behavior

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] slightly modify cache variable behaviour, Was: Re: option() behavior

Alexander Neundorf-3
On Sunday 28 June 2009, Alexander Neundorf wrote:

> On Tuesday 12 May 2009, Philip Lowman wrote:
> > On Mon, May 11, 2009 at 4:53 PM, Alexander Neundorf
> > <[hidden email]
> >
> > > wrote:
> > >
> > > On Monday 11 May 2009, Bill Hoffman wrote:
> > > > Alexander Neundorf wrote:
> > > > > On Monday 11 May 2009, Bill Hoffman wrote:
> > > > >> So, CMake has done what it does now from the start.   There was a
> > >
> > > short
> > >
> > > > >> period of time when it did not, and that was when a re-write was
> > > > >> done, and it quickly broke some existing projects.  Here is the
> > > > >> thread:
> > > > >
> > > > > Yes, I can imagine that.
> > > > > Still the current behaviour is not very intuitive, especially that
> > > > > set(CACHE) in the first cmake run overrides the "visible" variable,
> > >
> > > while
> > >
> > > > > the set(CACHE) in the second run basically does nothing, and so the
> > >
> > > value
> > >
> > > > > of the "visible" variable is different.
> > > > > I think for CMake 2.8 or 3.0 it would be a good idea to make this
> > > > > more consistent and easier to understand (especially since now more
> > > > > and more people are using cmake).
> > > > >
> > > > >> http://www.cmake.org/pipermail/cmake/2007-March/013204.html
> > > > >>
> > > > >> I think the rule should be if you want to change a variable you
> > > > >> have
> > >
> > > to
> > >
> > > > >> have the set AFTER the variable, or use CACHE INTERNAL.
> > > > >
> > > > > What do you mean exactly with "have the set after the variable" ?
> > > > > Have the set(FOO <value>) after the set(FOO <value> CACHE), or
> > >
> > > something
> > >
> > > > > else ?
> > > >
> > > > Yes,
> > > >
> > > > This always has worked as expected:
> > > >
> > > > set(FOO 2 CACHE)
> > > > set(FOO 1)  # this is always the value
> > > >
> > > > However, I think your change is OK.  Basically if the set CACHE comes
> > > > after the set, it will always set the value.   We do need a policy
> > > > for this, but it should be easy to detected.  You will have to see if
> > > > the REMOVE is going to do anything.    I wonder how many warnings
> > > > that policy will cause... I  guess we could create the policy code,
> > > > and try it on VTK, Boost, SecondLife, and some other big projects to
> > > > see what happens...
> > >
> > > Ok, I put it on my TODO, then we'll see what (would) happen.
> >
> > I'll be happy to test it out on our build system at work as well.
> >
> > I never thought to do anything crazy like define a local variable before
> > a cache variable until CMakePorts... so I don't think this is going to
> > impact us at all and I suspect it probably won't affect others much
> > either. Wrapping the cache and option declarations with if(DEFINED...) is
> > no big deal.
>
> The patch attached at http://public.kitware.com/Bug/view.php?id=9008
> implements this changed behaviour, together with a new cmake policy CMP0014
> (OLD behaviour: ignore the value in the cache if the variable has been
> already set, new behaviour: set the variable to the value from the cache)
>
> The cmake tests are all succeeding, and I'm also building kdelibs without
> problems with this patch.
>
> If you think it's ok I can commit it to HEAD.

Any comments on this one ?
If this should get in, I think it should be done before 2.8.0.
I'm using cmake built with this patch since June and didn't notice any issues.

Alex
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://www.cmake.org/mailman/listinfo/cmake
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] slightly modify cache variable behaviour, Was: Re: option() behavior

Alexander Neundorf-3
On Saturday 05 September 2009, Alexander Neundorf wrote:

> On Sunday 28 June 2009, Alexander Neundorf wrote:
> > On Tuesday 12 May 2009, Philip Lowman wrote:
> > > On Mon, May 11, 2009 at 4:53 PM, Alexander Neundorf
> > > <[hidden email]
> > >
> > > > wrote:
> > > >
> > > > On Monday 11 May 2009, Bill Hoffman wrote:
> > > > > Alexander Neundorf wrote:
> > > > > > On Monday 11 May 2009, Bill Hoffman wrote:
> > > > > >> So, CMake has done what it does now from the start.   There was
> > > > > >> a
> > > >
> > > > short
> > > >
> > > > > >> period of time when it did not, and that was when a re-write was
> > > > > >> done, and it quickly broke some existing projects.  Here is the
> > > > > >> thread:
> > > > > >
> > > > > > Yes, I can imagine that.
> > > > > > Still the current behaviour is not very intuitive, especially
> > > > > > that set(CACHE) in the first cmake run overrides the "visible"
> > > > > > variable,
> > > >
> > > > while
> > > >
> > > > > > the set(CACHE) in the second run basically does nothing, and so
> > > > > > the
> > > >
> > > > value
> > > >
> > > > > > of the "visible" variable is different.
> > > > > > I think for CMake 2.8 or 3.0 it would be a good idea to make this
> > > > > > more consistent and easier to understand (especially since now
> > > > > > more and more people are using cmake).
> > > > > >
> > > > > >> http://www.cmake.org/pipermail/cmake/2007-March/013204.html
> > > > > >>
> > > > > >> I think the rule should be if you want to change a variable you
> > > > > >> have
> > > >
> > > > to
> > > >
> > > > > >> have the set AFTER the variable, or use CACHE INTERNAL.
> > > > > >
> > > > > > What do you mean exactly with "have the set after the variable" ?
> > > > > > Have the set(FOO <value>) after the set(FOO <value> CACHE), or
> > > >
> > > > something
> > > >
> > > > > > else ?
> > > > >
> > > > > Yes,
> > > > >
> > > > > This always has worked as expected:
> > > > >
> > > > > set(FOO 2 CACHE)
> > > > > set(FOO 1)  # this is always the value
> > > > >
> > > > > However, I think your change is OK.  Basically if the set CACHE
> > > > > comes after the set, it will always set the value.   We do need a
> > > > > policy for this, but it should be easy to detected.  You will have
> > > > > to see if the REMOVE is going to do anything.    I wonder how many
> > > > > warnings that policy will cause... I  guess we could create the
> > > > > policy code, and try it on VTK, Boost, SecondLife, and some other
> > > > > big projects to see what happens...
> > > >
> > > > Ok, I put it on my TODO, then we'll see what (would) happen.
> > >
> > > I'll be happy to test it out on our build system at work as well.
> > >
> > > I never thought to do anything crazy like define a local variable
> > > before a cache variable until CMakePorts... so I don't think this is
> > > going to impact us at all and I suspect it probably won't affect others
> > > much either. Wrapping the cache and option declarations with
> > > if(DEFINED...) is no big deal.
> >
> > The patch attached at http://public.kitware.com/Bug/view.php?id=9008
> > implements this changed behaviour, together with a new cmake policy
> > CMP0014 (OLD behaviour: ignore the value in the cache if the variable has
> > been already set, new behaviour: set the variable to the value from the
> > cache)
> >
> > The cmake tests are all succeeding, and I'm also building kdelibs without
> > problems with this patch.
> >
> > If you think it's ok I can commit it to HEAD.
>
> Any comments on this one ?
> If this should get in, I think it should be done before 2.8.0.
> I'm using cmake built with this patch since June and didn't notice any
> issues.

I attached an updated version of the patch to
http://public.kitware.com/Bug/view.php?id=9008, the policy is now 0015.

Alex
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://www.cmake.org/mailman/listinfo/cmake