From 9e5e2d704abaef1b0a45201985f06a15290c1d09 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 15 Oct 2020 15:07:21 -0400 Subject: Remove unnecessary arbitrary CollapseFullPath second arguments Some calls to CollapseFullPath that already have an absolute path were updated by commit 22f38c0d6b (cmake: avoid getcwd in `CollapseFullPath`, 2020-01-14, v3.17.0-rc1~171^2) to pass an arbitrary second argument to prevent unnecessary `getcwd` calls. Since then, the KWSys implementation of CollapseFullPath has learned to avoid unnecessary `getcwd` calls on its own, so we can drop the arbitrary second arguments to our CollapseFullPath calls. --- Source/cmAddCustomCommandCommand.cxx | 3 +-- Source/cmCustomCommandGenerator.cxx | 3 +-- Source/cmSourceFileLocation.cxx | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/Source/cmAddCustomCommandCommand.cxx b/Source/cmAddCustomCommandCommand.cxx index bea89c0..a8f7509 100644 --- a/Source/cmAddCustomCommandCommand.cxx +++ b/Source/cmAddCustomCommandCommand.cxx @@ -215,8 +215,7 @@ bool cmAddCustomCommandCommand(std::vector const& args, } if (cmSystemTools::FileIsFullPath(filename)) { - filename = cmSystemTools::CollapseFullPath( - filename, status.GetMakefile().GetHomeOutputDirectory()); + filename = cmSystemTools::CollapseFullPath(filename); } switch (doing) { case doing_depfile: diff --git a/Source/cmCustomCommandGenerator.cxx b/Source/cmCustomCommandGenerator.cxx index d8307f6..6f5c8e9 100644 --- a/Source/cmCustomCommandGenerator.cxx +++ b/Source/cmCustomCommandGenerator.cxx @@ -35,8 +35,7 @@ void AppendPaths(const std::vector& inputs, for (std::string& it : result) { cmSystemTools::ConvertToUnixSlashes(it); if (cmSystemTools::FileIsFullPath(it)) { - it = cmSystemTools::CollapseFullPath( - it, lg->GetMakefile()->GetHomeOutputDirectory()); + it = cmSystemTools::CollapseFullPath(it); } } cm::append(output, result); diff --git a/Source/cmSourceFileLocation.cxx b/Source/cmSourceFileLocation.cxx index 222bafa..921eb0e 100644 --- a/Source/cmSourceFileLocation.cxx +++ b/Source/cmSourceFileLocation.cxx @@ -33,8 +33,7 @@ cmSourceFileLocation::cmSourceFileLocation(cmMakefile const* mf, this->AmbiguousExtension = true; this->Directory = cmSystemTools::GetFilenamePath(name); if (cmSystemTools::FileIsFullPath(this->Directory)) { - this->Directory = cmSystemTools::CollapseFullPath( - this->Directory, mf->GetHomeOutputDirectory()); + this->Directory = cmSystemTools::CollapseFullPath(this->Directory); } this->Name = cmSystemTools::GetFilenameName(name); if (kind == cmSourceFileLocationKind::Known) { -- cgit v0.12 From 84fecbf2141c892009ce40d852f9ebf0f79be25a Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 22 Oct 2020 13:52:24 -0400 Subject: cmLocalNinjaGenerator: Remove leftover local debugging comment --- Source/cmLocalNinjaGenerator.cxx | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx index ce85320..d90a37b 100644 --- a/Source/cmLocalNinjaGenerator.cxx +++ b/Source/cmLocalNinjaGenerator.cxx @@ -603,11 +603,6 @@ void cmLocalNinjaGenerator::WriteCustomCommandBuildStatement( } } -#if 0 -# error TODO: Once CC in an ExternalProject target must provide the \ - file of each imported target that has an add_dependencies pointing \ - at us. How to know which ExternalProject step actually provides it? -#endif cmNinjaDeps ninjaOutputs(outputs.size() + byproducts.size()); std::transform(outputs.begin(), outputs.end(), ninjaOutputs.begin(), gg->MapToNinjaPath()); -- cgit v0.12 From fab772c3e16862c7e8200b1e6a77d7f8abc1055e Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 15 Oct 2020 14:32:38 -0400 Subject: cmAddCustomCommandCommand: Drop outdated comment --- Source/cmAddCustomCommandCommand.cxx | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/Source/cmAddCustomCommandCommand.cxx b/Source/cmAddCustomCommandCommand.cxx index a8f7509..c1f98fa 100644 --- a/Source/cmAddCustomCommandCommand.cxx +++ b/Source/cmAddCustomCommandCommand.cxx @@ -190,15 +190,7 @@ bool cmAddCustomCommandCommand(std::vector const& args, case doing_byproducts: if (!cmSystemTools::FileIsFullPath(copy)) { // This is an output to be generated, so it should be - // under the build tree. CMake 2.4 placed this under the - // source tree. However the only case that this change - // will break is when someone writes - // - // add_custom_command(OUTPUT out.txt ...) - // - // and later references "${CMAKE_CURRENT_SOURCE_DIR}/out.txt". - // This is fairly obscure so we can wait for someone to - // complain. + // under the build tree. filename = cmStrCat(mf.GetCurrentBinaryDirectory(), '/'); } filename += copy; -- cgit v0.12 From 2a640d41998b2b61ed23090cc6edaf6445caf6ed Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 22 Oct 2020 13:37:41 -0400 Subject: cmCustomCommandGenerator: Add move operations --- Source/cmCustomCommandGenerator.cxx | 14 +++++++------- Source/cmCustomCommandGenerator.h | 6 ++++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/Source/cmCustomCommandGenerator.cxx b/Source/cmCustomCommandGenerator.cxx index 6f5c8e9..13d2c37 100644 --- a/Source/cmCustomCommandGenerator.cxx +++ b/Source/cmCustomCommandGenerator.cxx @@ -47,7 +47,7 @@ cmCustomCommandGenerator::cmCustomCommandGenerator(cmCustomCommand const& cc, std::string config, cmLocalGenerator* lg, bool transformDepfile) - : CC(cc) + : CC(&cc) , Config(std::move(config)) , LG(lg) , OldStyle(cc.GetEscapeOldStyle()) @@ -56,13 +56,13 @@ cmCustomCommandGenerator::cmCustomCommandGenerator(cmCustomCommand const& cc, { cmGeneratorExpression ge(cc.GetBacktrace()); - const cmCustomCommandLines& cmdlines = this->CC.GetCommandLines(); + const cmCustomCommandLines& cmdlines = this->CC->GetCommandLines(); for (cmCustomCommandLine const& cmdline : cmdlines) { cmCustomCommandLine argv; for (std::string const& clarg : cmdline) { std::unique_ptr cge = ge.Parse(clarg); std::string parsed_arg = cge->Evaluate(this->LG, this->Config); - if (this->CC.GetCommandExpandLists()) { + if (this->CC->GetCommandExpandLists()) { cm::append(argv, cmExpandedList(parsed_arg)); } else { argv.push_back(std::move(parsed_arg)); @@ -113,7 +113,7 @@ cmCustomCommandGenerator::cmCustomCommandGenerator(cmCustomCommand const& cc, this->Byproducts); AppendPaths(cc.GetDepends(), ge, this->LG, this->Config, this->Depends); - const std::string& workingdirectory = this->CC.GetWorkingDirectory(); + const std::string& workingdirectory = this->CC->GetWorkingDirectory(); if (!workingdirectory.empty()) { std::unique_ptr cge = ge.Parse(workingdirectory); @@ -270,7 +270,7 @@ void cmCustomCommandGenerator::AppendArguments(unsigned int c, std::string cmCustomCommandGenerator::GetFullDepfile() const { - std::string depfile = this->CC.GetDepfile(); + std::string depfile = this->CC->GetDepfile(); if (depfile.empty()) { return ""; } @@ -304,7 +304,7 @@ std::string cmCustomCommandGenerator::GetInternalDepfile() const const char* cmCustomCommandGenerator::GetComment() const { - return this->CC.GetComment(); + return this->CC->GetComment(); } std::string cmCustomCommandGenerator::GetWorkingDirectory() const @@ -314,7 +314,7 @@ std::string cmCustomCommandGenerator::GetWorkingDirectory() const std::vector const& cmCustomCommandGenerator::GetOutputs() const { - return this->CC.GetOutputs(); + return this->CC->GetOutputs(); } std::vector const& cmCustomCommandGenerator::GetByproducts() const diff --git a/Source/cmCustomCommandGenerator.h b/Source/cmCustomCommandGenerator.h index 8b5259d..c783b65 100644 --- a/Source/cmCustomCommandGenerator.h +++ b/Source/cmCustomCommandGenerator.h @@ -14,7 +14,7 @@ class cmLocalGenerator; class cmCustomCommandGenerator { - cmCustomCommand const& CC; + cmCustomCommand const* CC; std::string Config; cmLocalGenerator* LG; bool OldStyle; @@ -33,9 +33,11 @@ public: cmCustomCommandGenerator(cmCustomCommand const& cc, std::string config, cmLocalGenerator* lg, bool transformDepfile = true); cmCustomCommandGenerator(const cmCustomCommandGenerator&) = delete; + cmCustomCommandGenerator(cmCustomCommandGenerator&&) = default; cmCustomCommandGenerator& operator=(const cmCustomCommandGenerator&) = delete; - cmCustomCommand const& GetCC() const { return this->CC; } + cmCustomCommandGenerator& operator=(cmCustomCommandGenerator&&) = default; + cmCustomCommand const& GetCC() const { return *(this->CC); } unsigned int GetNumberOfCommands() const; std::string GetCommand(unsigned int c) const; void AppendArguments(unsigned int c, std::string& cmd) const; -- cgit v0.12 From c404f64289bbf93bb7212df913a115e8c0c81e9d Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 27 Oct 2020 11:24:51 -0400 Subject: cmCustomCommandGenerator: Collect genex target references in commands These will become target-level dependencies. --- Source/cmCustomCommandGenerator.cxx | 26 ++++++++++++++++++++++---- Source/cmCustomCommandGenerator.h | 5 +++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/Source/cmCustomCommandGenerator.cxx b/Source/cmCustomCommandGenerator.cxx index 13d2c37..08a0574 100644 --- a/Source/cmCustomCommandGenerator.cxx +++ b/Source/cmCustomCommandGenerator.cxx @@ -62,6 +62,10 @@ cmCustomCommandGenerator::cmCustomCommandGenerator(cmCustomCommand const& cc, for (std::string const& clarg : cmdline) { std::unique_ptr cge = ge.Parse(clarg); std::string parsed_arg = cge->Evaluate(this->LG, this->Config); + for (cmGeneratorTarget* gt : cge->GetTargets()) { + this->Utilities.emplace(BT>( + { gt->GetName(), true }, cge->GetBacktrace())); + } if (this->CC->GetCommandExpandLists()) { cm::append(argv, cmExpandedList(parsed_arg)); } else { @@ -69,10 +73,18 @@ cmCustomCommandGenerator::cmCustomCommandGenerator(cmCustomCommand const& cc, } } - // Later code assumes at least one entry exists, but expanding - // lists on an empty command may have left this empty. - // FIXME: Should we define behavior for removing empty commands? - if (argv.empty()) { + if (!argv.empty()) { + // If the command references an executable target by name, + // collect the target to add a target-level dependency on it. + cmGeneratorTarget* gt = this->LG->FindGeneratorTargetToUse(argv.front()); + if (gt && gt->GetType() == cmStateEnums::EXECUTABLE) { + this->Utilities.emplace(BT>( + { gt->GetName(), true }, cc.GetBacktrace())); + } + } else { + // Later code assumes at least one entry exists, but expanding + // lists on an empty command may have left this empty. + // FIXME: Should we define behavior for removing empty commands? argv.emplace_back(); } @@ -326,3 +338,9 @@ std::vector const& cmCustomCommandGenerator::GetDepends() const { return this->Depends; } + +std::set>> const& +cmCustomCommandGenerator::GetUtilities() const +{ + return this->Utilities; +} diff --git a/Source/cmCustomCommandGenerator.h b/Source/cmCustomCommandGenerator.h index c783b65..cb0d7df 100644 --- a/Source/cmCustomCommandGenerator.h +++ b/Source/cmCustomCommandGenerator.h @@ -4,10 +4,13 @@ #include "cmConfigure.h" // IWYU pragma: keep +#include #include +#include #include #include "cmCustomCommandLines.h" +#include "cmListFileCache.h" class cmCustomCommand; class cmLocalGenerator; @@ -24,6 +27,7 @@ class cmCustomCommandGenerator std::vector Byproducts; std::vector Depends; std::string WorkingDirectory; + std::set>> Utilities; void FillEmulatorsWithArguments(); std::vector GetCrossCompilingEmulator(unsigned int c) const; @@ -46,6 +50,7 @@ public: std::vector const& GetOutputs() const; std::vector const& GetByproducts() const; std::vector const& GetDepends() const; + std::set>> const& GetUtilities() const; bool HasOnlyEmptyCommandLines() const; std::string GetFullDepfile() const; std::string GetInternalDepfile() const; -- cgit v0.12 From 3e36d5e84693691a29561e4212b16b3a1c080673 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 27 Oct 2020 11:27:51 -0400 Subject: cmGeneratorTarget: Refactor custom command dependency evaluation Previously we only used cmCustomCommandGenerator to evaluate generator expressions for dependencies. Use it for command lines too. It also collects target references for us, with backtraces. --- Source/cmGeneratorTarget.cxx | 79 +++++++++----------------------------------- Source/cmTarget.cxx | 5 +++ Source/cmTarget.h | 1 + 3 files changed, 21 insertions(+), 64 deletions(-) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 01d9bec..eb5803e 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -24,9 +24,7 @@ #include "cmAlgorithms.h" #include "cmComputeLinkInformation.h" -#include "cmCustomCommand.h" #include "cmCustomCommandGenerator.h" -#include "cmCustomCommandLines.h" #include "cmFileTimes.h" #include "cmGeneratedFileStream.h" #include "cmGeneratorExpression.h" @@ -2892,9 +2890,6 @@ private: bool IsUtility(std::string const& dep); void CheckCustomCommand(cmCustomCommand const& cc); void CheckCustomCommands(const std::vector& commands); - void FollowCommandDepends(cmCustomCommand const& cc, - const std::string& config, - std::set& emitted); }; cmTargetTraceDependencies::cmTargetTraceDependencies(cmGeneratorTarget* target) @@ -3081,71 +3076,27 @@ bool cmTargetTraceDependencies::IsUtility(std::string const& dep) void cmTargetTraceDependencies::CheckCustomCommand(cmCustomCommand const& cc) { - // Transform command names that reference targets built in this - // project to corresponding target-level dependencies. - cmGeneratorExpression ge(cc.GetBacktrace()); - - // Add target-level dependencies referenced by generator expressions. - std::set targets; - - for (cmCustomCommandLine const& cCmdLine : cc.GetCommandLines()) { - std::string const& command = cCmdLine.front(); - // Check for a target with this name. - if (cmGeneratorTarget* t = - this->LocalGenerator->FindGeneratorTargetToUse(command)) { - if (t->GetType() == cmStateEnums::EXECUTABLE) { - // The command refers to an executable target built in - // this project. Add the target-level dependency to make - // sure the executable is up to date before this custom - // command possibly runs. - this->GeneratorTarget->Target->AddUtility(command, true); - } - } + // Collect dependencies referenced by all configurations. + std::set depends; + for (std::string const& config : + this->Makefile->GetGeneratorConfigs(cmMakefile::IncludeEmptyConfig)) { + cmCustomCommandGenerator ccg(cc, config, this->LocalGenerator); - // Check for target references in generator expressions. - std::vector const& configs = - this->Makefile->GetGeneratorConfigs(cmMakefile::IncludeEmptyConfig); - for (std::string const& c : configs) { - for (std::string const& cl : cCmdLine) { - const std::unique_ptr cge = - ge.Parse(cl); - cge->SetQuiet(true); - cge->Evaluate(this->GeneratorTarget->GetLocalGenerator(), c); - std::set geTargets = cge->GetTargets(); - targets.insert(geTargets.begin(), geTargets.end()); - } + // Collect target-level dependencies referenced in command lines. + for (auto const& util : ccg.GetUtilities()) { + this->GeneratorTarget->Target->AddUtility(util); } - } - for (cmGeneratorTarget* target : targets) { - this->GeneratorTarget->Target->AddUtility(target->GetName(), true); + // Collect file-level dependencies referenced in DEPENDS. + depends.insert(ccg.GetDepends().begin(), ccg.GetDepends().end()); } - // Queue the custom command dependencies. - std::set emitted; - std::vector const& configs = - this->Makefile->GetGeneratorConfigs(cmMakefile::IncludeEmptyConfig); - for (std::string const& conf : configs) { - this->FollowCommandDepends(cc, conf, emitted); - } -} - -void cmTargetTraceDependencies::FollowCommandDepends( - cmCustomCommand const& cc, const std::string& config, - std::set& emitted) -{ - cmCustomCommandGenerator ccg(cc, config, - this->GeneratorTarget->LocalGenerator); - - const std::vector& depends = ccg.GetDepends(); - + // Queue file-level dependencies. for (std::string const& dep : depends) { - if (emitted.insert(dep).second) { - if (!this->IsUtility(dep)) { - // The dependency does not name a target and may be a file we - // know how to generate. Queue it. - this->FollowName(dep); - } + if (!this->IsUtility(dep)) { + // The dependency does not name a target and may be a file we + // know how to generate. Queue it. + this->FollowName(dep); } } } diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 4eebec6..0860b71 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -623,6 +623,11 @@ void cmTarget::AddUtility(std::string const& name, bool cross, cmMakefile* mf) { name, cross }, mf ? mf->GetBacktrace() : cmListFileBacktrace())); } +void cmTarget::AddUtility(BT> util) +{ + impl->Utilities.emplace(std::move(util)); +} + std::set>> const& cmTarget::GetUtilities() const { diff --git a/Source/cmTarget.h b/Source/cmTarget.h index c2a4d86..27f0c59 100644 --- a/Source/cmTarget.h +++ b/Source/cmTarget.h @@ -164,6 +164,7 @@ public: */ void AddUtility(std::string const& name, bool cross, cmMakefile* mf = nullptr); + void AddUtility(BT> util); //! Get the utilities used by this target std::set>> const& GetUtilities() const; -- cgit v0.12