From d13ed01d54846d953854e70f2c9796e115b61e7a Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Fri, 6 Sep 2024 17:11:27 +1000 Subject: [PATCH 1/5] Tests: Remove unused files from RunCMake.install --- Tests/RunCMake/install/EXPORT-TargetTwice-pkg1.txt | 5 ----- Tests/RunCMake/install/EXPORT-TargetTwice-pkg2.txt | 5 ----- 2 files changed, 10 deletions(-) delete mode 100644 Tests/RunCMake/install/EXPORT-TargetTwice-pkg1.txt delete mode 100644 Tests/RunCMake/install/EXPORT-TargetTwice-pkg2.txt diff --git a/Tests/RunCMake/install/EXPORT-TargetTwice-pkg1.txt b/Tests/RunCMake/install/EXPORT-TargetTwice-pkg1.txt deleted file mode 100644 index 592f79c688..0000000000 --- a/Tests/RunCMake/install/EXPORT-TargetTwice-pkg1.txt +++ /dev/null @@ -1,5 +0,0 @@ -.+ -set_target_properties\(pkg1::foo PROPERTIES -.+INTERFACE_INCLUDE_DIRECTORIES "\${_IMPORT_PREFIX}/pkg1/inc" -\) -.+ diff --git a/Tests/RunCMake/install/EXPORT-TargetTwice-pkg2.txt b/Tests/RunCMake/install/EXPORT-TargetTwice-pkg2.txt deleted file mode 100644 index ebfc43e344..0000000000 --- a/Tests/RunCMake/install/EXPORT-TargetTwice-pkg2.txt +++ /dev/null @@ -1,5 +0,0 @@ -.+ -set_target_properties\(pkg2::foo PROPERTIES -.+INTERFACE_INCLUDE_DIRECTORIES "\${_IMPORT_PREFIX}/pkg2/inc" -\) -.+ From d810374b3dc324df9cd082fbe13e5bfce1599bf2 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sat, 7 Sep 2024 12:12:44 +1000 Subject: [PATCH 2/5] install(PACKAGE_INFO): Remove outdated TODO comment The DESTINATION keyword is supported for this form. --- Source/cmInstallCommand.cxx | 1 - 1 file changed, 1 deletion(-) diff --git a/Source/cmInstallCommand.cxx b/Source/cmInstallCommand.cxx index f4cc4c3b33..183e460bb6 100644 --- a/Source/cmInstallCommand.cxx +++ b/Source/cmInstallCommand.cxx @@ -2189,7 +2189,6 @@ bool HandlePackageInfoMode(std::vector const& args, ArgumentParser::NonEmpty> defaultConfigs; ArgumentParser::NonEmpty cxxModulesDirectory; - // TODO: Support DESTINATION. ica.Bind("PACKAGE_INFO"_s, pkg); ica.Bind("EXPORT"_s, exportName); ica.Bind("APPENDIX"_s, appendix); From 5a8a6dfe81860b4559c4adfac3d4b99503932ae8 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Mon, 16 Sep 2024 11:11:21 +1000 Subject: [PATCH 3/5] cmGeneratorExpression: Change Find() parameter type to cm::string_view --- Source/cmGeneratorExpression.cxx | 11 ++++++----- Source/cmGeneratorExpression.h | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Source/cmGeneratorExpression.cxx b/Source/cmGeneratorExpression.cxx index deb292dc0f..553ea1fb9c 100644 --- a/Source/cmGeneratorExpression.cxx +++ b/Source/cmGeneratorExpression.cxx @@ -375,14 +375,15 @@ std::string cmGeneratorExpression::Preprocess(const std::string& input, return std::string(); } -std::string::size_type cmGeneratorExpression::Find(const std::string& input) +cm::string_view::size_type cmGeneratorExpression::Find( + const cm::string_view& input) { - const std::string::size_type openpos = input.find("$<"); - if (openpos != std::string::npos && - input.find('>', openpos) != std::string::npos) { + const cm::string_view::size_type openpos = input.find("$<"); + if (openpos != cm::string_view::npos && + input.find('>', openpos) != cm::string_view::npos) { return openpos; } - return std::string::npos; + return cm::string_view::npos; } bool cmGeneratorExpression::IsValidTargetName(const std::string& input) diff --git a/Source/cmGeneratorExpression.h b/Source/cmGeneratorExpression.h index 4452f2dd09..134930804f 100644 --- a/Source/cmGeneratorExpression.h +++ b/Source/cmGeneratorExpression.h @@ -66,7 +66,7 @@ public: static void Split(const std::string& input, std::vector& output); - static std::string::size_type Find(const std::string& input); + static cm::string_view::size_type Find(const cm::string_view& input); static bool IsValidTargetName(const std::string& input); From 2184fcfb006691a6ecf79762b4ce3c6b93266ab6 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Tue, 17 Sep 2024 20:29:10 +1000 Subject: [PATCH 4/5] Tests: Configure RunCMake.install cases with correct build type The run_install_test() function would build and install with the configuration hard-coded to Debug, but the configuration step did not specify any configuration. This resulted in a "no config" configuration, and the install step then wouldn't install the Debug export files. This would only be a problem if using a single config CMake generator, and it appears none of the existing tests relied on actually installing the config-specific export file. --- Tests/RunCMake/install/RunCMakeTest.cmake | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tests/RunCMake/install/RunCMakeTest.cmake b/Tests/RunCMake/install/RunCMakeTest.cmake index 295922c0d3..844c4b93ce 100644 --- a/Tests/RunCMake/install/RunCMakeTest.cmake +++ b/Tests/RunCMake/install/RunCMakeTest.cmake @@ -7,7 +7,9 @@ function(run_install_test case) set(RunCMake_TEST_NO_CLEAN 1) file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}") file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}") + set(RunCMake_TEST_RAW_ARGS -DCMAKE_BUILD_TYPE:STRING=Debug) run_cmake(${case}) + unset(RunCMake_TEST_RAW_ARGS) set(RunCMake_TEST_OUTPUT_MERGE 1) run_cmake_command(${case}-build ${CMAKE_COMMAND} --build . --config Debug) unset(RunCMake_TEST_OUTPUT_MERGE) From 6a1fac14505619dfea26ca6a3020eb0b6b409557 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Wed, 18 Sep 2024 08:45:01 +1000 Subject: [PATCH 5/5] install: Normalize DESTINATION paths The file generated by install(EXPORT) computes _IMPORT_PREFIX in a way that assumes a normalized path. If the DESTINATION contains any ../ components, the computed _IMPORT_PREFIX would be wrong. Force the DESTINATION path to be normalized, subject to the new CMP0176 policy. Also normalize all other DESTINATION paths for consistency, except for INCLUDES DESTINATION, which is not strictly a destination but rather a search path to add. Fixes: #26252 --- Help/command/install.rst | 14 ++- Help/manual/cmake-policies.7.rst | 1 + Help/policy/CMP0177.rst | 38 ++++++ .../normalize-install-destination-paths.rst | 6 + Source/cmInstallCommand.cxx | 114 +++++++++++++----- Source/cmInstallCommandArguments.cxx | 59 ++++++++- Source/cmInstallCommandArguments.h | 8 +- Source/cmPolicies.h | 4 +- .../install/CMP0177-NEW-all-check.cmake | 36 ++++++ .../RunCMake/install/CMP0177-NEW-verify.cmake | 2 + Tests/RunCMake/install/CMP0177-NEW.cmake | 2 + .../install/CMP0177-OLD-verify-result.txt | 1 + .../install/CMP0177-OLD-verify-stderr.txt | 13 ++ .../RunCMake/install/CMP0177-OLD-verify.cmake | 2 + Tests/RunCMake/install/CMP0177-OLD.cmake | 2 + .../RunCMake/install/CMP0177-WARN-stderr.txt | 35 ++++++ .../install/CMP0177-WARN-verify-result.txt | 1 + .../install/CMP0177-WARN-verify-stderr.txt | 13 ++ .../install/CMP0177-WARN-verify.cmake | 2 + Tests/RunCMake/install/CMP0177-WARN.cmake | 1 + Tests/RunCMake/install/CMP0177.cmake | 29 +++++ Tests/RunCMake/install/RunCMakeTest.cmake | 6 + 22 files changed, 348 insertions(+), 41 deletions(-) create mode 100644 Help/policy/CMP0177.rst create mode 100644 Help/release/dev/normalize-install-destination-paths.rst create mode 100644 Tests/RunCMake/install/CMP0177-NEW-all-check.cmake create mode 100644 Tests/RunCMake/install/CMP0177-NEW-verify.cmake create mode 100644 Tests/RunCMake/install/CMP0177-NEW.cmake create mode 100644 Tests/RunCMake/install/CMP0177-OLD-verify-result.txt create mode 100644 Tests/RunCMake/install/CMP0177-OLD-verify-stderr.txt create mode 100644 Tests/RunCMake/install/CMP0177-OLD-verify.cmake create mode 100644 Tests/RunCMake/install/CMP0177-OLD.cmake create mode 100644 Tests/RunCMake/install/CMP0177-WARN-stderr.txt create mode 100644 Tests/RunCMake/install/CMP0177-WARN-verify-result.txt create mode 100644 Tests/RunCMake/install/CMP0177-WARN-verify-stderr.txt create mode 100644 Tests/RunCMake/install/CMP0177-WARN-verify.cmake create mode 100644 Tests/RunCMake/install/CMP0177-WARN.cmake create mode 100644 Tests/RunCMake/install/CMP0177.cmake diff --git a/Help/command/install.rst b/Help/command/install.rst index b1991475b7..f7171a4d71 100644 --- a/Help/command/install.rst +++ b/Help/command/install.rst @@ -58,7 +58,7 @@ signatures that specify them. The common options are: ```` should be a relative path. An absolute path is allowed, but not recommended. - When a relative path is given it is interpreted relative to the value + When a relative path is given, it is interpreted relative to the value of the :variable:`CMAKE_INSTALL_PREFIX` variable. The prefix can be relocated at install time using the ``DESTDIR`` mechanism explained in the :variable:`CMAKE_INSTALL_PREFIX` variable @@ -75,6 +75,11 @@ signatures that specify them. The common options are: If an absolute path (with a leading slash or drive letter) is given it is used verbatim. + .. versionchanged:: 3.31 + ```` will be normalized according to the same + :ref:`normalization rules ` as the + :command:`cmake_path` command. + ``PERMISSIONS ...`` Specify permissions for installed files. Valid permissions are ``OWNER_READ``, ``OWNER_WRITE``, ``OWNER_EXECUTE``, ``GROUP_READ``, @@ -396,6 +401,12 @@ Signatures If a relative path is specified, it is treated as relative to the :genex:`$`. + Unlike other ``DESTINATION`` arguments for the various ``install()`` + subcommands, paths given after ``INCLUDES DESTINATION`` are used as + given. They are not normalized, nor assumed to be normalized, although + it is recommended that they are given in normalized form (see + :ref:`Normalization`). + ``RUNTIME_DEPENDENCY_SET `` .. versionadded:: 3.21 @@ -807,6 +818,7 @@ Signatures the generated file will be called ``.cmake`` but the ``FILE`` option may be used to specify a different name. The value given to the ``FILE`` option must be a file name with the ``.cmake`` extension. + If a ``CONFIGURATIONS`` option is given then the file will only be installed when one of the named configurations is installed. Additionally, the generated import file will reference only the matching target diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst index 85bb6ec225..3b97848302 100644 --- a/Help/manual/cmake-policies.7.rst +++ b/Help/manual/cmake-policies.7.rst @@ -57,6 +57,7 @@ Policies Introduced by CMake 3.31 .. toctree:: :maxdepth: 1 + CMP0177: install() DESTINATION paths are normalized. CMP0176: execute_process() ENCODING is UTF-8 by default. CMP0175: add_custom_command() rejects invalid arguments. CMP0174: cmake_parse_arguments(PARSE_ARGV) defines a variable for an empty string after a single-value keyword. diff --git a/Help/policy/CMP0177.rst b/Help/policy/CMP0177.rst new file mode 100644 index 0000000000..3eca9f3b31 --- /dev/null +++ b/Help/policy/CMP0177.rst @@ -0,0 +1,38 @@ +CMP0177 +------- + +.. versionadded:: 3.31 + +:command:`install` ``DESTINATION`` paths are normalized. + +The :command:`install` command has a number of different forms, and most of +them take a ``DESTINATION`` keyword, some in more than one place. +CMake 3.30 and earlier used the value given after the ``DESTINATION`` keyword +as provided with no transformations. The :command:`install(EXPORT)` form +assumes the path contains no ``..`` or ``.`` path components when computing +a path relative to the ``DESTINATION``, and if the project provided a path +that violated that assumption, the computed path would be incorrect. + +CMake 3.31 normalizes all ``DESTINATION`` values given in any form of the +:command:`install` command, except for the ``INCLUDES DESTINATION`` of the +:command:`install(TARGETS)` form. The normalization performed is the same +as for the :command:`cmake_path` command (see :ref:`Normalization`). + +The ``OLD`` behavior of this policy performs no translation on the +``DESTINATION`` values of any :command:`install` command. They are used +exactly as provided. If a destination path contains ``..`` or ``.`` path +components, :command:`install(EXPORT)` will use the same wrong paths as +CMake 3.30 and earlier. + +The ``NEW`` behavior will normalize all ``DESTINATION`` values except for +``INCLUDES DESTINATION``. If a destination path contains a generator +expression, it will be wrapped in a ``$`` +generator expression. + +This policy was introduced in CMake version 3.31. +It may be set by :command:`cmake_policy` or :command:`cmake_minimum_required`. +If it is not set, CMake will warn if it detects a path that would be different +if normalized, and uses ``OLD`` behavior. If a destination path contains a +generator expression, no such warning will be issued regardless of the value. + +.. include:: DEPRECATED.txt diff --git a/Help/release/dev/normalize-install-destination-paths.rst b/Help/release/dev/normalize-install-destination-paths.rst new file mode 100644 index 0000000000..74a76a67ab --- /dev/null +++ b/Help/release/dev/normalize-install-destination-paths.rst @@ -0,0 +1,6 @@ +normalize-install-destination-paths +----------------------------------- + +* All ``DESTINATION`` arguments in :command:`install` commands + are now :ref:`normalized `, with the exception + of ``INCLUDES DESTINATION`` arguments in the ``TARGETS`` form. diff --git a/Source/cmInstallCommand.cxx b/Source/cmInstallCommand.cxx index 183e460bb6..961cb9188c 100644 --- a/Source/cmInstallCommand.cxx +++ b/Source/cmInstallCommand.cxx @@ -20,6 +20,7 @@ #include "cmArgumentParser.h" #include "cmArgumentParserTypes.h" +#include "cmCMakePath.h" #include "cmExecutionStatus.h" #include "cmExperimental.h" #include "cmExportSet.h" @@ -453,7 +454,8 @@ bool HandleTargetsMode(std::vector const& args, runtimeDependenciesArgVector; std::string runtimeDependencySetArg; std::vector unknownArgs; - cmInstallCommandArguments genericArgs(helper.DefaultComponentName); + cmInstallCommandArguments genericArgs(helper.DefaultComponentName, + *helper.Makefile); genericArgs.Bind("TARGETS"_s, targetList); genericArgs.Bind("EXPORT"_s, exports); genericArgs.Bind("RUNTIME_DEPENDENCIES"_s, runtimeDependenciesArgVector); @@ -467,19 +469,31 @@ bool HandleTargetsMode(std::vector const& args, &unknownArgs) : RuntimeDependenciesArgs(); - cmInstallCommandArguments archiveArgs(helper.DefaultComponentName); - cmInstallCommandArguments libraryArgs(helper.DefaultComponentName); - cmInstallCommandArguments runtimeArgs(helper.DefaultComponentName); - cmInstallCommandArguments objectArgs(helper.DefaultComponentName); - cmInstallCommandArguments frameworkArgs(helper.DefaultComponentName); - cmInstallCommandArguments bundleArgs(helper.DefaultComponentName); - cmInstallCommandArguments privateHeaderArgs(helper.DefaultComponentName); - cmInstallCommandArguments publicHeaderArgs(helper.DefaultComponentName); - cmInstallCommandArguments resourceArgs(helper.DefaultComponentName); + cmInstallCommandArguments archiveArgs(helper.DefaultComponentName, + *helper.Makefile); + cmInstallCommandArguments libraryArgs(helper.DefaultComponentName, + *helper.Makefile); + cmInstallCommandArguments runtimeArgs(helper.DefaultComponentName, + *helper.Makefile); + cmInstallCommandArguments objectArgs(helper.DefaultComponentName, + *helper.Makefile); + cmInstallCommandArguments frameworkArgs(helper.DefaultComponentName, + *helper.Makefile); + cmInstallCommandArguments bundleArgs(helper.DefaultComponentName, + *helper.Makefile); + cmInstallCommandArguments privateHeaderArgs(helper.DefaultComponentName, + *helper.Makefile); + cmInstallCommandArguments publicHeaderArgs(helper.DefaultComponentName, + *helper.Makefile); + cmInstallCommandArguments resourceArgs(helper.DefaultComponentName, + *helper.Makefile); cmInstallCommandIncludesArgument includesArgs; std::vector fileSetArgs( - argVectors.FileSets.size(), { helper.DefaultComponentName }); - cmInstallCommandArguments cxxModuleBmiArgs(helper.DefaultComponentName); + argVectors.FileSets.size(), + { cmInstallCommandFileSetArguments(helper.DefaultComponentName, + *helper.Makefile) }); + cmInstallCommandArguments cxxModuleBmiArgs(helper.DefaultComponentName, + *helper.Makefile); // now parse the args for specific parts of the target (e.g. LIBRARY, // RUNTIME, ARCHIVE etc. @@ -499,7 +513,8 @@ bool HandleTargetsMode(std::vector const& args, // cmArgumentParser::Bind() binds to a specific address, but the // objects in the vector can move around. So we parse in an object with a // fixed address and then copy the data into the vector. - cmInstallCommandFileSetArguments fileSetArg(helper.DefaultComponentName); + cmInstallCommandFileSetArguments fileSetArg(helper.DefaultComponentName, + *helper.Makefile); fileSetArg.Parse(argVectors.FileSets[i], &unknownArgs); fileSetArgs[i] = std::move(fileSetArg); } @@ -1310,16 +1325,21 @@ bool HandleImportedRuntimeArtifactsMode(std::vector const& args, ArgumentParser::MaybeEmpty> targetList; std::string runtimeDependencySetArg; std::vector unknownArgs; - cmInstallCommandArguments genericArgs(helper.DefaultComponentName); + cmInstallCommandArguments genericArgs(helper.DefaultComponentName, + *helper.Makefile); genericArgs.Bind("IMPORTED_RUNTIME_ARTIFACTS"_s, targetList) .Bind("RUNTIME_DEPENDENCY_SET"_s, runtimeDependencySetArg); genericArgs.Parse(genericArgVector, &unknownArgs); bool success = genericArgs.Finalize(); - cmInstallCommandArguments libraryArgs(helper.DefaultComponentName); - cmInstallCommandArguments runtimeArgs(helper.DefaultComponentName); - cmInstallCommandArguments frameworkArgs(helper.DefaultComponentName); - cmInstallCommandArguments bundleArgs(helper.DefaultComponentName); + cmInstallCommandArguments libraryArgs(helper.DefaultComponentName, + *helper.Makefile); + cmInstallCommandArguments runtimeArgs(helper.DefaultComponentName, + *helper.Makefile); + cmInstallCommandArguments frameworkArgs(helper.DefaultComponentName, + *helper.Makefile); + cmInstallCommandArguments bundleArgs(helper.DefaultComponentName, + *helper.Makefile); // now parse the args for specific parts of the target (e.g. LIBRARY, // RUNTIME etc. @@ -1547,7 +1567,7 @@ bool HandleFilesMode(std::vector const& args, // This is the FILES mode. bool programs = (args[0] == "PROGRAMS"); - cmInstallCommandArguments ica(helper.DefaultComponentName); + cmInstallCommandArguments ica(helper.DefaultComponentName, *helper.Makefile); ArgumentParser::MaybeEmpty> files; ica.Bind(programs ? "PROGRAMS"_s : "FILES"_s, files); std::vector unknownArgs; @@ -1681,7 +1701,7 @@ bool HandleDirectoryMode(std::vector const& args, bool exclude_from_all = false; bool message_never = false; std::vector dirs; - const std::string* destination = nullptr; + cm::optional destination; std::string permissions_file; std::string permissions_dir; std::vector configurations; @@ -1839,7 +1859,33 @@ bool HandleDirectoryMode(std::vector const& args, } else if (doing == DoingConfigurations) { configurations.push_back(args[i]); } else if (doing == DoingDestination) { - destination = &args[i]; + // A trailing slash is meaningful for this form, but normalization + // preserves it if present + switch (status.GetMakefile().GetPolicyStatus(cmPolicies::CMP0177)) { + case cmPolicies::NEW: + destination = cmCMakePath(args[i]).Normal().String(); + break; + case cmPolicies::WARN: + // We can't be certain if a warning is appropriate if there are any + // generator expressions + if (cmGeneratorExpression::Find(args[i]) == cm::string_view::npos && + args[i] != cmCMakePath(args[i]).Normal().String()) { + status.GetMakefile().IssueMessage( + MessageType::AUTHOR_WARNING, + cmPolicies::GetPolicyWarning(cmPolicies::CMP0177)); + } + CM_FALLTHROUGH; + case cmPolicies::OLD: + destination = args[i]; + break; + case cmPolicies::REQUIRED_ALWAYS: + case cmPolicies::REQUIRED_IF_USED: + // We should never get here, only OLD, WARN, and NEW are used + status.GetMakefile().IssueMessage( + MessageType::FATAL_ERROR, + cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0177)); + return false; + } doing = DoingNone; } else if (doing == DoingType) { if (allowedTypes.count(args[i]) == 0) { @@ -1909,7 +1955,7 @@ bool HandleDirectoryMode(std::vector const& args, } // Support installing an empty directory. - if (dirs.empty() && destination) { + if (dirs.empty() && destination.has_value()) { dirs.emplace_back(); } @@ -1917,15 +1963,13 @@ bool HandleDirectoryMode(std::vector const& args, if (dirs.empty()) { return true; } - std::string destinationStr; - if (!destination) { + if (!destination.has_value()) { if (type.empty()) { // A destination is required. status.SetError(cmStrCat(args[0], " given no DESTINATION!")); return false; } - destinationStr = helper.GetDestinationForType(nullptr, type); - destination = &destinationStr; + destination = helper.GetDestinationForType(nullptr, type); } else if (!type.empty()) { status.SetError(cmStrCat(args[0], " given both TYPE and DESTINATION " @@ -1957,7 +2001,7 @@ bool HandleExportAndroidMKMode(std::vector const& args, Helper helper(status); // This is the EXPORT mode. - cmInstallCommandArguments ica(helper.DefaultComponentName); + cmInstallCommandArguments ica(helper.DefaultComponentName, *helper.Makefile); std::string exp; std::string name_space; @@ -2050,7 +2094,7 @@ bool HandleExportMode(std::vector const& args, Helper helper(status); // This is the EXPORT mode. - cmInstallCommandArguments ica(helper.DefaultComponentName); + cmInstallCommandArguments ica(helper.DefaultComponentName, *helper.Makefile); std::string exp; std::string name_space; @@ -2176,7 +2220,7 @@ bool HandlePackageInfoMode(std::vector const& args, Helper helper(status); // This is the PACKAGE_INFO mode. - cmInstallCommandArguments ica(helper.DefaultComponentName); + cmInstallCommandArguments ica(helper.DefaultComponentName, *helper.Makefile); ArgumentParser::NonEmpty pkg; ArgumentParser::NonEmpty appendix; @@ -2335,14 +2379,18 @@ bool HandleRuntimeDependencySetMode(std::vector const& args, // These generic args also contain the runtime dependency set std::string runtimeDependencySetArg; std::vector runtimeDependencyArgVector; - cmInstallCommandArguments genericArgs(helper.DefaultComponentName); + cmInstallCommandArguments genericArgs(helper.DefaultComponentName, + *helper.Makefile); genericArgs.Bind("RUNTIME_DEPENDENCY_SET"_s, runtimeDependencySetArg); genericArgs.Parse(genericArgVector, &runtimeDependencyArgVector); bool success = genericArgs.Finalize(); - cmInstallCommandArguments libraryArgs(helper.DefaultComponentName); - cmInstallCommandArguments runtimeArgs(helper.DefaultComponentName); - cmInstallCommandArguments frameworkArgs(helper.DefaultComponentName); + cmInstallCommandArguments libraryArgs(helper.DefaultComponentName, + *helper.Makefile); + cmInstallCommandArguments runtimeArgs(helper.DefaultComponentName, + *helper.Makefile); + cmInstallCommandArguments frameworkArgs(helper.DefaultComponentName, + *helper.Makefile); // Now also parse the file(GET_RUNTIME_DEPENDENCY) args std::vector unknownArgs; diff --git a/Source/cmInstallCommandArguments.cxx b/Source/cmInstallCommandArguments.cxx index 25656473a8..f371cff208 100644 --- a/Source/cmInstallCommandArguments.cxx +++ b/Source/cmInstallCommandArguments.cxx @@ -3,11 +3,19 @@ #include "cmInstallCommandArguments.h" #include +#include #include +#include #include +#include "cmCMakePath.h" +#include "cmGeneratorExpression.h" +#include "cmMakefile.h" +#include "cmMessageType.h" +#include "cmPolicies.h" #include "cmRange.h" +#include "cmStringAlgorithms.h" #include "cmSystemTools.h" // Table of valid permissions. @@ -20,10 +28,53 @@ const char* cmInstallCommandArguments::PermissionsTable[] = { const std::string cmInstallCommandArguments::EmptyString; cmInstallCommandArguments::cmInstallCommandArguments( - std::string defaultComponent) + std::string defaultComponent, cmMakefile& makefile) : DefaultComponentName(std::move(defaultComponent)) { - this->Bind("DESTINATION"_s, this->Destination); + std::function normalizeDest; + + switch (makefile.GetPolicyStatus(cmPolicies::CMP0177)) { + case cmPolicies::OLD: + normalizeDest = [this](cm::string_view arg) -> ArgumentParser::Continue { + this->Destination = std::string(arg.begin(), arg.end()); + return ArgumentParser::Continue::Yes; + }; + break; + case cmPolicies::WARN: + normalizeDest = + [this, &makefile](cm::string_view arg) -> ArgumentParser::Continue { + this->Destination = std::string(arg.begin(), arg.end()); + // We can't be certain if a warning is appropriate if there are any + // generator expressions + if (cmGeneratorExpression::Find(arg) == cm::string_view::npos && + arg != cmCMakePath(arg).Normal().String()) { + makefile.IssueMessage( + MessageType::AUTHOR_WARNING, + cmPolicies::GetPolicyWarning(cmPolicies::CMP0177)); + } + return ArgumentParser::Continue::Yes; + }; + break; + case cmPolicies::NEW: + normalizeDest = [this](cm::string_view arg) -> ArgumentParser::Continue { + if (cmGeneratorExpression::Find(arg) == cm::string_view::npos) { + this->Destination = cmCMakePath(arg).Normal().String(); + } else { + this->Destination = + cmStrCat("$'); + } + return ArgumentParser::Continue::Yes; + }; + break; + case cmPolicies::REQUIRED_ALWAYS: + case cmPolicies::REQUIRED_IF_USED: + // We should never get here, only OLD, WARN, and NEW are used + makefile.IssueMessage( + MessageType::FATAL_ERROR, + cmPolicies::GetRequiredPolicyError(cmPolicies::CMP0177)); + } + + this->Bind("DESTINATION"_s, normalizeDest); this->Bind("COMPONENT"_s, this->Component); this->Bind("NAMELINK_COMPONENT"_s, this->NamelinkComponent); this->Bind("EXCLUDE_FROM_ALL"_s, this->ExcludeFromAll); @@ -227,8 +278,8 @@ void cmInstallCommandIncludesArgument::Parse( } cmInstallCommandFileSetArguments::cmInstallCommandFileSetArguments( - std::string defaultComponent) - : cmInstallCommandArguments(std::move(defaultComponent)) + std::string defaultComponent, cmMakefile& makefile) + : cmInstallCommandArguments(std::move(defaultComponent), makefile) { this->Bind("FILE_SET"_s, this->FileSet); } diff --git a/Source/cmInstallCommandArguments.h b/Source/cmInstallCommandArguments.h index 6e46aaca57..cb3917ef88 100644 --- a/Source/cmInstallCommandArguments.h +++ b/Source/cmInstallCommandArguments.h @@ -10,10 +10,13 @@ #include "cmArgumentParser.h" #include "cmArgumentParserTypes.h" +class cmMakefile; + class cmInstallCommandArguments : public cmArgumentParser { public: - cmInstallCommandArguments(std::string defaultComponent); + cmInstallCommandArguments(std::string defaultComponent, + cmMakefile& makefile); void SetGenericArguments(cmInstallCommandArguments* args) { this->GenericArguments = args; @@ -78,7 +81,8 @@ private: class cmInstallCommandFileSetArguments : public cmInstallCommandArguments { public: - cmInstallCommandFileSetArguments(std::string defaultComponent); + cmInstallCommandFileSetArguments(std::string defaultComponent, + cmMakefile& makefile); void Parse(std::vector args, std::vector* unconsumedArgs); diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h index 254c3234ee..88ced3fa72 100644 --- a/Source/cmPolicies.h +++ b/Source/cmPolicies.h @@ -541,7 +541,9 @@ class cmMakefile; SELECT(POLICY, CMP0175, "add_custom_command() rejects invalid arguments.", \ 3, 31, 0, cmPolicies::WARN) \ SELECT(POLICY, CMP0176, "execute_process() ENCODING is UTF-8 by default.", \ - 3, 31, 0, cmPolicies::WARN) + 3, 31, 0, cmPolicies::WARN) \ + SELECT(POLICY, CMP0177, "install() DESTINATION paths are normalized.", 3, \ + 31, 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/Tests/RunCMake/install/CMP0177-NEW-all-check.cmake b/Tests/RunCMake/install/CMP0177-NEW-all-check.cmake new file mode 100644 index 0000000000..3524b6dc6c --- /dev/null +++ b/Tests/RunCMake/install/CMP0177-NEW-all-check.cmake @@ -0,0 +1,36 @@ +set(installBase ${RunCMake_TEST_BINARY_DIR}/root-all) + +foreach(i RANGE 1 5) + set(subdir shouldNotRemain${i}) + if(IS_DIRECTORY ${installBase}/${subdir}) + set(RunCMake_TEST_FAILED "Check failed.") + string(APPEND RunCMake_TEST_FAILURE_MESSAGE + "\nUnexpectedly created install path that should have disappeared with " + "normalization:\n" + " ${installBase}/${subdir}" + ) + endif() +endforeach() + +file(GLOB perConfigFiles ${installBase}/lib/cmake/pkg/pkg-config-*.cmake) +foreach(file IN LISTS perConfigFiles ITEMS ${installBase}/lib/cmake/pkg/pkg-config.cmake) + file(STRINGS ${file} matches REGEX shouldNotRemain) + if(NOT matches STREQUAL "") + set(RunCMake_TEST_FAILED "Check failed.") + string(APPEND RunCMake_TEST_FAILURE_MESSAGE + "\nNon-normalized path found in ${file}:" + ) + foreach(match IN LISTS matches) + string(APPEND RunCMake_TEST_FAILURE_MESSAGE "\n ${match}") + endforeach() + endif() +endforeach() + +if(NOT EXISTS "${installBase}/dirs/dir/empty.txt") + set(RunCMake_TEST_FAILED "Check failed.") + string(APPEND RunCMake_TEST_FAILURE_MESSAGE + "\nNon-normalized DIRECTORY destination not handled correctly. " + "Expected to find the following file, but it was missing:\n" + " ${installBase}/dirs/dir/empty.txt" + ) +endif() diff --git a/Tests/RunCMake/install/CMP0177-NEW-verify.cmake b/Tests/RunCMake/install/CMP0177-NEW-verify.cmake new file mode 100644 index 0000000000..7b8b2dcfa6 --- /dev/null +++ b/Tests/RunCMake/install/CMP0177-NEW-verify.cmake @@ -0,0 +1,2 @@ +enable_language(C) +find_package(pkg REQUIRED) diff --git a/Tests/RunCMake/install/CMP0177-NEW.cmake b/Tests/RunCMake/install/CMP0177-NEW.cmake new file mode 100644 index 0000000000..698d9ceca7 --- /dev/null +++ b/Tests/RunCMake/install/CMP0177-NEW.cmake @@ -0,0 +1,2 @@ +cmake_policy(SET CMP0177 NEW) +include(${CMAKE_CURRENT_LIST_DIR}/CMP0177.cmake) diff --git a/Tests/RunCMake/install/CMP0177-OLD-verify-result.txt b/Tests/RunCMake/install/CMP0177-OLD-verify-result.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/Tests/RunCMake/install/CMP0177-OLD-verify-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/install/CMP0177-OLD-verify-stderr.txt b/Tests/RunCMake/install/CMP0177-OLD-verify-stderr.txt new file mode 100644 index 0000000000..366660bac2 --- /dev/null +++ b/Tests/RunCMake/install/CMP0177-OLD-verify-stderr.txt @@ -0,0 +1,13 @@ +CMake Error at [^ +]*CMP0177-OLD-build/root-all/lib/cmake/pkg/pkg-config\.cmake:[0-9]+ \(message\): + The imported target "foo1" references the file ++ ".*/shouldNotRemain1/\.\./lib/(libfoo1\.a|foo1\.l(ib)?)" ++ but this file does not exist\. Possible reasons include: ++ \* The file was deleted, renamed, or moved to another location\. ++ \* An install or uninstall procedure did not complete successfully\. ++ \* The installation package was faulty and contained ++ ".*/Tests/RunCMake/install/CMP0177-OLD-build/root-all/lib/cmake/pkg/pkg-config\.cmake" ++ but not all the files it references\. ++Call Stack \(most recent call first\): + CMP0177-OLD-verify\.cmake:2 \(find_package\) + CMakeLists\.txt:3 \(include\) diff --git a/Tests/RunCMake/install/CMP0177-OLD-verify.cmake b/Tests/RunCMake/install/CMP0177-OLD-verify.cmake new file mode 100644 index 0000000000..7b8b2dcfa6 --- /dev/null +++ b/Tests/RunCMake/install/CMP0177-OLD-verify.cmake @@ -0,0 +1,2 @@ +enable_language(C) +find_package(pkg REQUIRED) diff --git a/Tests/RunCMake/install/CMP0177-OLD.cmake b/Tests/RunCMake/install/CMP0177-OLD.cmake new file mode 100644 index 0000000000..6389983440 --- /dev/null +++ b/Tests/RunCMake/install/CMP0177-OLD.cmake @@ -0,0 +1,2 @@ +cmake_policy(SET CMP0177 OLD) +include(${CMAKE_CURRENT_LIST_DIR}/CMP0177.cmake) diff --git a/Tests/RunCMake/install/CMP0177-WARN-stderr.txt b/Tests/RunCMake/install/CMP0177-WARN-stderr.txt new file mode 100644 index 0000000000..a7d8b2e549 --- /dev/null +++ b/Tests/RunCMake/install/CMP0177-WARN-stderr.txt @@ -0,0 +1,35 @@ +CMake Warning \(dev\) at CMP0177\.cmake:[0-9]+ \(install\): + Policy CMP0177 is not set: install\(\) DESTINATION paths are normalized\. Run + "cmake --help-policy CMP0177" for policy details\. Use the cmake_policy + command to set the policy and suppress this warning\. +Call Stack \(most recent call first\): + CMP0177-WARN\.cmake:1 \(include\) + CMakeLists\.txt:3 \(include\) +This warning is for project developers\. Use -Wno-dev to suppress it\. + +CMake Warning \(dev\) at CMP0177\.cmake:[0-9]+ \(install\): + Policy CMP0177 is not set: install\(\) DESTINATION paths are normalized\. Run + "cmake --help-policy CMP0177" for policy details\. Use the cmake_policy + command to set the policy and suppress this warning\. +Call Stack \(most recent call first\): + CMP0177-WARN\.cmake:1 \(include\) + CMakeLists\.txt:3 \(include\) +This warning is for project developers\. Use -Wno-dev to suppress it\. + +CMake Warning \(dev\) at CMP0177\.cmake:[0-9]+ \(install\): + Policy CMP0177 is not set: install\(\) DESTINATION paths are normalized\. Run + "cmake --help-policy CMP0177" for policy details\. Use the cmake_policy + command to set the policy and suppress this warning\. +Call Stack \(most recent call first\): + CMP0177-WARN\.cmake:1 \(include\) + CMakeLists\.txt:3 \(include\) +This warning is for project developers\. Use -Wno-dev to suppress it\. + +CMake Warning \(dev\) at CMP0177\.cmake:[0-9]+ \(install\): + Policy CMP0177 is not set: install\(\) DESTINATION paths are normalized\. Run + "cmake --help-policy CMP0177" for policy details\. Use the cmake_policy + command to set the policy and suppress this warning\. +Call Stack \(most recent call first\): + CMP0177-WARN\.cmake:1 \(include\) + CMakeLists\.txt:3 \(include\) +This warning is for project developers\. Use -Wno-dev to suppress it\. diff --git a/Tests/RunCMake/install/CMP0177-WARN-verify-result.txt b/Tests/RunCMake/install/CMP0177-WARN-verify-result.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/Tests/RunCMake/install/CMP0177-WARN-verify-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/install/CMP0177-WARN-verify-stderr.txt b/Tests/RunCMake/install/CMP0177-WARN-verify-stderr.txt new file mode 100644 index 0000000000..0eca815666 --- /dev/null +++ b/Tests/RunCMake/install/CMP0177-WARN-verify-stderr.txt @@ -0,0 +1,13 @@ +CMake Error at [^ +]*CMP0177-WARN-build/root-all/lib/cmake/pkg/pkg-config\.cmake:[0-9]+ \(message\): + The imported target "foo1" references the file ++ ".*/shouldNotRemain1/\.\./lib/(libfoo1\.a|foo1\.l(ib)?)" ++ but this file does not exist\. Possible reasons include: ++ \* The file was deleted, renamed, or moved to another location\. ++ \* An install or uninstall procedure did not complete successfully\. ++ \* The installation package was faulty and contained ++ ".*/Tests/RunCMake/install/CMP0177-WARN-build/root-all/lib/cmake/pkg/pkg-config\.cmake" ++ but not all the files it references\. ++Call Stack \(most recent call first\): + CMP0177-WARN-verify\.cmake:2 \(find_package\) + CMakeLists\.txt:3 \(include\) diff --git a/Tests/RunCMake/install/CMP0177-WARN-verify.cmake b/Tests/RunCMake/install/CMP0177-WARN-verify.cmake new file mode 100644 index 0000000000..7b8b2dcfa6 --- /dev/null +++ b/Tests/RunCMake/install/CMP0177-WARN-verify.cmake @@ -0,0 +1,2 @@ +enable_language(C) +find_package(pkg REQUIRED) diff --git a/Tests/RunCMake/install/CMP0177-WARN.cmake b/Tests/RunCMake/install/CMP0177-WARN.cmake new file mode 100644 index 0000000000..5bad118b93 --- /dev/null +++ b/Tests/RunCMake/install/CMP0177-WARN.cmake @@ -0,0 +1 @@ +include(${CMAKE_CURRENT_LIST_DIR}/CMP0177.cmake) diff --git a/Tests/RunCMake/install/CMP0177.cmake b/Tests/RunCMake/install/CMP0177.cmake new file mode 100644 index 0000000000..176367604d --- /dev/null +++ b/Tests/RunCMake/install/CMP0177.cmake @@ -0,0 +1,29 @@ +enable_language(C) + +add_library(foo1 STATIC obj1.c) +add_library(foo2 STATIC obj2.c) + +set_target_properties(foo2 PROPERTIES HIDDEN_DOUBLE_DOT "..") + +# All the shouldNotRemainX path components below should be normalized out when +# CMP0177 is set to NEW, and retained for OLD and WARN. + +install(TARGETS foo1 + EXPORT pkg + ARCHIVE DESTINATION shouldNotRemain1/../lib +) +install(TARGETS foo2 + EXPORT pkg + ARCHIVE DESTINATION shouldNotRemain2/$/lib +) +install(EXPORT pkg + DESTINATION shouldNotRemain3/deeper/../.././lib/cmake/pkg + FILE pkg-config.cmake +) +install(FILES obj1.c + DESTINATION shouldNotRemain4/anotherSubdir/../../files +) +install(DIRECTORY dir + # Trailing slash here is significant + DESTINATION shouldNotRemain5/../dirs/more/../ +) diff --git a/Tests/RunCMake/install/RunCMakeTest.cmake b/Tests/RunCMake/install/RunCMakeTest.cmake index 844c4b93ce..39c3373dd4 100644 --- a/Tests/RunCMake/install/RunCMakeTest.cmake +++ b/Tests/RunCMake/install/RunCMakeTest.cmake @@ -94,6 +94,12 @@ run_cmake(CMP0062-WARN) run_cmake(CMP0087-OLD) run_cmake(CMP0087-NEW) run_cmake(CMP0087-WARN) +foreach(policy IN ITEMS NEW OLD WARN) + run_install_test(CMP0177-${policy}) + run_cmake_with_options(CMP0177-${policy}-verify + -DCMAKE_PREFIX_PATH=${RunCMake_BINARY_DIR}/CMP0177-${policy}-build/root-all + ) +endforeach() run_cmake(TARGETS-ImportedGlobal) run_cmake(TARGETS-NAMELINK_COMPONENT-bad-all) run_cmake(TARGETS-NAMELINK_COMPONENT-bad-exc)