From fc8955e8891c645cd369a3cc8b607a14a8ed5bd7 Mon Sep 17 00:00:00 2001
From: Kyle Edwards <kyle.edwards@kitware.com>
Date: Tue, 2 Oct 2018 16:38:26 -0400
Subject: [PATCH] add_subdirectory: Run subdirectory install rules in correct
order
Before this change, install rules created by add_subdirectory()
would be executed after all of the top-level install rules, even
if they were declared before the top-level rules. This change
adds a new policy, CMP0082, which interleaves the add_subdirectory()
install rules with the other install rules so they are run in the
correct order.
---
Help/manual/cmake-policies.7.rst | 8 ++
Help/policy/CMP0082.rst | 24 ++++++
.../dev/install-subdirectory-order.rst | 5 ++
.../variable/CMAKE_POLICY_WARNING_CMPNNNN.rst | 2 +
Source/CMakeLists.txt | 2 +
Source/cmInstallGenerator.cxx | 13 ++++
Source/cmInstallGenerator.h | 4 +
Source/cmInstallScriptGenerator.cxx | 4 +-
Source/cmInstallSubdirectoryGenerator.cxx | 77 +++++++++++++++++++
Source/cmInstallSubdirectoryGenerator.h | 41 ++++++++++
Source/cmLocalGenerator.cxx | 63 +++++++++++----
Source/cmMakefile.cxx | 4 +
Source/cmPolicies.h | 6 +-
bootstrap | 1 +
14 files changed, 235 insertions(+), 19 deletions(-)
create mode 100644 Help/policy/CMP0082.rst
create mode 100644 Help/release/dev/install-subdirectory-order.rst
create mode 100644 Source/cmInstallSubdirectoryGenerator.cxx
create mode 100644 Source/cmInstallSubdirectoryGenerator.h
diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst
index 2cc52fe1aa..0587429050 100644
--- a/Help/manual/cmake-policies.7.rst
+++ b/Help/manual/cmake-policies.7.rst
@@ -51,6 +51,14 @@ The :variable:`CMAKE_MINIMUM_REQUIRED_VERSION` variable may also be used
to determine whether to report an error on use of deprecated macros or
functions.
+Policies Introduced by CMake 3.14
+=================================
+
+.. toctree::
+ :maxdepth: 1
+
+ CMP0082: Install rules from add_subdirectory() are interleaved with those in caller. </policy/CMP0082>
+
Policies Introduced by CMake 3.13
=================================
diff --git a/Help/policy/CMP0082.rst b/Help/policy/CMP0082.rst
new file mode 100644
index 0000000000..7b2ef04f06
--- /dev/null
+++ b/Help/policy/CMP0082.rst
@@ -0,0 +1,24 @@
+CMP0082
+-------
+
+Install rules from :command:`add_subdirectory` calls are interleaved with
+those in caller.
+
+CMake 3.13 and lower ran the install rules from :command:`add_subdirectory`
+after all other install rules, even if :command:`add_subdirectory` was called
+before the other install rules. CMake 3.14 and later interleaves these
+:command:`add_subdirectory` install rules with the others so that they are
+run in the order they are declared.
+
+The ``OLD`` behavior for this policy is to run the install rules from
+:command:`add_subdirectory` after the other install rules. The ``NEW``
+behavior for this policy is to run all install rules in the order they are
+declared.
+
+This policy was introduced in CMake version 3.14. Unlike most policies,
+CMake version |release| does *not* warn by default when this policy
+is not set and simply uses OLD behavior. See documentation of the
+:variable:`CMAKE_POLICY_WARNING_CMP0082 <CMAKE_POLICY_WARNING_CMP<NNNN>>`
+variable to control the warning.
+
+.. include:: DEPRECATED.txt
diff --git a/Help/release/dev/install-subdirectory-order.rst b/Help/release/dev/install-subdirectory-order.rst
new file mode 100644
index 0000000000..c52e6176a3
--- /dev/null
+++ b/Help/release/dev/install-subdirectory-order.rst
@@ -0,0 +1,5 @@
+install-subdirectory-order
+--------------------------
+
+* Install rules under :command:`add_subdirectory` now interleave with those in
+ the calling directory. See policy :policy:`CMP0082` for details.
diff --git a/Help/variable/CMAKE_POLICY_WARNING_CMPNNNN.rst b/Help/variable/CMAKE_POLICY_WARNING_CMPNNNN.rst
index aa23b65f9f..d179728735 100644
--- a/Help/variable/CMAKE_POLICY_WARNING_CMPNNNN.rst
+++ b/Help/variable/CMAKE_POLICY_WARNING_CMPNNNN.rst
@@ -19,6 +19,8 @@ warn by default:
policy :policy:`CMP0066`.
* ``CMAKE_POLICY_WARNING_CMP0067`` controls the warning for
policy :policy:`CMP0067`.
+* ``CMAKE_POLICY_WARNING_CMP0082`` controls the warning for
+ policy :policy:`CMP0082`.
This variable should not be set by a project in CMake code. Project
developers running CMake may set this variable in their cache to
diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt
index 3cf6c8f871..5e2758adfa 100644
--- a/Source/CMakeLists.txt
+++ b/Source/CMakeLists.txt
@@ -260,6 +260,8 @@ set(SRCS
cmInstallFilesGenerator.cxx
cmInstallScriptGenerator.h
cmInstallScriptGenerator.cxx
+ cmInstallSubdirectoryGenerator.h
+ cmInstallSubdirectoryGenerator.cxx
cmInstallTargetGenerator.h
cmInstallTargetGenerator.cxx
cmInstallDirectoryGenerator.h
diff --git a/Source/cmInstallGenerator.cxx b/Source/cmInstallGenerator.cxx
index 53ac71622d..2b236584a4 100644
--- a/Source/cmInstallGenerator.cxx
+++ b/Source/cmInstallGenerator.cxx
@@ -22,6 +22,19 @@ cmInstallGenerator::~cmInstallGenerator()
{
}
+bool cmInstallGenerator::HaveInstall()
+{
+ return true;
+}
+
+void cmInstallGenerator::CheckCMP0082(bool& haveSubdirectoryInstall,
+ bool& haveInstallAfterSubdirectory)
+{
+ if (haveSubdirectoryInstall) {
+ haveInstallAfterSubdirectory = true;
+ }
+}
+
void cmInstallGenerator::AddInstallRule(
std::ostream& os, std::string const& dest, cmInstallType type,
std::vector<std::string> const& files, bool optional /* = false */,
diff --git a/Source/cmInstallGenerator.h b/Source/cmInstallGenerator.h
index fc1ce864f7..e5b88c3858 100644
--- a/Source/cmInstallGenerator.h
+++ b/Source/cmInstallGenerator.h
@@ -38,6 +38,10 @@ public:
bool exclude_from_all);
~cmInstallGenerator() override;
+ virtual bool HaveInstall();
+ virtual void CheckCMP0082(bool& haveSubdirectoryInstall,
+ bool& haveInstallAfterSubdirectory);
+
void AddInstallRule(
std::ostream& os, std::string const& dest, cmInstallType type,
std::vector<std::string> const& files, bool optional = false,
diff --git a/Source/cmInstallScriptGenerator.cxx b/Source/cmInstallScriptGenerator.cxx
index 3a90f4c8d7..1d7784efb8 100644
--- a/Source/cmInstallScriptGenerator.cxx
+++ b/Source/cmInstallScriptGenerator.cxx
@@ -37,9 +37,9 @@ void cmInstallScriptGenerator::AddScriptInstallRule(std::ostream& os,
std::string const& script)
{
if (this->Code) {
- os << indent.Next() << script << "\n";
+ os << indent << script << "\n";
} else {
- os << indent.Next() << "include(\"" << script << "\")\n";
+ os << indent << "include(\"" << script << "\")\n";
}
}
diff --git a/Source/cmInstallSubdirectoryGenerator.cxx b/Source/cmInstallSubdirectoryGenerator.cxx
new file mode 100644
index 0000000000..ca9f13454c
--- /dev/null
+++ b/Source/cmInstallSubdirectoryGenerator.cxx
@@ -0,0 +1,77 @@
+/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying
+ file Copyright.txt or https://cmake.org/licensing for details. */
+#include "cmInstallSubdirectoryGenerator.h"
+
+#include "cmLocalGenerator.h"
+#include "cmMakefile.h"
+#include "cmPolicies.h"
+#include "cmScriptGenerator.h"
+#include "cmSystemTools.h"
+
+#include <sstream>
+#include <vector>
+
+cmInstallSubdirectoryGenerator::cmInstallSubdirectoryGenerator(
+ cmMakefile* makefile, const char* binaryDirectory, bool excludeFromAll)
+ : cmInstallGenerator(nullptr, std::vector<std::string>(), nullptr,
+ MessageDefault, excludeFromAll)
+ , Makefile(makefile)
+ , BinaryDirectory(binaryDirectory)
+{
+}
+
+cmInstallSubdirectoryGenerator::~cmInstallSubdirectoryGenerator()
+{
+}
+
+bool cmInstallSubdirectoryGenerator::HaveInstall()
+{
+ for (auto generator : this->Makefile->GetInstallGenerators()) {
+ if (generator->HaveInstall()) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
+void cmInstallSubdirectoryGenerator::CheckCMP0082(
+ bool& haveSubdirectoryInstall, bool& /*unused*/)
+{
+ if (this->HaveInstall()) {
+ haveSubdirectoryInstall = true;
+ }
+}
+
+void cmInstallSubdirectoryGenerator::Compute(cmLocalGenerator* lg)
+{
+ this->LocalGenerator = lg;
+}
+
+void cmInstallSubdirectoryGenerator::GenerateScript(std::ostream& os)
+{
+ if (!this->ExcludeFromAll) {
+ cmPolicies::PolicyStatus status =
+ this->LocalGenerator->GetPolicyStatus(cmPolicies::CMP0082);
+ switch (status) {
+ case cmPolicies::WARN:
+ case cmPolicies::OLD:
+ // OLD behavior is handled in cmLocalGenerator::GenerateInstallRules()
+ break;
+
+ case cmPolicies::NEW:
+ case cmPolicies::REQUIRED_IF_USED:
+ case cmPolicies::REQUIRED_ALWAYS: {
+ Indent indent;
+ std::string odir = this->BinaryDirectory;
+ cmSystemTools::ConvertToUnixSlashes(odir);
+ os << indent << "if(NOT CMAKE_INSTALL_LOCAL_ONLY)\n"
+ << indent.Next()
+ << "# Include the install script for the subdirectory.\n"
+ << indent.Next() << "include(\"" << odir
+ << "/cmake_install.cmake\")\n"
+ << indent << "endif()\n\n";
+ } break;
+ }
+ }
+}
diff --git a/Source/cmInstallSubdirectoryGenerator.h b/Source/cmInstallSubdirectoryGenerator.h
new file mode 100644
index 0000000000..35471dda05
--- /dev/null
+++ b/Source/cmInstallSubdirectoryGenerator.h
@@ -0,0 +1,41 @@
+/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying
+ file Copyright.txt or https://cmake.org/licensing for details. */
+#ifndef cmInstallSubdirectoryGenerator_h
+#define cmInstallSubdirectoryGenerator_h
+
+#include "cmConfigure.h" // IWYU pragma: keep
+
+#include "cmInstallGenerator.h"
+
+#include <iosfwd>
+#include <string>
+
+class cmLocalGenerator;
+class cmMakefile;
+
+/** \class cmInstallSubdirectoryGenerator
+ * \brief Generate target installation rules.
+ */
+class cmInstallSubdirectoryGenerator : public cmInstallGenerator
+{
+public:
+ cmInstallSubdirectoryGenerator(cmMakefile* makefile,
+ const char* binaryDirectory,
+ bool excludeFromAll);
+ ~cmInstallSubdirectoryGenerator() override;
+
+ bool HaveInstall() override;
+ void CheckCMP0082(bool& haveSubdirectoryInstall,
+ bool& haveInstallAfterSubdirectory) override;
+
+ void Compute(cmLocalGenerator* lg) override;
+
+protected:
+ void GenerateScript(std::ostream& os) override;
+
+ cmMakefile* Makefile;
+ std::string BinaryDirectory;
+ cmLocalGenerator* LocalGenerator;
+};
+
+#endif
diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx
index 70307252e2..21b029ed1e 100644
--- a/Source/cmLocalGenerator.cxx
+++ b/Source/cmLocalGenerator.cxx
@@ -517,31 +517,62 @@ void cmLocalGenerator::GenerateInstallRules()
}
// Ask each install generator to write its code.
+ cmPolicies::PolicyStatus status = this->GetPolicyStatus(cmPolicies::CMP0082);
std::vector<cmInstallGenerator*> const& installers =
this->Makefile->GetInstallGenerators();
- for (cmInstallGenerator* installer : installers) {
- installer->Generate(fout, config, configurationTypes);
+ bool haveSubdirectoryInstall = false;
+ bool haveInstallAfterSubdirectory = false;
+ if (status == cmPolicies::WARN) {
+ for (cmInstallGenerator* installer : installers) {
+ installer->CheckCMP0082(haveSubdirectoryInstall,
+ haveInstallAfterSubdirectory);
+ installer->Generate(fout, config, configurationTypes);
+ }
+ } else {
+ for (cmInstallGenerator* installer : installers) {
+ installer->Generate(fout, config, configurationTypes);
+ }
}
// Write rules from old-style specification stored in targets.
this->GenerateTargetInstallRules(fout, config, configurationTypes);
// Include install scripts from subdirectories.
- std::vector<cmStateSnapshot> children =
- this->Makefile->GetStateSnapshot().GetChildren();
- if (!children.empty()) {
- fout << "if(NOT CMAKE_INSTALL_LOCAL_ONLY)\n";
- fout << " # Include the install script for each subdirectory.\n";
- for (cmStateSnapshot const& c : children) {
- if (!c.GetDirectory().GetPropertyAsBool("EXCLUDE_FROM_ALL")) {
- std::string odir = c.GetDirectory().GetCurrentBinary();
- cmSystemTools::ConvertToUnixSlashes(odir);
- fout << " include(\"" << odir << "/cmake_install.cmake\")"
- << std::endl;
+ switch (status) {
+ case cmPolicies::WARN:
+ if (haveInstallAfterSubdirectory &&
+ this->Makefile->PolicyOptionalWarningEnabled(
+ "CMAKE_POLICY_WARNING_CMP0082")) {
+ std::ostringstream e;
+ e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0082) << "\n";
+ this->IssueMessage(cmake::AUTHOR_WARNING, e.str());
}
- }
- fout << "\n";
- fout << "endif()\n\n";
+ CM_FALLTHROUGH;
+ case cmPolicies::OLD: {
+ std::vector<cmStateSnapshot> children =
+ this->Makefile->GetStateSnapshot().GetChildren();
+ if (!children.empty()) {
+ fout << "if(NOT CMAKE_INSTALL_LOCAL_ONLY)\n";
+ fout << " # Include the install script for each subdirectory.\n";
+ for (cmStateSnapshot const& c : children) {
+ if (!c.GetDirectory().GetPropertyAsBool("EXCLUDE_FROM_ALL")) {
+ std::string odir = c.GetDirectory().GetCurrentBinary();
+ cmSystemTools::ConvertToUnixSlashes(odir);
+ fout << " include(\"" << odir << "/cmake_install.cmake\")"
+ << std::endl;
+ }
+ }
+ fout << "\n";
+ fout << "endif()\n\n";
+ }
+ } break;
+
+ case cmPolicies::REQUIRED_IF_USED:
+ case cmPolicies::REQUIRED_ALWAYS:
+ case cmPolicies::NEW:
+ // NEW behavior is handled in
+ // cmInstallSubdirectoryGenerator::GenerateScript()
+ break;
}
// Record the install manifest.
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index 8d163b77d9..0a69d0930f 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -28,6 +28,7 @@
#include "cmGeneratorExpressionEvaluationFile.h"
#include "cmGlobalGenerator.h"
#include "cmInstallGenerator.h" // IWYU pragma: keep
+#include "cmInstallSubdirectoryGenerator.h"
#include "cmListFileCache.h"
#include "cmSourceFile.h"
#include "cmSourceFileLocation.h"
@@ -1669,6 +1670,9 @@ void cmMakefile::AddSubDirectory(const std::string& srcPath,
} else {
this->UnConfiguredDirectories.push_back(subMf);
}
+
+ this->AddInstallGenerator(new cmInstallSubdirectoryGenerator(
+ subMf, binPath.c_str(), excludeFromAll));
}
const std::string& cmMakefile::GetCurrentSourceDirectory() const
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index a367e47ddd..a469f1ecf4 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -240,7 +240,11 @@ class cmMakefile;
cmPolicies::WARN) \
SELECT(POLICY, CMP0081, \
"Relative paths not allowed in LINK_DIRECTORIES target property.", \
- 3, 13, 0, cmPolicies::WARN)
+ 3, 13, 0, cmPolicies::WARN) \
+ SELECT(POLICY, CMP0082, \
+ "Install rules from add_subdirectory() are interleaved with those " \
+ "in caller.", \
+ 3, 14, 0, cmPolicies::WARN)
#define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
#define CM_FOR_EACH_POLICY_ID(POLICY) \
diff --git a/bootstrap b/bootstrap
index 416a3d6780..6710d1a072 100755
--- a/bootstrap
+++ b/bootstrap
@@ -348,6 +348,7 @@ CMAKE_CXX_SOURCES="\
cmInstallFilesGenerator \
cmInstallGenerator \
cmInstallScriptGenerator \
+ cmInstallSubdirectoryGenerator \
cmInstallTargetGenerator \
cmInstallTargetsCommand \
cmInstalledFile \
--
GitLab