From f5f80305ef69dd33fbedd31ef1d2cfd3d2bc15b4 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sun, 25 Aug 2024 14:21:16 +1000 Subject: [PATCH 1/4] ExternalProject: Ensure keywords requiring an argument have one Issue: #21089 --- Modules/ExternalProject.cmake | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake index 86d83f53b7..bef918dd98 100644 --- a/Modules/ExternalProject.cmake +++ b/Modules/ExternalProject.cmake @@ -2157,6 +2157,12 @@ function(ExternalProject_Add_Step name step) byproducts ) + if(NOT "x${work_dir}" STREQUAL "x") + set(maybe_WORKING_DIRECTORY "WORKING_DIRECTORY [==[${work_dir}]==]") + else() + set(maybe_WORKING_DIRECTORY "") + endif() + # Custom comment? get_property(comment_set TARGET ${name} @@ -2176,9 +2182,9 @@ function(ExternalProject_Add_Step name step) PROPERTY _EP_${step}_USES_TERMINAL ) if(uses_terminal) - set(uses_terminal USES_TERMINAL) + set(maybe_USES_TERMINAL USES_TERMINAL) else() - set(uses_terminal "") + set(maybe_USES_TERMINAL "") endif() # Run every time? @@ -2245,14 +2251,14 @@ function(ExternalProject_Add_Step name step) add_custom_command( OUTPUT \${stamp_file} BYPRODUCTS \${byproducts} - COMMENT \${comment} + COMMENT [===[${comment}]===] COMMAND ${__cmdQuoted} + DEPENDS \${depends} + VERBATIM ${maybe_COMMAND_touch} ${maybe_JOB_SERVER_AWARE} - DEPENDS \${depends} - WORKING_DIRECTORY \${work_dir} - VERBATIM - ${uses_terminal} + ${maybe_WORKING_DIRECTORY} + ${maybe_USES_TERMINAL} )" ) set_property(TARGET ${name} APPEND PROPERTY _EP_STEPS ${step}) From 316840b430b2f1fdc8dce67ec3c7b66e66de4c38 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sun, 25 Aug 2024 14:21:58 +1000 Subject: [PATCH 2/4] Tests: Add missing POST_BUILD to custom commands Issue: #21089 --- .../RunCMake/NinjaMultiConfig/CustomCommandsAndTargets.cmake | 4 ++-- Tests/RunCMake/add_custom_command/RemoveEmptyCommands.cmake | 3 +++ Tests/RunCMake/add_custom_command/TargetImported.cmake | 2 +- Tests/RunCMake/add_custom_command/TargetNotInDir.cmake | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Tests/RunCMake/NinjaMultiConfig/CustomCommandsAndTargets.cmake b/Tests/RunCMake/NinjaMultiConfig/CustomCommandsAndTargets.cmake index 7bed090905..b7e4aeab20 100644 --- a/Tests/RunCMake/NinjaMultiConfig/CustomCommandsAndTargets.cmake +++ b/Tests/RunCMake/NinjaMultiConfig/CustomCommandsAndTargets.cmake @@ -16,11 +16,11 @@ function(create_targets prefix) get_write_file_command(cmd ${prefix}PostBuild.txt) add_executable(${prefix}PostBuild ${CMAKE_SOURCE_DIR}/main.c) - add_custom_command(TARGET ${prefix}PostBuild COMMAND ${cmd} BYPRODUCTS ${prefix}PostBuild.txt) + add_custom_command(TARGET ${prefix}PostBuild POST_BUILD COMMAND ${cmd} BYPRODUCTS ${prefix}PostBuild.txt) get_write_file_command(cmd ${prefix}TargetPostBuild.txt) add_custom_target(${prefix}TargetPostBuild) - add_custom_command(TARGET ${prefix}TargetPostBuild COMMAND ${cmd} BYPRODUCTS ${prefix}TargetPostBuild.txt) + add_custom_command(TARGET ${prefix}TargetPostBuild POST_BUILD COMMAND ${cmd} BYPRODUCTS ${prefix}TargetPostBuild.txt) file(APPEND "${CMAKE_BINARY_DIR}/target_files_custom.cmake" "set(TARGET_DEPENDS_${prefix}Command [==[${CMAKE_CURRENT_BINARY_DIR}/${prefix}Command.txt]==]) diff --git a/Tests/RunCMake/add_custom_command/RemoveEmptyCommands.cmake b/Tests/RunCMake/add_custom_command/RemoveEmptyCommands.cmake index eb190cc99b..5354d0ac14 100644 --- a/Tests/RunCMake/add_custom_command/RemoveEmptyCommands.cmake +++ b/Tests/RunCMake/add_custom_command/RemoveEmptyCommands.cmake @@ -9,14 +9,17 @@ add_executable(exe "${main_file}") # add one command for all and one for debug only add_custom_command(TARGET exe + POST_BUILD COMMAND "cmd_1" "cmd_1_arg" COMMAND $<$:cmd_1_dbg> $<$:cmd_1_dbg_arg>) # add command for debug only add_custom_command(TARGET exe + POST_BUILD COMMAND $<$:cmd_2_dbg> $<$:cmd_2_dbg_arg>) # add separate commands for configurations add_custom_command(TARGET exe + POST_BUILD COMMAND $<$:cmd_3_dbg> $<$:cmd_3_dbg_arg> COMMAND $<$:cmd_3_rel> $<$:cmd_3_rel_arg>) diff --git a/Tests/RunCMake/add_custom_command/TargetImported.cmake b/Tests/RunCMake/add_custom_command/TargetImported.cmake index c0f2d66a07..1bfd3f929a 100644 --- a/Tests/RunCMake/add_custom_command/TargetImported.cmake +++ b/Tests/RunCMake/add_custom_command/TargetImported.cmake @@ -1,2 +1,2 @@ add_library(TargetImported UNKNOWN IMPORTED) -add_custom_command(TARGET TargetImported COMMAND ${CMAKE_COMMAND} -E echo tada) +add_custom_command(TARGET TargetImported POST_BUILD COMMAND ${CMAKE_COMMAND} -E echo tada) diff --git a/Tests/RunCMake/add_custom_command/TargetNotInDir.cmake b/Tests/RunCMake/add_custom_command/TargetNotInDir.cmake index a5510262aa..0bd4fbd4dc 100644 --- a/Tests/RunCMake/add_custom_command/TargetNotInDir.cmake +++ b/Tests/RunCMake/add_custom_command/TargetNotInDir.cmake @@ -1,2 +1,2 @@ add_subdirectory(TargetNotInDir) -add_custom_command(TARGET TargetNotInDir COMMAND ${CMAKE_COMMAND} -E echo tada) +add_custom_command(TARGET TargetNotInDir POST_BUILD COMMAND ${CMAKE_COMMAND} -E echo tada) From 8dc8be08849acb75446a53fe763eee5b3296d739 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sun, 25 Aug 2024 14:25:29 +1000 Subject: [PATCH 3/4] AndroidTestUtilities: Remove DEPENDS that was being silently ignored The add_custom_command(TARGET) form does not support a DEPENDS keyword, but it was silently ignored up to now. It will soon be reported as an error, so remove the DEPENDS. The behavior will be the same as before. Issue: #26096 --- Modules/AndroidTestUtilities.cmake | 1 - 1 file changed, 1 deletion(-) diff --git a/Modules/AndroidTestUtilities.cmake b/Modules/AndroidTestUtilities.cmake index ddccf586bb..55efb36517 100644 --- a/Modules/AndroidTestUtilities.cmake +++ b/Modules/AndroidTestUtilities.cmake @@ -139,7 +139,6 @@ function(android_add_test_data test_name) get_filename_component(extern_data_basename ${output} NAME) add_custom_command( TARGET ${DATA_TARGET_NAME} POST_BUILD - DEPENDS ${extern_data_source} COMMAND ${CMAKE_COMMAND} -E copy_if_different ${extern_data_source} ${DEST}/${extern_data_basename} ) endif() From ec519f3e975467443a8286b4ddc1904d964f851f Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sun, 25 Aug 2024 14:41:41 +1000 Subject: [PATCH 4/4] add_custom_command: Validate arguments more rigorously Add a new CMP0175 policy to preserve backward compatibility for projects that were using unsupported keywords or arguments. Fixes: #26096, #21089, #18976 --- Help/command/add_custom_command.rst | 69 +++++- Help/manual/cmake-policies.7.rst | 1 + Help/policy/CMP0175.rst | 40 ++++ Source/cmAddCustomCommandCommand.cxx | 203 ++++++++++++++++++ Source/cmPolicies.h | 2 + .../add_custom_command/CMP0175-NEW-result.txt | 1 + .../add_custom_command/CMP0175-NEW-stderr.txt | 29 +++ .../add_custom_command/CMP0175-NEW.cmake | 2 + .../add_custom_command/CMP0175-OLD.cmake | 2 + .../CMP0175-WARN-stderr.txt | 72 +++++++ .../add_custom_command/CMP0175-WARN.cmake | 1 + .../RunCMake/add_custom_command/CMP0175.cmake | 65 ++++++ .../add_custom_command/RunCMakeTest.cmake | 3 + 13 files changed, 481 insertions(+), 9 deletions(-) create mode 100644 Help/policy/CMP0175.rst create mode 100644 Tests/RunCMake/add_custom_command/CMP0175-NEW-result.txt create mode 100644 Tests/RunCMake/add_custom_command/CMP0175-NEW-stderr.txt create mode 100644 Tests/RunCMake/add_custom_command/CMP0175-NEW.cmake create mode 100644 Tests/RunCMake/add_custom_command/CMP0175-OLD.cmake create mode 100644 Tests/RunCMake/add_custom_command/CMP0175-WARN-stderr.txt create mode 100644 Tests/RunCMake/add_custom_command/CMP0175-WARN.cmake create mode 100644 Tests/RunCMake/add_custom_command/CMP0175.cmake diff --git a/Help/command/add_custom_command.rst b/Help/command/add_custom_command.rst index a69c8dcefe..122bb4eda6 100644 --- a/Help/command/add_custom_command.rst +++ b/Help/command/add_custom_command.rst @@ -5,6 +5,8 @@ Add a custom build rule to the generated build system. There are two main signatures for ``add_custom_command``. +.. _`add_custom_command(OUTPUT)`: + Generating Files ^^^^^^^^^^^^^^^^ @@ -54,7 +56,7 @@ The options are: the appended commands and dependencies apply to all configurations. The ``COMMENT``, ``MAIN_DEPENDENCY``, and ``WORKING_DIRECTORY`` - options are currently ignored when APPEND is given, but may be + options are currently ignored when ``APPEND`` is given, but may be used in the future. ``BYPRODUCTS`` @@ -82,6 +84,10 @@ The options are: The :ref:`Makefile Generators` will remove ``BYPRODUCTS`` and other :prop_sf:`GENERATED` files during ``make clean``. + This keyword cannot be used with ``APPEND`` (see policy :policy:`CMP0175`). + All byproducts must be set in the first call to + ``add_custom_command(OUTPUT...)`` for the output files. + .. versionadded:: 3.20 Arguments to ``BYPRODUCTS`` may use a restricted set of :manual:`generator expressions `. @@ -95,11 +101,15 @@ The options are: ``COMMAND`` Specify the command-line(s) to execute at build time. - If more than one ``COMMAND`` is specified they will be executed in order, + At least one ``COMMAND`` would normally be given, but certain patterns + may omit it, such as adding commands in separate calls using `APPEND`. + + If more than one ``COMMAND`` is specified, they will be executed in order, but *not* necessarily composed into a stateful shell or batch script. - (To run a full script, use the :command:`configure_file` command or the + To run a full script, use the :command:`configure_file` command or the :command:`file(GENERATE)` command to create it, and then specify - a ``COMMAND`` to launch it.) + a ``COMMAND`` to launch it. + The optional ``ARGS`` argument is for backward compatibility and will be ignored. @@ -144,7 +154,8 @@ The options are: ``COMMENT`` Display the given message before the commands are executed at - build time. + build time. This will be ignored if ``APPEND`` is given, although a future + version may use it. .. versionadded:: 3.26 Arguments to ``COMMENT`` may use @@ -204,6 +215,10 @@ The options are: ``${CC} "-I$,;-I>" foo.cc`` to be properly expanded. + This keyword cannot be used with ``APPEND`` (see policy :policy:`CMP0175`). + If the appended commands need this option to be set, it must be set on the + first call to ``add_custom_command(OUTPUT...)`` for the output files. + ``CODEGEN`` .. versionadded:: 3.31 @@ -216,6 +231,10 @@ The options are: Furthermore, this option is allowed only if policy :policy:`CMP0171` is set to ``NEW``. + This keyword cannot be used with ``APPEND`` (see policy :policy:`CMP0175`). + It can only be set on the first call to ``add_custom_command(OUTPUT...)`` + for the output files. + ``IMPLICIT_DEPENDS`` Request scanning of implicit dependencies of an input file. The language given specifies the programming language whose @@ -240,6 +259,10 @@ The options are: Using a pool that is not defined by :prop_gbl:`JOB_POOLS` causes an error by ninja at build time. + This keyword cannot be used with ``APPEND`` (see policy :policy:`CMP0175`). + Job pools can only be specified in the first call to + ``add_custom_command(OUTPUT...)`` for the output files. + ``JOB_SERVER_AWARE`` .. versionadded:: 3.28 @@ -251,6 +274,10 @@ The options are: This option is silently ignored by other generators. + This keyword cannot be used with ``APPEND`` (see policy :policy:`CMP0175`). + Job server awareness can only be specified in the first call to + ``add_custom_command(OUTPUT...)`` for the output files. + .. _`GNU Make Documentation`: https://www.gnu.org/software/make/manual/html_node/MAKE-Variable.html ``MAIN_DEPENDENCY`` @@ -262,6 +289,9 @@ The options are: library or an executable) counts as an implicit main dependency which gets silently overwritten by a custom command specification. + This option is currently ignored if ``APPEND`` is given, but a future + version may use it. + ``OUTPUT`` Specify the output files the command is expected to produce. Each output file will be marked with the :prop_sf:`GENERATED` @@ -306,6 +336,10 @@ The options are: With the :generator:`Ninja` generator, this places the command in the ``console`` :prop_gbl:`pool `. + This keyword cannot be used with ``APPEND`` (see policy :policy:`CMP0175`). + If the appended commands need access to the terminal, it must be set on + the first call to ``add_custom_command(OUTPUT...)`` for the output files. + ``VERBATIM`` All arguments to the commands will be escaped properly for the build tool so that the invoked command receives each argument @@ -316,11 +350,18 @@ The options are: is platform specific because there is no protection of tool-specific special characters. + This keyword cannot be used with ``APPEND`` (see policy :policy:`CMP0175`). + If the appended commands need to be treated as ``VERBATIM``, it must be set + on the first call to ``add_custom_command(OUTPUT...)`` for the output files. + ``WORKING_DIRECTORY`` Execute the command with the given current working directory. - If it is a relative path it will be interpreted relative to the + If it is a relative path, it will be interpreted relative to the build tree directory corresponding to the current source directory. + This option is currently ignored if ``APPEND`` is given, but a future + version may use it. + .. versionadded:: 3.13 Arguments to ``WORKING_DIRECTORY`` may use :manual:`generator expressions `. @@ -406,6 +447,10 @@ The options are: :ref:`Makefile Generators`, :ref:`Visual Studio Generators`, and the :generator:`Xcode` generator. + This keyword cannot be used with ``APPEND`` (see policy :policy:`CMP0175`). + Depfiles can only be set on the first call to + ``add_custom_command(OUTPUT...)`` for the output files. + ``DEPENDS_EXPLICIT_ONLY`` .. versionadded:: 3.27 @@ -421,6 +466,10 @@ The options are: This option can be enabled on all custom commands by setting :variable:`CMAKE_ADD_CUSTOM_COMMAND_DEPENDS_EXPLICIT_ONLY` to ``ON``. + This keyword cannot be used with ``APPEND`` (see policy :policy:`CMP0175`). + It can only be set on the first call to ``add_custom_command(OUTPUT...)`` + for the output files. + Only the :ref:`Ninja Generators` actually use this information to remove unnecessary implicit dependencies. @@ -575,9 +624,11 @@ of the following is specified: Run after all other rules within the target have been executed. Projects should always specify one of the above three keywords when using -the ``TARGET`` form. For backward compatibility reasons, ``POST_BUILD`` is -assumed if no such keyword is given, but projects should explicitly provide -one of the keywords to make clear the behavior they expect. +the ``TARGET`` form. See policy :policy:`CMP0175`. + +All other keywords shown in the signature above have the same meaning as they +do for the :command:`add_custom_command(OUTPUT)` form of the command. +At least one ``COMMAND`` must be given, see policy :policy:`CMP0175`. .. note:: Because generator expressions can be used in custom commands, diff --git a/Help/manual/cmake-policies.7.rst b/Help/manual/cmake-policies.7.rst index e257c36160..e5284e9fb3 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 + 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. CMP0173: The CMakeFindFrameworks module is removed. CMP0172: The CPack module enables per-machine installation by default in the CPack WIX Generator. diff --git a/Help/policy/CMP0175.rst b/Help/policy/CMP0175.rst new file mode 100644 index 0000000000..f2c372d6e9 --- /dev/null +++ b/Help/policy/CMP0175.rst @@ -0,0 +1,40 @@ +CMP0175 +------- + +.. versionadded:: 3.31 + +:command:`add_custom_command` rejects invalid arguments. + +CMake 3.30 and earlier silently ignored unsupported keywords and missing or +invalid arguments for the different forms of the :command:`add_custom_command` +command. CMake 3.31 implements more rigorous argument checking and will flag +invalid or missing arguments as errors. + +The ``OLD`` behavior of this policy will accept the same invalid keywords or +arguments as CMake 3.30 and earlier. The ``NEW`` behavior will flag the +following as errors that previously went unreported: + +* The ``OUTPUT`` form does not accept ``PRE_BUILD``, ``PRE_LINK``, or + ``POST_BUILD`` keywords. +* When the ``APPEND`` keyword is given, the ``OUTPUT`` form also does not + accept ``BYPRODUCTS``, ``COMMAND_EXPAND_LISTS``, ``DEPENDS_EXPLICIT_ONLY``, + ``DEPFILE``, ``JOB_POOL``, ``JOB_SERVER_AWARE``, ``USES_TERMINAL``, or + ``VERBATIM`` keywords. +* The ``TARGET`` form requires exactly one of ``PRE_BUILD``, ``PRE_LINK``, or + ``POST_BUILD`` to be given. Previously, if none were given, ``POST_BUILD`` + was assumed, or if multiple keywords were given, the last one was used. +* The ``TARGET`` form does not accept ``DEPENDS``, ``DEPENDS_EXPLICIT_ONLY``, + ``DEPFILE``, ``IMPLICIT_DEPENDS``, ``MAIN_DEPENDENCY``, ``JOB_POOL``, + ``JOB_SERVER_AWARE``, or ``USES_TERMINAL`` keywords. +* The ``TARGET`` form now requires at least one ``COMMAND`` to be given. +* If a keyword expects a value to be given after it, but no value is provided, + that was previously treated as though the keyword was not given at all. +* The ``COMMENT`` keyword expects exactly one value after it. If multiple + values are given, or if the ``COMMENT`` keyword is given more than once, + this is an error. + +.. |INTRODUCED_IN_CMAKE_VERSION| replace:: 3.31 +.. |WARNS_OR_DOES_NOT_WARN| replace:: warns +.. include:: STANDARD_ADVICE.txt + +.. include:: DEPRECATED.txt diff --git a/Source/cmAddCustomCommandCommand.cxx b/Source/cmAddCustomCommandCommand.cxx index d5adba7642..885094d3e3 100644 --- a/Source/cmAddCustomCommandCommand.cxx +++ b/Source/cmAddCustomCommandCommand.cxx @@ -2,11 +2,15 @@ file Copyright.txt or https://cmake.org/licensing for details. */ #include "cmAddCustomCommandCommand.h" +#include +#include +#include #include #include #include #include +#include #include "cmCustomCommand.h" #include "cmCustomCommandLines.h" @@ -140,11 +144,82 @@ bool cmAddCustomCommandCommand(std::vector const& args, keyDEPENDS_EXPLICIT_ONLY, keyCODEGEN }; + /* clang-format off */ + static std::set const supportedTargetKeywords{ + keyARGS, + keyBYPRODUCTS, + keyCOMMAND, + keyCOMMAND_EXPAND_LISTS, + keyCOMMENT, + keyPOST_BUILD, + keyPRE_BUILD, + keyPRE_LINK, + keyTARGET, + keyVERBATIM, + keyWORKING_DIRECTORY + }; + /* clang-format on */ + static std::set const supportedOutputKeywords{ + keyAPPEND, + keyARGS, + keyBYPRODUCTS, + keyCODEGEN, + keyCOMMAND, + keyCOMMAND_EXPAND_LISTS, + keyCOMMENT, + keyDEPENDS, + keyDEPENDS_EXPLICIT_ONLY, + keyDEPFILE, + keyIMPLICIT_DEPENDS, + keyJOB_POOL, + keyJOB_SERVER_AWARE, + keyMAIN_DEPENDENCY, + keyOUTPUT, + keyUSES_TERMINAL, + keyVERBATIM, + keyWORKING_DIRECTORY + }; + /* clang-format off */ + static std::set const supportedAppendKeywords{ + keyAPPEND, + keyARGS, + keyCOMMAND, + keyCOMMENT, // Allowed but ignored + keyDEPENDS, + keyIMPLICIT_DEPENDS, + keyMAIN_DEPENDENCY, // Allowed but ignored + keyOUTPUT, + keyWORKING_DIRECTORY // Allowed but ignored + }; + /* clang-format on */ + std::set keywordsSeen; + std::string const* keywordExpectingValue = nullptr; + auto const cmp0175 = mf.GetPolicyStatus(cmPolicies::CMP0175); for (std::string const& copy : args) { if (keywords.count(copy)) { + // Check if a preceding keyword expected a value but there wasn't one + if (keywordExpectingValue) { + std::string const msg = + cmStrCat("Keyword ", *keywordExpectingValue, + " requires a value, but none was given."); + if (cmp0175 == cmPolicies::NEW) { + mf.IssueMessage(MessageType::FATAL_ERROR, msg); + return false; + } + if (cmp0175 == cmPolicies::WARN) { + mf.IssueMessage( + MessageType::AUTHOR_WARNING, + cmStrCat(msg, '\n', + cmPolicies::GetPolicyWarning(cmPolicies::CMP0175))); + } + } + keywordExpectingValue = nullptr; + keywordsSeen.insert(copy); + if (copy == keySOURCE) { doing = doing_source; + keywordExpectingValue = &keySOURCE; } else if (copy == keyCOMMAND) { doing = doing_command; @@ -173,6 +248,7 @@ bool cmAddCustomCommandCommand(std::vector const& args, codegen = true; } else if (copy == keyTARGET) { doing = doing_target; + keywordExpectingValue = &keyTARGET; } else if (copy == keyARGS) { // Ignore this old keyword. } else if (copy == keyDEPENDS) { @@ -181,16 +257,20 @@ bool cmAddCustomCommandCommand(std::vector const& args, doing = doing_outputs; } else if (copy == keyOUTPUT) { doing = doing_output; + keywordExpectingValue = &keyOUTPUT; } else if (copy == keyBYPRODUCTS) { doing = doing_byproducts; } else if (copy == keyWORKING_DIRECTORY) { doing = doing_working_directory; + keywordExpectingValue = &keyWORKING_DIRECTORY; } else if (copy == keyMAIN_DEPENDENCY) { doing = doing_main_dependency; + keywordExpectingValue = &keyMAIN_DEPENDENCY; } else if (copy == keyIMPLICIT_DEPENDS) { doing = doing_implicit_depends_lang; } else if (copy == keyCOMMENT) { doing = doing_comment; + keywordExpectingValue = &keyCOMMENT; } else if (copy == keyDEPFILE) { doing = doing_depfile; if (!mf.GetGlobalGenerator()->SupportsCustomCommandDepfile()) { @@ -198,12 +278,16 @@ bool cmAddCustomCommandCommand(std::vector const& args, mf.GetGlobalGenerator()->GetName())); return false; } + keywordExpectingValue = &keyDEPFILE; } else if (copy == keyJOB_POOL) { doing = doing_job_pool; + keywordExpectingValue = &keyJOB_POOL; } else if (copy == keyJOB_SERVER_AWARE) { doing = doing_job_server_aware; + keywordExpectingValue = &keyJOB_SERVER_AWARE; } } else { + keywordExpectingValue = nullptr; // Value is being processed now std::string filename; switch (doing) { case doing_output: @@ -288,6 +372,21 @@ bool cmAddCustomCommandCommand(std::vector const& args, byproducts.push_back(filename); break; case doing_comment: + if (!comment_buffer.empty()) { + std::string const msg = + "COMMENT requires exactly one argument, but multiple values " + "or COMMENT keywords have been given."; + if (cmp0175 == cmPolicies::NEW) { + mf.IssueMessage(MessageType::FATAL_ERROR, msg); + return false; + } + if (cmp0175 == cmPolicies::WARN) { + mf.IssueMessage( + MessageType::AUTHOR_WARNING, + cmStrCat(msg, '\n', + cmPolicies::GetPolicyWarning(cmPolicies::CMP0175))); + } + } comment_buffer = copy; comment = comment_buffer.c_str(); break; @@ -351,6 +450,27 @@ bool cmAddCustomCommandCommand(std::vector const& args, // Check for an append request. if (append) { + std::vector unsupportedKeywordsUsed; + std::set_difference(keywordsSeen.begin(), keywordsSeen.end(), + supportedAppendKeywords.begin(), + supportedAppendKeywords.end(), + std::back_inserter(unsupportedKeywordsUsed)); + if (!unsupportedKeywordsUsed.empty()) { + std::string const msg = + cmJoin(unsupportedKeywordsUsed, ", "_s, + "The following keywords are not supported when using " + "APPEND with add_custom_command(OUTPUT): "_s); + if (cmp0175 == cmPolicies::NEW) { + mf.IssueMessage(MessageType::FATAL_ERROR, msg); + return false; + } + if (cmp0175 == cmPolicies::WARN) { + mf.IssueMessage( + MessageType::AUTHOR_WARNING, + cmStrCat(msg, ".\n", + cmPolicies::GetPolicyWarning(cmPolicies::CMP0175))); + } + } mf.AppendCustomCommandToOutput(output[0], depends, implicit_depends, commandLines); return true; @@ -376,9 +496,92 @@ bool cmAddCustomCommandCommand(std::vector const& args, cc->SetDependsExplicitOnly(depends_explicit_only); if (source.empty() && output.empty()) { // Source is empty, use the target. + if (commandLines.empty()) { + std::string const msg = "At least one COMMAND must be given."; + if (cmp0175 == cmPolicies::NEW) { + mf.IssueMessage(MessageType::FATAL_ERROR, msg); + return false; + } + if (cmp0175 == cmPolicies::WARN) { + mf.IssueMessage( + MessageType::AUTHOR_WARNING, + cmStrCat(msg, '\n', + cmPolicies::GetPolicyWarning(cmPolicies::CMP0175))); + } + } + + std::vector unsupportedKeywordsUsed; + std::set_difference(keywordsSeen.begin(), keywordsSeen.end(), + supportedTargetKeywords.begin(), + supportedTargetKeywords.end(), + std::back_inserter(unsupportedKeywordsUsed)); + if (!unsupportedKeywordsUsed.empty()) { + std::string const msg = + cmJoin(unsupportedKeywordsUsed, ", "_s, + "The following keywords are not supported when using " + "add_custom_command(TARGET): "_s); + if (cmp0175 == cmPolicies::NEW) { + mf.IssueMessage(MessageType::FATAL_ERROR, msg); + return false; + } + if (cmp0175 == cmPolicies::WARN) { + mf.IssueMessage( + MessageType::AUTHOR_WARNING, + cmStrCat(msg, ".\n", + cmPolicies::GetPolicyWarning(cmPolicies::CMP0175))); + } + } + auto const prePostCount = keywordsSeen.count(keyPRE_BUILD) + + keywordsSeen.count(keyPRE_LINK) + keywordsSeen.count(keyPOST_BUILD); + if (prePostCount != 1) { + std::string msg = + "Exactly one of PRE_BUILD, PRE_LINK, or POST_BUILD must be given."; + if (cmp0175 == cmPolicies::NEW) { + mf.IssueMessage(MessageType::FATAL_ERROR, msg); + return false; + } + if (cmp0175 == cmPolicies::WARN) { + msg += " Assuming "; + switch (cctype) { + case cmCustomCommandType::PRE_BUILD: + msg += "PRE_BUILD"; + break; + case cmCustomCommandType::PRE_LINK: + msg += "PRE_LINK"; + break; + case cmCustomCommandType::POST_BUILD: + msg += "POST_BUILD"; + } + mf.IssueMessage( + MessageType::AUTHOR_WARNING, + cmStrCat(msg, " to preserve backward compatibility.\n", + cmPolicies::GetPolicyWarning(cmPolicies::CMP0175))); + } + } mf.AddCustomCommandToTarget(target, cctype, std::move(cc)); } else if (target.empty()) { // Target is empty, use the output. + std::vector unsupportedKeywordsUsed; + std::set_difference(keywordsSeen.begin(), keywordsSeen.end(), + supportedOutputKeywords.begin(), + supportedOutputKeywords.end(), + std::back_inserter(unsupportedKeywordsUsed)); + if (!unsupportedKeywordsUsed.empty()) { + std::string const msg = + cmJoin(unsupportedKeywordsUsed, ", "_s, + "The following keywords are not supported when using " + "add_custom_command(OUTPUT): "_s); + if (cmp0175 == cmPolicies::NEW) { + mf.IssueMessage(MessageType::FATAL_ERROR, msg); + return false; + } + if (cmp0175 == cmPolicies::WARN) { + mf.IssueMessage( + MessageType::AUTHOR_WARNING, + cmStrCat(msg, ".\n", + cmPolicies::GetPolicyWarning(cmPolicies::CMP0175))); + } + } cc->SetOutputs(output); cc->SetMainDependency(main_dependency); cc->SetDepends(depends); diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h index 7b056aeba3..644cb9ec59 100644 --- a/Source/cmPolicies.h +++ b/Source/cmPolicies.h @@ -537,6 +537,8 @@ class cmMakefile; SELECT(POLICY, CMP0174, \ "cmake_parse_arguments(PARSE_ARGV) defines a variable for an empty " \ "string after a single-value keyword.", \ + 3, 31, 0, cmPolicies::WARN) \ + SELECT(POLICY, CMP0175, "add_custom_command() rejects invalid arguments.", \ 3, 31, 0, cmPolicies::WARN) #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1) diff --git a/Tests/RunCMake/add_custom_command/CMP0175-NEW-result.txt b/Tests/RunCMake/add_custom_command/CMP0175-NEW-result.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/Tests/RunCMake/add_custom_command/CMP0175-NEW-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/add_custom_command/CMP0175-NEW-stderr.txt b/Tests/RunCMake/add_custom_command/CMP0175-NEW-stderr.txt new file mode 100644 index 0000000000..c3569713ee --- /dev/null +++ b/Tests/RunCMake/add_custom_command/CMP0175-NEW-stderr.txt @@ -0,0 +1,29 @@ +CMake Error at CMP0175\.cmake:[0-9]+ \(add_custom_command\): + The following keywords are not supported when using + add_custom_command\(TARGET\): DEPENDS, DEPENDS_EXPLICIT_ONLY, DEPFILE, + JOB_POOL, MAIN_DEPENDENCY +Call Stack \(most recent call first\): + CMP0175-NEW\.cmake:2 \(include\) + CMakeLists\.txt:3 \(include\) + + +CMake Error at CMP0175\.cmake:[0-9]+ \(add_custom_command\): + The following keywords are not supported when using + add_custom_command\(TARGET\): IMPLICIT_DEPENDS, USES_TERMINAL +Call Stack \(most recent call first\): + CMP0175-NEW\.cmake:2 \(include\) + CMakeLists\.txt:3 \(include\) + + +CMake Error at CMP0175\.cmake:[0-9]+ \(add_custom_command\): + Exactly one of PRE_BUILD, PRE_LINK, or POST_BUILD must be given\. +Call Stack \(most recent call first\): + CMP0175-NEW\.cmake:2 \(include\) + CMakeLists\.txt:3 \(include\) + + +CMake Error at CMP0175\.cmake:[0-9]+ \(add_custom_command\): + Exactly one of PRE_BUILD, PRE_LINK, or POST_BUILD must be given\. +Call Stack \(most recent call first\): + CMP0175-NEW\.cmake:2 \(include\) + CMakeLists\.txt:3 \(include\) diff --git a/Tests/RunCMake/add_custom_command/CMP0175-NEW.cmake b/Tests/RunCMake/add_custom_command/CMP0175-NEW.cmake new file mode 100644 index 0000000000..d8cb4fb905 --- /dev/null +++ b/Tests/RunCMake/add_custom_command/CMP0175-NEW.cmake @@ -0,0 +1,2 @@ +cmake_policy(SET CMP0175 NEW) +include(CMP0175.cmake) diff --git a/Tests/RunCMake/add_custom_command/CMP0175-OLD.cmake b/Tests/RunCMake/add_custom_command/CMP0175-OLD.cmake new file mode 100644 index 0000000000..a66c02d39a --- /dev/null +++ b/Tests/RunCMake/add_custom_command/CMP0175-OLD.cmake @@ -0,0 +1,2 @@ +cmake_policy(SET CMP0175 OLD) +include(CMP0175.cmake) diff --git a/Tests/RunCMake/add_custom_command/CMP0175-WARN-stderr.txt b/Tests/RunCMake/add_custom_command/CMP0175-WARN-stderr.txt new file mode 100644 index 0000000000..f49dd73444 --- /dev/null +++ b/Tests/RunCMake/add_custom_command/CMP0175-WARN-stderr.txt @@ -0,0 +1,72 @@ +CMake Warning \(dev\) at CMP0175\.cmake:[0-9]+ \(add_custom_command\): + The following keywords are not supported when using + add_custom_command\(TARGET\): DEPENDS, DEPENDS_EXPLICIT_ONLY, DEPFILE, + JOB_POOL, MAIN_DEPENDENCY\. + + Policy CMP0175 is not set: add_custom_command\(\) rejects invalid arguments\. + Run "cmake --help-policy CMP0175" for policy details\. Use the cmake_policy + command to set the policy and suppress this warning\. +Call Stack \(most recent call first\): + CMP0175-WARN\.cmake:1 \(include\) + CMakeLists.txt:3 \(include\) +This warning is for project developers\. Use -Wno-dev to suppress it\. + +CMake Warning \(dev\) at CMP0175\.cmake:[0-9]+ \(add_custom_command\): + The following keywords are not supported when using + add_custom_command\(TARGET\): IMPLICIT_DEPENDS, USES_TERMINAL\. + + Policy CMP0175 is not set: add_custom_command\(\) rejects invalid arguments\. + Run "cmake --help-policy CMP0175" for policy details\. Use the cmake_policy + command to set the policy and suppress this warning\. +Call Stack \(most recent call first\): + CMP0175-WARN\.cmake:1 \(include\) + CMakeLists\.txt:3 \(include\) +This warning is for project developers\. Use -Wno-dev to suppress it\. + +CMake Warning \(dev\) at CMP0175\.cmake:[0-9]+ \(add_custom_command\): + Exactly one of PRE_BUILD, PRE_LINK, or POST_BUILD must be given\. Assuming + POST_BUILD to preserve backward compatibility\. + + Policy CMP0175 is not set: add_custom_command\(\) rejects invalid arguments\. + Run "cmake --help-policy CMP0175" for policy details\. Use the cmake_policy + command to set the policy and suppress this warning\. +Call Stack \(most recent call first\): + CMP0175-WARN\.cmake:1 \(include\) + CMakeLists\.txt:3 \(include\) +This warning is for project developers\. Use -Wno-dev to suppress it\. + +CMake Warning \(dev\) at CMP0175\.cmake:[0-9]+ \(add_custom_command\): + Exactly one of PRE_BUILD, PRE_LINK, or POST_BUILD must be given\. Assuming + POST_BUILD to preserve backward compatibility\. + + Policy CMP0175 is not set: add_custom_command\(\) rejects invalid arguments\. + Run "cmake --help-policy CMP0175" for policy details\. Use the cmake_policy + command to set the policy and suppress this warning\. +Call Stack \(most recent call first\): + CMP0175-WARN\.cmake:1 \(include\) + CMakeLists\.txt:3 \(include\) +This warning is for project developers\. Use -Wno-dev to suppress it\. + +CMake Warning \(dev\) at CMP0175\.cmake:[0-9]+ \(add_custom_command\): + At least one COMMAND must be given\. + + Policy CMP0175 is not set: add_custom_command\(\) rejects invalid arguments\. + Run "cmake --help-policy CMP0175" for policy details\. Use the cmake_policy + command to set the policy and suppress this warning\. +Call Stack \(most recent call first\): + CMP0175-WARN\.cmake:1 \(include\) + CMakeLists\.txt:3 \(include\) +This warning is for project developers\. Use -Wno-dev to suppress it\. + +CMake Warning \(dev\) at CMP0175\.cmake:[0-9]+ \(add_custom_command\): + The following keywords are not supported when using + add_custom_command\(OUTPUT\): OUTPUTS, POST_BUILD, PRE_BUILD, PRE_LINK, + SOURCE\. + + Policy CMP0175 is not set: add_custom_command\(\) rejects invalid arguments\. + Run "cmake --help-policy CMP0175" for policy details\. Use the cmake_policy + command to set the policy and suppress this warning\. +Call Stack \(most recent call first\): + CMP0175-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/add_custom_command/CMP0175-WARN.cmake b/Tests/RunCMake/add_custom_command/CMP0175-WARN.cmake new file mode 100644 index 0000000000..cd89b53588 --- /dev/null +++ b/Tests/RunCMake/add_custom_command/CMP0175-WARN.cmake @@ -0,0 +1 @@ +include(CMP0175.cmake) diff --git a/Tests/RunCMake/add_custom_command/CMP0175.cmake b/Tests/RunCMake/add_custom_command/CMP0175.cmake new file mode 100644 index 0000000000..0c20f4e3be --- /dev/null +++ b/Tests/RunCMake/add_custom_command/CMP0175.cmake @@ -0,0 +1,65 @@ +enable_language(CXX) +file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/main.cpp "int main() {}") +add_executable(main ${CMAKE_CURRENT_BINARY_DIR}/main.cpp) + +#============================================================================ +# add_custom_command(TARGET) +#============================================================================ + +# Unsupported keywords. Need to test them in batches to avoid other checks. +add_custom_command(TARGET main + POST_BUILD + COMMAND ${CMAKE_COMMAND} -E true + + # None of the following are allowed for the TARGET form + + #APPEND # Has its own check requiring OUTPUT to be set + #CODEGEN # Other checks will fail before the CMP0175 check + DEPENDS valueDoesNotMatterHere + DEPENDS_EXPLICIT_ONLY YES + DEPFILE valueDoesNotMatterHere + #IMPLICIT_DEPENDS # Earlier check fails when DEPFILE is present + JOB_POOL valueDoesNotMatterHere + MAIN_DEPENDENCY valueDoesNotMatterHere + #OUTPUT # Other checks will fail before the CMP0175 check + #OUTPUTS # Special case, not a documented keyword (used for deprecated form) + #SOURCE # Old signature, special handling makes it hard to check + #USES_TERMINAL +) +add_custom_command(TARGET main + POST_BUILD + COMMAND ${CMAKE_COMMAND} -E true + # Has to be tested separately due to separate check for clash with DEPFILE + IMPLICIT_DEPENDS valueDoesNotMatterHere + # Has to be tested separately due to separate check for clash with JOB_POOL + USES_TERMINAL NO +) + +# Missing any PRE_BUILD, PRE_LINK, or POST_BUILD +add_custom_command(TARGET main + COMMAND ${CMAKE_COMMAND} -E true +) + +# More than one of PRE_BUILD, PRE_LINK, or POST_BUILD +add_custom_command(TARGET main + PRE_BUILD PRE_LINK POST_BUILD + COMMAND ${CMAKE_COMMAND} -E true +) + +# Missing COMMAND +add_custom_command(TARGET main + POST_BUILD + COMMENT "Need at least 4 arguments, so added this comment" +) + +#============================================================================ +# add_custom_command(OUTPUT) +#============================================================================ + +add_custom_command(OUTPUT blah.txt + OUTPUTS + POST_BUILD + PRE_BUILD + PRE_LINK + SOURCE +) diff --git a/Tests/RunCMake/add_custom_command/RunCMakeTest.cmake b/Tests/RunCMake/add_custom_command/RunCMakeTest.cmake index 46e7baea1a..820591c933 100644 --- a/Tests/RunCMake/add_custom_command/RunCMakeTest.cmake +++ b/Tests/RunCMake/add_custom_command/RunCMakeTest.cmake @@ -1,5 +1,8 @@ include(RunCMake) +run_cmake(CMP0175-OLD) +run_cmake(CMP0175-WARN) +run_cmake(CMP0175-NEW) run_cmake(AppendLiteralQuotes) run_cmake(AppendNoOutput) run_cmake(AppendNotOutput)