On 3/5/2012 10:47 AM, Stephen Kelly wrote:
e1b4fec8dd73cc675b3a20f2e045c6282c47553a in my gitorious clone contains the
updated state. It behaved as I expected in all combinations I tested.

Much cleaner, thanks.  Here are some more comments.

>      outputRuntime && linking_for_install &&
> -    this->Target->GetPropertyAsBool("INSTALL_RPATH_USE_LINK_PATH");
> +    this->Target->GetPropertyAsBool("INSTALL_RPATH_USE_LINK_PATH") &&
> +    !this->Makefile->IsOn("CMAKE_SKIP_INSTALL_RPATH");

I think this can be simplified to

>      outputRuntime && linking_for_install &&
> +    !this->Makefile->IsOn("CMAKE_SKIP_INSTALL_RPATH") &&
>      this->Target->GetPropertyAsBool("INSTALL_RPATH_USE_LINK_PATH");

which is also a more readable order for the logic IMO.

> -     "This allows for easy running from the build tree.",false,
> +     "This allows for easy running from the build tree.  To omit RPATH"
> +     "in both the install step, but not the build step, use "
> +     "CMAKE_SKIP_INSTALL_RPATH",false,

The wording "in both the install step" looks like a cut-n-paste goober.
Also the sentence should end in a period.

> +     "is always built with RPATH but installed without RPATH.  This can be "

This is imprecise.  The option installs without RPATH whether or not
the build tree gets an RPATH, but the option does not affect the build
tree by itself.

-Brad
--

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://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Reply via email to