[cmake-developers] Modifying RPATH feature to run tests uninstalled

Brad King brad.king at kitware.com
Mon Mar 5 09:30:07 EST 2012


On Sun, Mar 4, 2012 at 5:18 PM, Stephen Kelly <steveire at gmail.com> wrote:
>> That's a good start.  However I do not think the logic plays well
>> with BUILD_WITH_INSTALL_RPATH as currently written.  If that is
>> set then the build tree should end up with the install-tree rpath
>> which should be empty if SKIP_INSTALL_RPATH is on.  Therefore
>> the relink/chrpath paths should not need to return true always
>> when SKIP_INSTALL_RPATH is on.
>
> Sorry this took so long. I have updated the branch (It is now
> e96c16ae23885be2e66d67291344369585ebfece)

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

s/for_install/linking_for_install/ because BUILD_WITH_INSTALL_RPATH
should cause the build tree to get the install tree rpath which is
empty when CMAKE_SKIP_INSTALL_RPATH is ON.

>    cm->DefineProperty
> +    ("CMAKE_SKIP_INSTALL_RPATH", cmProperty::VARIABLE,
> +     "Do not include RPATHs in the install tree.",

Please update the documentation of both this variable and
CMAKE_SKIP_RPATH so each references the other.  We want packagers
using CMAKE_SKIP_RPATH to discover CMAKE_SKIP_INSTALL_RPATH.

> +  this->SetPropertyDefault("SKIP_INSTALL_RPATH", 0);

This is unnecessary because it is not a target-level property.  The
new SKIP variable is supposed to be a packager's hammer so we cannot
let projects turn it off for some targets but not others.

> @@ -3625,28 +3627,33 @@ bool cmTarget::NeedRelinkBeforeInstall(const char* config)
...
> +  if(this->Makefile->IsOn("CMAKE_SKIP_INSTALL_RPATH"))
> +    {
> +    return true;
> +    }
> +
>    // If chrpath is going to be used no relinking is needed.
>    if(this->IsChrpathUsed(config))
>      {
>      return false;
>      }
...
> @@ -4013,28 +4021,33 @@ bool cmTarget::IsChrpathUsed(const char* config)
...
> +  if(this->Makefile->IsOn("CMAKE_SKIP_INSTALL_RPATH"))
> +    {
> +    return true;
> +    }
> +
>    // Allow the user to disable builtin chrpath explicitly.
>    if(this->Makefile->IsOn("CMAKE_NO_BUILTIN_CHRPATH"))
>      {
>      return false;
>      }

Drop these tests.  These two methods are about deciding how to rewrite
the RPATH during installation.  The code below these hunks decides
whether it is even possible to do.

Just changes to

  cmComputeLinkInformation::GetRPath
  cmTarget::HaveInstallTreeRPATH
  cmTarget::GetInstallNameDirForInstallTree

should be enough for the rest of the logic to work.

Thanks,
-Brad



More information about the cmake-developers mailing list