[cmake-developers] Generator expressions for install destination

Brad King brad.king at kitware.com
Mon Sep 21 13:54:35 EDT 2015


On 09/18/2015 03:49 PM, Robert Goulet wrote:
> Here's a version that is more conservative. It doesn't change
> the install(EXPORT) behavior.

> install(TARGET) already supported genex

Right.  I'd forgotten about these changes:

 install: Allow generator expressions in TARGETS DESTINATION (#14317)
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=f30022eb
 http://www.cmake.org/Bug/view.php?id=14317#c37959

>, so basically this patch adds install(FILES) destination genex.

Okay.  I see many hunks of this form:

> -                       this->Destination,
> +                       this->GetDestination(),

Making the Destination member private and moving GetDestination()
back into cmInstallGenerator is refactoring that should be split
out into its own commit.  Note that GetDestination() was removed
from the base class here:

 cmInstallGenerator: Move GetDestination to subclasses that need it
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=f99991db

Restoring it will need a justification in the commit message.
I'm concerned about having both overloads avilable:

> +  std::string GetDestination() const;
> +  std::string GetDestination(std::string const& config) const;

when only one is safe to call from each subclass.  This is why the
above-linked commit removed the method from cmInstallGenerator in
favor of having the needed overload in each subclass.

> -  cmInstallGenerator(const char* destination,
> +  cmInstallGenerator(cmMakefile* mf,
> +                     const char* destination,

This and its related changes are also refactoring that should go
in its own commit.

> +  // We need per-config actions if destination have generator expressions.
> +  if(cmGeneratorExpression::Find(Destination) != std::string::npos)
> +    {
> +    this->ActionsPerConfig = true;
> +    }

Was this the solution to the availability of a configuration for
calls to GetDestination()?

> Perhaps we should update the Help to only mention install(FILES)
> destination instead of all variations of install?

Yes, the actual availability of the behavior should be documented.

Also please look at making the test suite actually verify that
the installation works as expected.  The above-linked commit
f30022eb adds a test that fails if generator expressions are
not evaluated to the correct values.

Thanks,
-Brad



More information about the cmake-developers mailing list