From 0a4af0735f2aeec74a723dd6af31db585f1b664e Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 12 Jun 2016 18:38:33 +0200 Subject: [PATCH] cmake: Issue message independent of cmMakefile definition The makefile is only used when called by the cmMessageCommand, so inline the use of it there. It otherwise creates an undesirable dependency on cmMakefile for issuing messages in the cmake instance, a violation of the Interface Segregation Principle. https://en.wikipedia.org/wiki/Interface_segregation_principle This also makes it more explicit that the variable definitions only affect the message() command. If an AUTHOR_WARNING is issued for any other reason, it is not affected. To affect that, it is necessary to set the cache variable instead of the regular variable. This is an unfortunate interface quirk, but one which can't be fixed easily now. --- Source/cmMessageCommand.cxx | 11 ++-- Source/cmake.cxx | 58 +++++-------------- Source/cmake.h | 8 +-- Tests/RunCMake/message/RunCMakeTest.cmake | 2 + .../message-internal-warning-stderr.txt | 13 +++++ .../message/message-internal-warning.cmake | 5 ++ .../nomessage-internal-warning-stderr.txt | 0 .../message/nomessage-internal-warning.cmake | 5 ++ 8 files changed, 51 insertions(+), 51 deletions(-) create mode 100644 Tests/RunCMake/message/message-internal-warning-stderr.txt create mode 100644 Tests/RunCMake/message/message-internal-warning.cmake create mode 100644 Tests/RunCMake/message/nomessage-internal-warning-stderr.txt create mode 100644 Tests/RunCMake/message/nomessage-internal-warning.cmake diff --git a/Source/cmMessageCommand.cxx b/Source/cmMessageCommand.cxx index f4458a7dac..dab90c9144 100644 --- a/Source/cmMessageCommand.cxx +++ b/Source/cmMessageCommand.cxx @@ -24,7 +24,6 @@ bool cmMessageCommand::InitialPass(std::vector const& args, cmake::MessageType type = cmake::MESSAGE; bool status = false; bool fatal = false; - cmake* cm = this->Makefile->GetCMakeInstance(); if (*i == "SEND_ERROR") { type = cmake::FATAL_ERROR; ++i; @@ -36,10 +35,11 @@ bool cmMessageCommand::InitialPass(std::vector const& args, type = cmake::WARNING; ++i; } else if (*i == "AUTHOR_WARNING") { - if (cm->GetDevWarningsAsErrors(this->Makefile)) { + if (this->Makefile->IsSet("CMAKE_SUPPRESS_DEVELOPER_ERRORS") && + !this->Makefile->IsOn("CMAKE_SUPPRESS_DEVELOPER_ERRORS")) { fatal = true; type = cmake::AUTHOR_ERROR; - } else if (!cm->GetSuppressDevWarnings(this->Makefile)) { + } else if (!this->Makefile->IsOn("CMAKE_SUPPRESS_DEVELOPER_WARNINGS")) { type = cmake::AUTHOR_WARNING; } else { return true; @@ -49,10 +49,11 @@ bool cmMessageCommand::InitialPass(std::vector const& args, status = true; ++i; } else if (*i == "DEPRECATION") { - if (cm->GetDeprecatedWarningsAsErrors(this->Makefile)) { + if (this->Makefile->IsOn("CMAKE_ERROR_DEPRECATED")) { fatal = true; type = cmake::DEPRECATION_ERROR; - } else if (!cm->GetSuppressDeprecatedWarnings(this->Makefile)) { + } else if ((!this->Makefile->IsSet("CMAKE_WARN_DEPRECATED") || + this->Makefile->IsOn("CMAKE_WARN_DEPRECATED"))) { type = cmake::DEPRECATION_WARNING; } else { return true; diff --git a/Source/cmake.cxx b/Source/cmake.cxx index ecbdc61a81..98ac518f6a 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -2447,19 +2447,11 @@ void cmake::RunCheckForUnusedVariables() #endif } -bool cmake::GetSuppressDevWarnings(cmMakefile const* mf) const +bool cmake::GetSuppressDevWarnings() const { - /* - * The suppression CMake variable may be set in the CMake configuration file - * itself, so we have to check what its set to in the makefile if we can. - */ - if (mf) { - return mf->IsOn("CMAKE_SUPPRESS_DEVELOPER_WARNINGS"); - } else { - const char* cacheEntryValue = - this->State->GetCacheEntryValue("CMAKE_SUPPRESS_DEVELOPER_WARNINGS"); - return cmSystemTools::IsOn(cacheEntryValue); - } + const char* cacheEntryValue = + this->State->GetCacheEntryValue("CMAKE_SUPPRESS_DEVELOPER_WARNINGS"); + return cmSystemTools::IsOn(cacheEntryValue); } void cmake::SetSuppressDevWarnings(bool b) @@ -2481,20 +2473,11 @@ void cmake::SetSuppressDevWarnings(bool b) cmState::INTERNAL); } -bool cmake::GetSuppressDeprecatedWarnings(cmMakefile const* mf) const +bool cmake::GetSuppressDeprecatedWarnings() const { - /* - * The suppression CMake variable may be set in the CMake configuration file - * itself, so we have to check what its set to in the makefile if we can. - */ - if (mf) { - return (mf->IsSet("CMAKE_WARN_DEPRECATED") && - !mf->IsOn("CMAKE_WARN_DEPRECATED")); - } else { - const char* cacheEntryValue = - this->State->GetCacheEntryValue("CMAKE_WARN_DEPRECATED"); - return cacheEntryValue && cmSystemTools::IsOff(cacheEntryValue); - } + const char* cacheEntryValue = + this->State->GetCacheEntryValue("CMAKE_WARN_DEPRECATED"); + return cacheEntryValue && cmSystemTools::IsOff(cacheEntryValue); } void cmake::SetSuppressDeprecatedWarnings(bool b) @@ -2516,16 +2499,11 @@ void cmake::SetSuppressDeprecatedWarnings(bool b) cmState::INTERNAL); } -bool cmake::GetDevWarningsAsErrors(cmMakefile const* mf) const +bool cmake::GetDevWarningsAsErrors() const { - if (mf) { - return (mf->IsSet("CMAKE_SUPPRESS_DEVELOPER_ERRORS") && - !mf->IsOn("CMAKE_SUPPRESS_DEVELOPER_ERRORS")); - } else { - const char* cacheEntryValue = - this->State->GetCacheEntryValue("CMAKE_SUPPRESS_DEVELOPER_ERRORS"); - return cacheEntryValue && cmSystemTools::IsOff(cacheEntryValue); - } + const char* cacheEntryValue = + this->State->GetCacheEntryValue("CMAKE_SUPPRESS_DEVELOPER_ERRORS"); + return cacheEntryValue && cmSystemTools::IsOff(cacheEntryValue); } void cmake::SetDevWarningsAsErrors(bool b) @@ -2547,15 +2525,11 @@ void cmake::SetDevWarningsAsErrors(bool b) cmState::INTERNAL); } -bool cmake::GetDeprecatedWarningsAsErrors(cmMakefile const* mf) const +bool cmake::GetDeprecatedWarningsAsErrors() const { - if (mf) { - return mf->IsOn("CMAKE_ERROR_DEPRECATED"); - } else { - const char* cacheEntryValue = - this->State->GetCacheEntryValue("CMAKE_ERROR_DEPRECATED"); - return cmSystemTools::IsOn(cacheEntryValue); - } + const char* cacheEntryValue = + this->State->GetCacheEntryValue("CMAKE_ERROR_DEPRECATED"); + return cmSystemTools::IsOn(cacheEntryValue); } void cmake::SetDeprecatedWarningsAsErrors(bool b) diff --git a/Source/cmake.h b/Source/cmake.h index 4958a05dc6..aa1ff2caf1 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -339,7 +339,7 @@ public: * Returns false, by default, if developer warnings should be shown, true * otherwise. */ - bool GetSuppressDevWarnings(cmMakefile const* mf = NULL) const; + bool GetSuppressDevWarnings() const; /* * Set the state of the suppression of developer (author) warnings. */ @@ -350,7 +350,7 @@ public: * Returns false, by default, if deprecated warnings should be shown, true * otherwise. */ - bool GetSuppressDeprecatedWarnings(cmMakefile const* mf = NULL) const; + bool GetSuppressDeprecatedWarnings() const; /* * Set the state of the suppression of deprecated warnings. */ @@ -361,7 +361,7 @@ public: * Returns false, by default, if warnings should not be treated as errors, * true otherwise. */ - bool GetDevWarningsAsErrors(cmMakefile const* mf = NULL) const; + bool GetDevWarningsAsErrors() const; /** * Set the state of treating developer (author) warnings as errors. */ @@ -372,7 +372,7 @@ public: * Returns false, by default, if warnings should not be treated as errors, * true otherwise. */ - bool GetDeprecatedWarningsAsErrors(cmMakefile const* mf = NULL) const; + bool GetDeprecatedWarningsAsErrors() const; /** * Set the state of treating developer (author) warnings as errors. */ diff --git a/Tests/RunCMake/message/RunCMakeTest.cmake b/Tests/RunCMake/message/RunCMakeTest.cmake index 94896930a9..2346c86754 100644 --- a/Tests/RunCMake/message/RunCMakeTest.cmake +++ b/Tests/RunCMake/message/RunCMakeTest.cmake @@ -2,6 +2,8 @@ include(RunCMake) run_cmake(defaultmessage) run_cmake(nomessage) +run_cmake(message-internal-warning) +run_cmake(nomessage-internal-warning) run_cmake(warnmessage) # message command sets fatal occurred flag, so check each type of error diff --git a/Tests/RunCMake/message/message-internal-warning-stderr.txt b/Tests/RunCMake/message/message-internal-warning-stderr.txt new file mode 100644 index 0000000000..25946e9b2f --- /dev/null +++ b/Tests/RunCMake/message/message-internal-warning-stderr.txt @@ -0,0 +1,13 @@ +^CMake Warning \(dev\) in message-internal-warning.cmake: + A logical block opening on the line + + .*Tests/RunCMake/message/message-internal-warning.cmake:4 \(macro\) + + closes on the line + + .*Tests/RunCMake/message/message-internal-warning.cmake:5 \(endmacro\) + + with mis-matching arguments. +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) +This warning is for project developers. Use -Wno-dev to suppress it.$ diff --git a/Tests/RunCMake/message/message-internal-warning.cmake b/Tests/RunCMake/message/message-internal-warning.cmake new file mode 100644 index 0000000000..33993c7ffc --- /dev/null +++ b/Tests/RunCMake/message/message-internal-warning.cmake @@ -0,0 +1,5 @@ + +set(CMAKE_SUPPRESS_DEVELOPER_WARNINGS ON) + +macro(mymacro) +endmacro(notmymacro) diff --git a/Tests/RunCMake/message/nomessage-internal-warning-stderr.txt b/Tests/RunCMake/message/nomessage-internal-warning-stderr.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/Tests/RunCMake/message/nomessage-internal-warning.cmake b/Tests/RunCMake/message/nomessage-internal-warning.cmake new file mode 100644 index 0000000000..3ec2e28d78 --- /dev/null +++ b/Tests/RunCMake/message/nomessage-internal-warning.cmake @@ -0,0 +1,5 @@ + +set(CMAKE_SUPPRESS_DEVELOPER_WARNINGS ON CACHE BOOL "") + +macro(mymacro) +endmacro(notmymacro)