diff --git a/Help/release/dev/install-nonextant-file-set.rst b/Help/release/dev/install-nonextant-file-set.rst new file mode 100644 index 0000000000..fd94089fcc --- /dev/null +++ b/Help/release/dev/install-nonextant-file-set.rst @@ -0,0 +1,7 @@ +install-nonextant-file-set +-------------------------- + +* The :command:`install(TARGETS)` command no longer ignores file sets which + haven't been defined at the point it is called. The ordering of + :command:`target_sources(FILE_SET)` and ``install(TARGETS)`` is no longer + semantically relevant. diff --git a/Source/cmExportInstallCMakeConfigGenerator.cxx b/Source/cmExportInstallCMakeConfigGenerator.cxx index 2154e691f4..166833d3c4 100644 --- a/Source/cmExportInstallCMakeConfigGenerator.cxx +++ b/Source/cmExportInstallCMakeConfigGenerator.cxx @@ -286,7 +286,8 @@ std::string cmExportInstallCMakeConfigGenerator::GetFileSetDirectories( gte->Makefile->GetGeneratorConfigs(cmMakefile::IncludeEmptyConfig); cmGeneratorExpression ge(*gte->Makefile->GetCMakeInstance()); - auto cge = ge.Parse(te->FileSetGenerators.at(fileSet)->GetDestination()); + auto cge = + ge.Parse(te->FileSetGenerators.at(fileSet->GetName())->GetDestination()); for (auto const& config : configs) { auto unescapedDest = cge->Evaluate(gte->LocalGenerator, config, gte); @@ -334,8 +335,8 @@ std::string cmExportInstallCMakeConfigGenerator::GetFileSetFiles( auto directoryEntries = fileSet->CompileDirectoryEntries(); cmGeneratorExpression destGe(*gte->Makefile->GetCMakeInstance()); - auto destCge = - destGe.Parse(te->FileSetGenerators.at(fileSet)->GetDestination()); + auto destCge = destGe.Parse( + te->FileSetGenerators.at(fileSet->GetName())->GetDestination()); for (auto const& config : configs) { auto directories = fileSet->EvaluateDirectoryEntries( diff --git a/Source/cmExportSet.cxx b/Source/cmExportSet.cxx index f5d93ba64b..d96523ebf4 100644 --- a/Source/cmExportSet.cxx +++ b/Source/cmExportSet.cxx @@ -40,9 +40,8 @@ bool cmExportSet::Compute(cmLocalGenerator* lg) tgtExport->Target->Target->GetAllInterfaceFileSets(); auto const fileSetInTargetExport = [&tgtExport, lg](std::string const& fileSetName) -> bool { - auto* fileSet = tgtExport->Target->Target->GetFileSet(fileSetName); - - if (!tgtExport->FileSetGenerators.count(fileSet)) { + if (tgtExport->FileSetGenerators.find(fileSetName) == + tgtExport->FileSetGenerators.end()) { lg->IssueMessage(MessageType::FATAL_ERROR, cmStrCat("File set \"", fileSetName, "\" is listed in interface file sets of ", diff --git a/Source/cmFileAPICodemodel.cxx b/Source/cmFileAPICodemodel.cxx index 5054021b81..b6d9bba95a 100644 --- a/Source/cmFileAPICodemodel.cxx +++ b/Source/cmFileAPICodemodel.cxx @@ -1071,7 +1071,7 @@ Json::Value DirectoryObject::DumpInstaller(cmInstallGenerator* gen) installer["type"] = "fileSet"; installer["destination"] = installFileSet->GetDestination(this->Config); - auto* fileSet = installFileSet->GetFileSet(); + auto const* fileSet = installFileSet->GetFileSet(); auto* target = installFileSet->GetTarget(); auto dirCges = fileSet->CompileDirectoryEntries(); diff --git a/Source/cmInstallCommand.cxx b/Source/cmInstallCommand.cxx index 8e3293e940..0535d5378a 100644 --- a/Source/cmInstallCommand.cxx +++ b/Source/cmInstallCommand.cxx @@ -24,7 +24,6 @@ #include "cmExecutionStatus.h" #include "cmExperimental.h" #include "cmExportSet.h" -#include "cmFileSet.h" #include "cmGeneratorExpression.h" #include "cmGlobalGenerator.h" #include "cmInstallAndroidMKExportGenerator.h" @@ -198,15 +197,15 @@ std::unique_ptr CreateInstallFilesGenerator( } std::unique_ptr CreateInstallFileSetGenerator( - Helper& helper, cmTarget& target, cmFileSet* fileSet, - std::string const& destination, cmInstallCommandArguments const& args) + Helper& helper, cmTarget& target, cmFileSetDestinations dests, + cmInstallCommandFileSetArguments const& args) { cmInstallGenerator::MessageLevel message = cmInstallGenerator::SelectMessageLevel(helper.Makefile); return cm::make_unique( - target.GetName(), fileSet, destination, args.GetPermissions(), - args.GetConfigurations(), args.GetComponent(), message, - args.GetExcludeFromAll(), args.GetOptional(), + target.GetName(), args.GetFileSet(), std::move(dests), + args.GetPermissions(), args.GetConfigurations(), args.GetComponent(), + message, args.GetExcludeFromAll(), args.GetOptional(), helper.Makefile->GetBacktrace()); } @@ -809,7 +808,7 @@ bool HandleTargetsMode(std::vector const& args, te->RuntimeGenerator = runtimeGenerator.get(); te->ObjectsGenerator = objectGenerator.get(); for (auto const& gen : fileSetGenerators) { - te->FileSetGenerators[gen->GetFileSet()] = gen.get(); + te->FileSetGenerators[gen->GetFileSetName()] = gen.get(); } te->CxxModuleBmiGenerator = cxxModuleBmiGenerator.get(); target.AddInstallIncludeDirectories( @@ -1145,31 +1144,12 @@ bool HandleTargetsMode(std::vector const& args, if (!namelinkOnly) { for (std::size_t i = 0; i < fileSetArgs.size(); i++) { - if (auto* fileSet = target.GetFileSet(fileSetArgs[i].GetFileSet())) { - cmList interfaceFileSetEntries{ target.GetSafeProperty( - cmTarget::GetInterfaceFileSetsPropertyName(fileSet->GetType())) }; - if (std::find(interfaceFileSetEntries.begin(), - interfaceFileSetEntries.end(), - fileSetArgs[i].GetFileSet()) != - interfaceFileSetEntries.end()) { - std::string destination; - if (fileSet->GetType() == "HEADERS"_s) { - destination = helper.GetIncludeDestination(&fileSetArgs[i]); - } else { - destination = fileSetArgs[i].GetDestination(); - if (destination.empty()) { - status.SetError(cmStrCat( - "TARGETS given no FILE_SET DESTINATION for target \"", - target.GetName(), "\" file set \"", fileSet->GetName(), - "\".")); - return false; - } - } - fileSetGenerators.push_back(CreateInstallFileSetGenerator( - helper, target, fileSet, destination, fileSetArgs[i])); - installsFileSet[i] = true; - } - } + cmFileSetDestinations dests; + dests.Headers = helper.GetIncludeDestination(&fileSetArgs[i]); + dests.CXXModules = fileSetArgs[i].GetDestination(); + fileSetGenerators.push_back(CreateInstallFileSetGenerator( + helper, target, std::move(dests), fileSetArgs[i])); + installsFileSet[i] = true; } } diff --git a/Source/cmInstallFileSetGenerator.cxx b/Source/cmInstallFileSetGenerator.cxx index d40d262fd5..e92f4299d7 100644 --- a/Source/cmInstallFileSetGenerator.cxx +++ b/Source/cmInstallFileSetGenerator.cxx @@ -2,29 +2,38 @@ file LICENSE.rst or https://cmake.org/licensing for details. */ #include "cmInstallFileSetGenerator.h" +#include #include #include #include #include +#include +#include + #include "cmFileSet.h" #include "cmGeneratorExpression.h" +#include "cmGeneratorTarget.h" #include "cmGlobalGenerator.h" #include "cmInstallType.h" +#include "cmList.h" #include "cmListFileCache.h" #include "cmLocalGenerator.h" +#include "cmMessageType.h" #include "cmStringAlgorithms.h" +#include "cmTarget.h" cmInstallFileSetGenerator::cmInstallFileSetGenerator( - std::string targetName, cmFileSet* fileSet, std::string const& dest, + std::string targetName, std::string fileSetName, cmFileSetDestinations dests, std::string file_permissions, std::vector const& configurations, std::string const& component, MessageLevel message, bool exclude_from_all, bool optional, cmListFileBacktrace backtrace) - : cmInstallGenerator(dest, configurations, component, message, + : cmInstallGenerator("", configurations, component, message, exclude_from_all, false, std::move(backtrace)) , TargetName(std::move(targetName)) - , FileSet(fileSet) + , FileSetName(std::move(fileSetName)) , FilePermissions(std::move(file_permissions)) + , FileSetDestinations(std::move(dests)) , Optional(optional) { this->ActionsPerConfig = true; @@ -44,6 +53,39 @@ bool cmInstallFileSetGenerator::Compute(cmLocalGenerator* lg) lg->GetGlobalGenerator()->FindGeneratorTarget(this->TargetName); } + auto const& target = *this->Target->Target; + this->FileSet = target.GetFileSet(this->FileSetName); + + if (!this->FileSet) { + // No file set of the given name was ever provided for this target, nothing + // for this generator to do + return true; + } + + cmList interfaceFileSetEntries{ target.GetSafeProperty( + cmTarget::GetInterfaceFileSetsPropertyName(this->FileSet->GetType())) }; + + if (std::find(interfaceFileSetEntries.begin(), interfaceFileSetEntries.end(), + this->FileSetName) != interfaceFileSetEntries.end()) { + if (this->FileSet->GetType() == "HEADERS"_s) { + this->Destination = this->FileSetDestinations.Headers; + } else { + this->Destination = this->FileSetDestinations.CXXModules; + } + } else { + // File set of the given name was provided but it's private, so give up + this->FileSet = nullptr; + return true; + } + + if (this->Destination.empty()) { + lg->IssueMessage(MessageType::FATAL_ERROR, + cmStrCat("File set \"", this->FileSetName, + "\" installed by target \"", this->TargetName, + "\" has no destination.")); + return false; + } + return true; } @@ -57,6 +99,11 @@ std::string cmInstallFileSetGenerator::GetDestination( void cmInstallFileSetGenerator::GenerateScriptForConfig( std::ostream& os, std::string const& config, Indent indent) { + + if (!this->FileSet) { + return; + } + for (auto const& dirEntry : this->CalculateFilesPerDir(config)) { std::string destSub; if (!dirEntry.first.empty()) { diff --git a/Source/cmInstallFileSetGenerator.h b/Source/cmInstallFileSetGenerator.h index b13a5e06f2..ba30414edc 100644 --- a/Source/cmInstallFileSetGenerator.h +++ b/Source/cmInstallFileSetGenerator.h @@ -14,11 +14,17 @@ class cmFileSet; class cmListFileBacktrace; class cmLocalGenerator; +struct cmFileSetDestinations +{ + std::string Headers; + std::string CXXModules; +}; + class cmInstallFileSetGenerator : public cmInstallGenerator { public: - cmInstallFileSetGenerator(std::string targetName, cmFileSet* fileSet, - std::string const& dest, + cmInstallFileSetGenerator(std::string targetName, std::string fileSetName, + cmFileSetDestinations dests, std::string file_permissions, std::vector const& configurations, std::string const& component, MessageLevel message, @@ -31,7 +37,8 @@ public: std::string GetDestination(std::string const& config) const; std::string GetDestination() const { return this->Destination; } bool GetOptional() const { return this->Optional; } - cmFileSet* GetFileSet() const { return this->FileSet; } + std::string GetFileSetName() const { return this->FileSetName; } + cmFileSet const* GetFileSet() const { return this->FileSet; }; cmGeneratorTarget* GetTarget() const { return this->Target; } protected: @@ -41,8 +48,10 @@ protected: private: std::string TargetName; cmLocalGenerator* LocalGenerator; - cmFileSet* const FileSet; + cmFileSet const* FileSet; + std::string const FileSetName; std::string const FilePermissions; + cmFileSetDestinations FileSetDestinations; bool const Optional; cmGeneratorTarget* Target; diff --git a/Source/cmInstallGenerator.h b/Source/cmInstallGenerator.h index 27f4b4f2e5..572d192361 100644 --- a/Source/cmInstallGenerator.h +++ b/Source/cmInstallGenerator.h @@ -93,7 +93,7 @@ protected: TweakMethod const& tweak); // Information shared by most generator types. - std::string const Destination; + std::string Destination; std::string const Component; MessageLevel const Message; bool const ExcludeFromAll; diff --git a/Source/cmTargetExport.h b/Source/cmTargetExport.h index 3c46c35c13..88788cfb64 100644 --- a/Source/cmTargetExport.h +++ b/Source/cmTargetExport.h @@ -7,7 +7,6 @@ #include #include -class cmFileSet; class cmGeneratorTarget; class cmInstallCxxModuleBmiGenerator; class cmInstallFileSetGenerator; @@ -33,7 +32,7 @@ public: cmInstallTargetGenerator* FrameworkGenerator; cmInstallTargetGenerator* BundleGenerator; cmInstallFilesGenerator* HeaderGenerator; - std::map FileSetGenerators; + std::map FileSetGenerators; cmInstallCxxModuleBmiGenerator* CxxModuleBmiGenerator; ///@} diff --git a/Tests/RunCMake/target_sources/FileSetInstallFirstExport.cmake b/Tests/RunCMake/target_sources/FileSetInstallFirstExport.cmake new file mode 100644 index 0000000000..445ec4f4d7 --- /dev/null +++ b/Tests/RunCMake/target_sources/FileSetInstallFirstExport.cmake @@ -0,0 +1,22 @@ +enable_language(C) + +add_library(lib1 STATIC lib1.c) + +install(TARGETS lib1 EXPORT export FILE_SET HEADERS FILE_SET a FILE_SET b FILE_SET c DESTINATION include/dir FILE_SET d FILE_SET e FILE_SET f DESTINATION include/$,debug,release> FILE_SET g FILE_SET dir3 DESTINATION include/dir3) +install(EXPORT export FILE export.cmake NAMESPACE install:: DESTINATION lib/cmake) +export(EXPORT export FILE export.cmake NAMESPACE export::) + +target_sources(lib1 + PUBLIC FILE_SET HEADERS BASE_DIRS ${CMAKE_CURRENT_SOURCE_DIR} FILES error.c + PRIVATE FILE_SET a TYPE HEADERS BASE_DIRS ${CMAKE_CURRENT_SOURCE_DIR} FILES h1.h + PUBLIC FILE_SET b TYPE HEADERS BASE_DIRS ${CMAKE_CURRENT_SOURCE_DIR} FILES h2.h + INTERFACE FILE_SET c TYPE HEADERS BASE_DIRS "$<1:${CMAKE_CURRENT_SOURCE_DIR}/dir>" FILES "$<1:dir/dir.h>" + INTERFACE FILE_SET d TYPE HEADERS BASE_DIRS FILES "${CMAKE_CURRENT_SOURCE_DIR}/$,debug,release>/empty.h" + INTERFACE FILE_SET e TYPE HEADERS BASE_DIRS "${CMAKE_CURRENT_SOURCE_DIR}/$,debug,release>" FILES "${CMAKE_CURRENT_SOURCE_DIR}/$,debug,release>/empty2.h" + INTERFACE FILE_SET f TYPE HEADERS BASE_DIRS "${CMAKE_CURRENT_SOURCE_DIR}" FILES "${CMAKE_CURRENT_SOURCE_DIR}/empty3.h" + INTERFACE FILE_SET g TYPE HEADERS BASE_DIRS "${CMAKE_CURRENT_SOURCE_DIR}/dir1" "${CMAKE_CURRENT_SOURCE_DIR}/dir2" FILES "${CMAKE_CURRENT_SOURCE_DIR}/dir1/file1.h" "${CMAKE_CURRENT_SOURCE_DIR}/dir2/file2.h" + INTERFACE FILE_SET dir3 TYPE HEADERS BASE_DIRS "${CMAKE_CURRENT_SOURCE_DIR}/dir3" FILES dir3/dir3.h + ) + +add_library(lib2 STATIC lib2.c) +target_link_libraries(lib2 PRIVATE lib1) diff --git a/Tests/RunCMake/target_sources/RunCMakeTest.cmake b/Tests/RunCMake/target_sources/RunCMakeTest.cmake index 1fe9427ef3..57e8b8928c 100644 --- a/Tests/RunCMake/target_sources/RunCMakeTest.cmake +++ b/Tests/RunCMake/target_sources/RunCMakeTest.cmake @@ -61,6 +61,12 @@ run_cmake(FileSetFileNoExist) unset(RunCMake_TEST_OPTIONS) function(run_export_import name) + if(ARGV1) + set(importname ${ARGV1}) + else() + set(importname ${name}) + endif() + if(RunCMake_GENERATOR_IS_MULTI_CONFIG) set(_config_options "-DCMAKE_CONFIGURATION_TYPES=Debug\\\\;Release") else() @@ -81,14 +87,14 @@ function(run_export_import name) endif() unset(RunCMake_TEST_OPTIONS) - set(RunCMake_TEST_BINARY_DIR "${RunCMake_BINARY_DIR}/${name}Import-build") + set(RunCMake_TEST_BINARY_DIR "${RunCMake_BINARY_DIR}/${importname}Import-build") unset(RunCMake_TEST_OPTIONS) file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}") file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}") - run_cmake(${name}Import) - run_cmake_command(${name}Import-build ${CMAKE_COMMAND} --build . --config Debug) + run_cmake(${importname}Import) + run_cmake_command(${importname}Import-build ${CMAKE_COMMAND} --build . --config Debug) if(RunCMake_GENERATOR_IS_MULTI_CONFIG) - run_cmake_command(${name}Import-build ${CMAKE_COMMAND} --build . --config Release) + run_cmake_command(${importname}Import-build ${CMAKE_COMMAND} --build . --config Release) endif() unset(RunCMake_TEST_BINARY_DIR) @@ -96,4 +102,5 @@ function(run_export_import name) endfunction() run_export_import(FileSet) +run_export_import(FileSetInstallFirst FileSet) run_export_import(FileSetAbsoluteInstallIncludeDir)