From 47f0dbd70b83ccb5e314abee5feded2ed50a23fb Mon Sep 17 00:00:00 2001 From: Eric NOULARD Date: Mon, 14 May 2012 23:29:42 +0200 Subject: [PATCH] CPack add necessary check to detect/warns/error on ABSOLUTE DESTINATION The [usually] wrong usage of absolute DESTINATION in INSTALL rules keeps popping-up on the ML. We shall have some way to: 1) easily detect it. 2) forbids this for some CPack generator like NSIS In fact it should certainly be forbidden for *any* generators when used on Windows but we may implements that on top of the current patch. The patch ask the task to the generated cmake_install.cmake scripts. Those scripts are a little bit more complicated with that but iff there are absolute DESTINATION. This cost nothing if relative DESTINATION are used. Two new vars are introduced (and documented to handle that): CPACK_WARN_ON_ABSOLUTE_INSTALL_DESTINATION and CPACK_ERROR_ON_ABSOLUTE_INSTALL_DESTINATION --- Source/CPack/cmCPackDocumentVariables.cxx | 16 +++++++++++++++ Source/CPack/cmCPackGenerator.cxx | 25 +++++++++++++++++++++++ Source/CPack/cmCPackGenerator.h | 11 +++++++++- Source/CPack/cmCPackNSISGenerator.cxx | 6 ++++++ Source/CPack/cmCPackNSISGenerator.h | 1 + Source/cmInstallGenerator.cxx | 9 ++++++++ 6 files changed, 67 insertions(+), 1 deletion(-) diff --git a/Source/CPack/cmCPackDocumentVariables.cxx b/Source/CPack/cmCPackDocumentVariables.cxx index d2e3802c6..b5294805d 100644 --- a/Source/CPack/cmCPackDocumentVariables.cxx +++ b/Source/CPack/cmCPackDocumentVariables.cxx @@ -77,4 +77,20 @@ void cmCPackDocumentVariables::DefineVariables(cmake* cm) "which is done right before packaging the files." " The script is not called by e.g.: make install.", false, "Variables common to all CPack generators"); + + cm->DefineProperty + ("CPACK_WARN_ON_ABSOLUTE_INSTALL_DESTINATION", cmProperty::VARIABLE, + "Ask CPack to warn each time a file with absolute INSTALL" + " DESTINATION is encountered.", + "", false, + "Variables common to all CPack generators"); + + cm->DefineProperty + ("CPACK_ERROR_ON_ABSOLUTE_INSTALL_DESTINATION", cmProperty::VARIABLE, + "Ask CPack to error out as soon as a file with absolute INSTALL" + " DESTINATION is encountered", + "The fatal error is emitted before the installation of " + "the offending file takes place. Some CPack generators, like NSIS," + "enforce this internally.", false, + "Variables common to all CPack generators"); } diff --git a/Source/CPack/cmCPackGenerator.cxx b/Source/CPack/cmCPackGenerator.cxx index bf652718e..ca790c02c 100644 --- a/Source/CPack/cmCPackGenerator.cxx +++ b/Source/CPack/cmCPackGenerator.cxx @@ -849,8 +849,27 @@ int cmCPackGenerator::InstallProjectViaInstallCMakeProjects( filesBefore = glB.GetFiles(); std::sort(filesBefore.begin(),filesBefore.end()); } + + // If CPack was asked to warn on ABSOLUTE INSTALL DESTINATION + // then forward request to cmake_install.cmake script + if (this->GetOption("CPACK_WARN_ON_ABSOLUTE_INSTALL_DESTINATION")) + { + mf->AddDefinition("CPACK_WARN_ON_ABSOLUTE_INSTALL_DESTINATION", + "1"); + } + // If current CPack generator does support + // ABSOLUTE INSTALL DESTINATION or CPack has been asked for + // then ask cmake_install.cmake script to error out + // as soon as it occurs (before installing file) + if (!SupportsAbsoluteDestination() || + this->GetOption("CPACK_ERROR_ON_ABSOLUTE_INSTALL_DESTINATION")) + { + mf->AddDefinition("CPACK_ERROR_ON_ABSOLUTE_INSTALL_DESTINATION", + "1"); + } // do installation int res = mf->ReadListFile(0, installFile.c_str()); + // Now rebuild the list of files after installation // of the current component (if we are in component install) if (componentInstall) @@ -1460,6 +1479,12 @@ cmCPackGenerator::SupportsSetDestdir() const return cmCPackGenerator::SETDESTDIR_SUPPORTED; } +//---------------------------------------------------------------------- +bool cmCPackGenerator::SupportsAbsoluteDestination() const +{ + return true; +} + //---------------------------------------------------------------------- bool cmCPackGenerator::SupportsComponentInstallation() const { diff --git a/Source/CPack/cmCPackGenerator.h b/Source/CPack/cmCPackGenerator.h index 68ee3f2e0..ddde8b5d8 100644 --- a/Source/CPack/cmCPackGenerator.h +++ b/Source/CPack/cmCPackGenerator.h @@ -206,12 +206,21 @@ protected: /** * Does the CPack generator support CPACK_SET_DESTDIR? - * The default legacy value is 'true' generator + * The default legacy value is 'SETDESTDIR_SUPPORTED' generator * have to override it in order change this. * @return CPackSetDestdirSupport */ virtual enum CPackSetDestdirSupport SupportsSetDestdir() const; + /** + * Does the CPack generator support absolute path + * in INSTALL DESTINATION? + * The default legacy value is 'true' generator + * have to override it in order change this. + * @return true if supported false otherwise + */ + virtual bool SupportsAbsoluteDestination() const; + /** * Does the CPack generator support component installation?. * Some Generators requires the user to set diff --git a/Source/CPack/cmCPackNSISGenerator.cxx b/Source/CPack/cmCPackNSISGenerator.cxx index 63189de2e..481fd2b6e 100644 --- a/Source/CPack/cmCPackNSISGenerator.cxx +++ b/Source/CPack/cmCPackNSISGenerator.cxx @@ -638,6 +638,12 @@ cmCPackNSISGenerator::SupportsSetDestdir() const return cmCPackGenerator::SETDESTDIR_SHOULD_NOT_BE_USED; } +//---------------------------------------------------------------------- +bool cmCPackNSISGenerator::SupportsAbsoluteDestination() const +{ + return false; +} + //---------------------------------------------------------------------- bool cmCPackNSISGenerator::SupportsComponentInstallation() const { diff --git a/Source/CPack/cmCPackNSISGenerator.h b/Source/CPack/cmCPackNSISGenerator.h index c2987d0ba..82248546f 100644 --- a/Source/CPack/cmCPackNSISGenerator.h +++ b/Source/CPack/cmCPackNSISGenerator.h @@ -45,6 +45,7 @@ protected: std::vector& dirs); enum cmCPackGenerator::CPackSetDestdirSupport SupportsSetDestdir() const; + virtual bool SupportsAbsoluteDestination() const; virtual bool SupportsComponentInstallation() const; /// Produce a string that contains the NSIS code to describe a diff --git a/Source/cmInstallGenerator.cxx b/Source/cmInstallGenerator.cxx index 807168e0d..2150ec1b8 100644 --- a/Source/cmInstallGenerator.cxx +++ b/Source/cmInstallGenerator.cxx @@ -80,6 +80,15 @@ void cmInstallGenerator } } os << "\")\n"; + os << indent << "IF (CPACK_WARN_ON_ABSOLUTE_INSTALL_DESTINATION)\n"; + os << indent << indent << "message(WARNING \"ABSOLUTE path INSTALL " + << "DESTINATION : ${CPACK_ABSOLUTE_DESTINATION_FILES}\")\n"; + os << indent << "ENDIF (CPACK_WARN_ON_ABSOLUTE_INSTALL_DESTINATION)\n"; + + os << indent << "IF (CPACK_ERROR_ON_ABSOLUTE_INSTALL_DESTINATION)\n"; + os << indent << indent << "message(FATAL_ERROR \"ABSOLUTE path INSTALL " + << "DESTINATION forbidden (by CPack): ${CPACK_ABSOLUTE_DESTINATION_FILES}\")\n"; + os << indent << "ENDIF (CPACK_ERROR_ON_ABSOLUTE_INSTALL_DESTINATION)\n"; } os << "FILE(INSTALL DESTINATION \"" << dest << "\" TYPE " << stype.c_str(); if(optional)