Browse Source

cmMakefile: Extract utilities used for creation of custom commands

Decompose creation of custom commands further into logical steps.
Daniel Eiband 6 years ago
parent
commit
ea1bed34b2
3 changed files with 192 additions and 130 deletions
  1. 9 0
      Source/cmCustomCommandTypes.h
  2. 144 117
      Source/cmMakefile.cxx
  3. 39 13
      Source/cmMakefile.h

+ 9 - 0
Source/cmCustomCommandTypes.h

@@ -5,6 +5,8 @@
 
 #include "cmConfigure.h" // IWYU pragma: keep
 
+#include <string>
+
 /** Target custom command type */
 enum class cmCustomCommandType
 {
@@ -27,4 +29,11 @@ enum class cmObjectLibraryCommands
   Accept
 };
 
+/** Utility target output source file name.  */
+struct cmUtilityOutput
+{
+  std::string Name;
+  std::string NameCMP0049;
+};
+
 #endif

+ 144 - 117
Source/cmMakefile.cxx

@@ -838,13 +838,8 @@ bool cmMakefile::ValidateCustomCommand(
   return true;
 }
 
-cmTarget* cmMakefile::AddCustomCommandToTarget(
-  const std::string& target, const std::vector<std::string>& byproducts,
-  const std::vector<std::string>& depends,
-  const cmCustomCommandLines& commandLines, cmCustomCommandType type,
-  const char* comment, const char* workingDir, bool escapeOldStyle,
-  bool uses_terminal, const std::string& depfile, const std::string& job_pool,
-  bool command_expand_lists, cmObjectLibraryCommands objLibCommands)
+cmTarget* cmMakefile::GetCustomCommandTarget(
+  const std::string& target, cmObjectLibraryCommands objLibCommands) const
 {
   // Find the target to which to add the custom command.
   auto ti = this->Targets.find(target);
@@ -903,8 +898,21 @@ cmTarget* cmMakefile::AddCustomCommandToTarget(
     return nullptr;
   }
 
+  return t;
+}
+
+cmTarget* cmMakefile::AddCustomCommandToTarget(
+  const std::string& target, const std::vector<std::string>& byproducts,
+  const std::vector<std::string>& depends,
+  const cmCustomCommandLines& commandLines, cmCustomCommandType type,
+  const char* comment, const char* workingDir, bool escapeOldStyle,
+  bool uses_terminal, const std::string& depfile, const std::string& job_pool,
+  bool command_expand_lists, cmObjectLibraryCommands objLibCommands)
+{
+  cmTarget* t = this->GetCustomCommandTarget(target, objLibCommands);
+
   // Validate custom commands.
-  if (!this->ValidateCustomCommand(commandLines)) {
+  if (!t || !this->ValidateCustomCommand(commandLines)) {
     return t;
   }
 
@@ -947,37 +955,8 @@ void cmMakefile::CommitCustomCommandToTarget(
       target->AddPostBuildCommand(std::move(cc));
       break;
   }
-  this->UpdateOutputToSourceMap(byproducts, target);
-}
 
-void cmMakefile::UpdateOutputToSourceMap(
-  std::vector<std::string> const& byproducts, cmTarget* target)
-{
-  for (std::string const& o : byproducts) {
-    this->UpdateOutputToSourceMap(o, target);
-  }
-}
-
-void cmMakefile::UpdateOutputToSourceMap(std::string const& byproduct,
-                                         cmTarget* target)
-{
-  SourceEntry entry;
-  entry.Sources.Target = target;
-
-  auto pr = this->OutputToSource.emplace(byproduct, entry);
-  if (!pr.second) {
-    SourceEntry& current = pr.first->second;
-    // Has the target already been set?
-    if (!current.Sources.Target) {
-      current.Sources.Target = target;
-    } else {
-      // Multiple custom commands/targets produce the same output (source file
-      // or target).  See also comment in other UpdateOutputToSourceMap
-      // overload.
-      //
-      // TODO: Warn the user about this case.
-    }
-  }
+  this->AddTargetByproducts(target, byproducts);
 }
 
 cmSourceFile* cmMakefile::AddCustomCommandToOutput(
@@ -1103,47 +1082,10 @@ cmSourceFile* cmMakefile::CommitCustomCommandToOutput(
     cc->SetDepfile(depfile);
     cc->SetJobPool(job_pool);
     file->SetCustomCommand(std::move(cc));
-    this->UpdateOutputToSourceMap(outputs, file, false);
-    this->UpdateOutputToSourceMap(byproducts, file, true);
-  }
-  return file;
-}
 
-void cmMakefile::UpdateOutputToSourceMap(
-  std::vector<std::string> const& outputs, cmSourceFile* source,
-  bool byproduct)
-{
-  for (std::string const& o : outputs) {
-    this->UpdateOutputToSourceMap(o, source, byproduct);
-  }
-}
-
-void cmMakefile::UpdateOutputToSourceMap(std::string const& output,
-                                         cmSourceFile* source, bool byproduct)
-{
-  SourceEntry entry;
-  entry.Sources.Source = source;
-  entry.Sources.SourceIsByproduct = byproduct;
-
-  auto pr = this->OutputToSource.emplace(output, entry);
-  if (!pr.second) {
-    SourceEntry& current = pr.first->second;
-    // Outputs take precedence over byproducts
-    if (!current.Sources.Source ||
-        (current.Sources.SourceIsByproduct && !byproduct)) {
-      current.Sources.Source = source;
-      current.Sources.SourceIsByproduct = false;
-    } else {
-      // Multiple custom commands produce the same output but may
-      // be attached to a different source file (MAIN_DEPENDENCY).
-      // LinearGetSourceFileWithOutput would return the first one,
-      // so keep the mapping for the first one.
-      //
-      // TODO: Warn the user about this case.  However, the VS 8 generator
-      // triggers it for separate generate.stamp rules in ZERO_CHECK and
-      // individual targets.
-    }
+    this->AddSourceOutputs(file, outputs, byproducts);
   }
+  return file;
 }
 
 void cmMakefile::AddCustomCommandOldStyle(
@@ -1248,6 +1190,27 @@ void cmMakefile::CommitAppendCustomCommandToOutput(
   }
 }
 
+cmUtilityOutput cmMakefile::GetUtilityOutput(cmTarget* target)
+{
+  std::string force = cmStrCat(this->GetCurrentBinaryDirectory(),
+                               "/CMakeFiles/", target->GetName());
+  std::string forceCMP0049 = target->GetSourceCMP0049(force);
+  {
+    cmSourceFile* sf = nullptr;
+    if (!forceCMP0049.empty()) {
+      sf = this->GetOrCreateSource(forceCMP0049, false,
+                                   cmSourceFileLocationKind::Known);
+    }
+    // The output is not actually created so mark it symbolic.
+    if (sf) {
+      sf->SetProperty("SYMBOLIC", "1");
+    } else {
+      cmSystemTools::Error("Could not get source file entry for " + force);
+    }
+  }
+  return { std::move(force), std::move(forceCMP0049) };
+}
+
 cmTarget* cmMakefile::AddUtilityCommand(
   const std::string& utilityName, cmCommandOrigin origin, bool excludeFromAll,
   const char* workingDirectory, const std::vector<std::string>& depends,
@@ -1270,12 +1233,8 @@ cmTarget* cmMakefile::AddUtilityCommand(
   const char* comment, bool uses_terminal, bool command_expand_lists,
   const std::string& job_pool)
 {
-  // Create a target instance for this utility.
-  cmTarget* target = this->AddNewTarget(cmStateEnums::UTILITY, utilityName);
-  target->SetIsGeneratorProvided(origin == cmCommandOrigin::Generator);
-  if (excludeFromAll || this->GetPropertyAsBool("EXCLUDE_FROM_ALL")) {
-    target->SetProperty("EXCLUDE_FROM_ALL", "TRUE");
-  }
+  cmTarget* target =
+    this->AddNewUtilityTarget(utilityName, origin, excludeFromAll);
 
   // Validate custom commands.
   if (!this->ValidateCustomCommand(commandLines) ||
@@ -1283,50 +1242,35 @@ cmTarget* cmMakefile::AddUtilityCommand(
     return target;
   }
 
+  // Get the output name of the utility target and mark it generated.
+  cmUtilityOutput force = this->GetUtilityOutput(target);
+  this->GetOrCreateGeneratedSource(force.Name);
+
   // Always create the byproduct sources and mark them generated.
   this->CreateGeneratedSources(byproducts);
 
-  std::string force =
-    cmStrCat(this->GetCurrentBinaryDirectory(), "/CMakeFiles/", utilityName);
-  this->CreateGeneratedSource(force);
-  std::string forceCMP0049 = target->GetSourceCMP0049(force);
-  {
-    cmSourceFile* sf = nullptr;
-    if (!forceCMP0049.empty()) {
-      sf = this->GetOrCreateSource(forceCMP0049, false,
-                                   cmSourceFileLocationKind::Known);
-    }
-    // The output is not actually created so mark it symbolic.
-    if (sf) {
-      sf->SetProperty("SYMBOLIC", "1");
-    } else {
-      cmSystemTools::Error("Could not get source file entry for " + force);
-    }
-  }
-
   if (!comment) {
     // Use an empty comment to avoid generation of default comment.
     comment = "";
   }
 
-  this->CommitUtilityCommand(target, force, forceCMP0049, workingDirectory,
-                             byproducts, depends, commandLines, escapeOldStyle,
-                             comment, uses_terminal, command_expand_lists,
-                             job_pool);
+  this->CommitUtilityCommand(target, force, workingDirectory, byproducts,
+                             depends, commandLines, escapeOldStyle, comment,
+                             uses_terminal, command_expand_lists, job_pool);
 
   return target;
 }
 
 void cmMakefile::CommitUtilityCommand(
-  cmTarget* target, const std::string& force, const std::string& forceCMP0049,
-  const char* workingDirectory, const std::vector<std::string>& byproducts,
+  cmTarget* target, const cmUtilityOutput& force, const char* workingDirectory,
+  const std::vector<std::string>& byproducts,
   const std::vector<std::string>& depends,
   const cmCustomCommandLines& commandLines, bool escapeOldStyle,
   const char* comment, bool uses_terminal, bool command_expand_lists,
   const std::string& job_pool)
 {
   std::vector<std::string> forced;
-  forced.push_back(force);
+  forced.push_back(force.Name);
   std::string no_main_dependency;
   cmImplicitDependsList no_implicit_depends;
   bool no_replace = false;
@@ -1334,11 +1278,11 @@ void cmMakefile::CommitUtilityCommand(
     forced, byproducts, depends, no_main_dependency, no_implicit_depends,
     commandLines, comment, workingDirectory, no_replace, escapeOldStyle,
     uses_terminal, command_expand_lists, /*depfile=*/"", job_pool);
-  if (!forceCMP0049.empty()) {
-    target->AddSource(forceCMP0049);
+  if (!force.NameCMP0049.empty()) {
+    target->AddSource(force.NameCMP0049);
   }
   if (sf) {
-    this->UpdateOutputToSourceMap(byproducts, target);
+    this->AddTargetByproducts(target, byproducts);
   }
 }
 
@@ -2157,6 +2101,18 @@ cmTarget* cmMakefile::AddNewTarget(cmStateEnums::TargetType type,
   return &it->second;
 }
 
+cmTarget* cmMakefile::AddNewUtilityTarget(const std::string& utilityName,
+                                          cmCommandOrigin origin,
+                                          bool excludeFromAll)
+{
+  cmTarget* target = this->AddNewTarget(cmStateEnums::UTILITY, utilityName);
+  target->SetIsGeneratorProvided(origin == cmCommandOrigin::Generator);
+  if (excludeFromAll || this->GetPropertyAsBool("EXCLUDE_FROM_ALL")) {
+    target->SetProperty("EXCLUDE_FROM_ALL", "TRUE");
+  }
+  return target;
+}
+
 namespace {
 bool AnyOutputMatches(const std::string& name,
                       const std::vector<std::string>& outputs)
@@ -2287,6 +2243,76 @@ bool cmMakefile::MightHaveCustomCommand(const std::string& name) const
   return false;
 }
 
+void cmMakefile::AddTargetByproducts(
+  cmTarget* target, const std::vector<std::string>& byproducts)
+{
+  for (std::string const& o : byproducts) {
+    this->UpdateOutputToSourceMap(o, target);
+  }
+}
+
+void cmMakefile::AddSourceOutputs(cmSourceFile* source,
+                                  const std::vector<std::string>& outputs,
+                                  const std::vector<std::string>& byproducts)
+{
+  for (std::string const& o : outputs) {
+    this->UpdateOutputToSourceMap(o, source, false);
+  }
+  for (std::string const& o : byproducts) {
+    this->UpdateOutputToSourceMap(o, source, true);
+  }
+}
+
+void cmMakefile::UpdateOutputToSourceMap(std::string const& byproduct,
+                                         cmTarget* target)
+{
+  SourceEntry entry;
+  entry.Sources.Target = target;
+
+  auto pr = this->OutputToSource.emplace(byproduct, entry);
+  if (!pr.second) {
+    SourceEntry& current = pr.first->second;
+    // Has the target already been set?
+    if (!current.Sources.Target) {
+      current.Sources.Target = target;
+    } else {
+      // Multiple custom commands/targets produce the same output (source file
+      // or target).  See also comment in other UpdateOutputToSourceMap
+      // overload.
+      //
+      // TODO: Warn the user about this case.
+    }
+  }
+}
+
+void cmMakefile::UpdateOutputToSourceMap(std::string const& output,
+                                         cmSourceFile* source, bool byproduct)
+{
+  SourceEntry entry;
+  entry.Sources.Source = source;
+  entry.Sources.SourceIsByproduct = byproduct;
+
+  auto pr = this->OutputToSource.emplace(output, entry);
+  if (!pr.second) {
+    SourceEntry& current = pr.first->second;
+    // Outputs take precedence over byproducts
+    if (!current.Sources.Source ||
+        (current.Sources.SourceIsByproduct && !byproduct)) {
+      current.Sources.Source = source;
+      current.Sources.SourceIsByproduct = false;
+    } else {
+      // Multiple custom commands produce the same output but may
+      // be attached to a different source file (MAIN_DEPENDENCY).
+      // LinearGetSourceFileWithOutput would return the first one,
+      // so keep the mapping for the first one.
+      //
+      // TODO: Warn the user about this case.  However, the VS 8 generator
+      // triggers it for separate generate.stamp rules in ZERO_CHECK and
+      // individual targets.
+    }
+  }
+}
+
 #if !defined(CMAKE_BOOTSTRAP)
 cmSourceGroup* cmMakefile::GetSourceGroup(
   const std::vector<std::string>& name) const
@@ -3531,19 +3557,20 @@ cmSourceFile* cmMakefile::GetOrCreateSource(const std::string& sourceName,
   return this->CreateSource(sourceName, generated, kind);
 }
 
-void cmMakefile::CreateGeneratedSource(const std::string& output)
+cmSourceFile* cmMakefile::GetOrCreateGeneratedSource(
+  const std::string& sourceName)
 {
-  if (cmSourceFile* out = this->GetOrCreateSource(
-        output, true, cmSourceFileLocationKind::Known)) {
-    out->SetProperty("GENERATED", "1");
-  }
+  cmSourceFile* sf =
+    this->GetOrCreateSource(sourceName, true, cmSourceFileLocationKind::Known);
+  sf->SetProperty("GENERATED", "1");
+  return sf;
 }
 
 void cmMakefile::CreateGeneratedSources(
   const std::vector<std::string>& outputs)
 {
   for (std::string const& output : outputs) {
-    this->CreateGeneratedSource(output);
+    this->GetOrCreateGeneratedSource(output);
   }
 }
 

+ 39 - 13
Source/cmMakefile.h

@@ -168,6 +168,12 @@ public:
    */
   void FinalPass();
 
+  /**
+   * Get the target for PRE_BUILD, PRE_LINK, or POST_BUILD commands.
+   */
+  cmTarget* GetCustomCommandTarget(
+    const std::string& target, cmObjectLibraryCommands objLibCommands) const;
+
   /** Add a custom command to the build.  */
   cmTarget* AddCustomCommandToTarget(
     const std::string& target, const std::vector<std::string>& byproducts,
@@ -205,6 +211,19 @@ public:
     const cmImplicitDependsList& implicit_depends,
     const cmCustomCommandLines& commandLines);
 
+  /**
+   * Add target byproducts.
+   */
+  void AddTargetByproducts(cmTarget* target,
+                           const std::vector<std::string>& byproducts);
+
+  /**
+   * Add source file outputs.
+   */
+  void AddSourceOutputs(cmSourceFile* source,
+                        const std::vector<std::string>& outputs,
+                        const std::vector<std::string>& byproducts);
+
   /**
    * Add a define flag to the build.
    */
@@ -222,6 +241,10 @@ public:
   cmTarget* AddNewTarget(cmStateEnums::TargetType type,
                          const std::string& name);
 
+  /** Create a target instance for the utility.  */
+  cmTarget* AddNewUtilityTarget(const std::string& utilityName,
+                                cmCommandOrigin origin, bool excludeFromAll);
+
   /**
    * Add an executable to the build.
    */
@@ -229,6 +252,11 @@ public:
                           const std::vector<std::string>& srcs,
                           bool excludeFromAll = false);
 
+  /**
+   * Return the utility target output source file name and the CMP0049 name.
+   */
+  cmUtilityOutput GetUtilityOutput(cmTarget* target);
+
   /**
    * Add a utility to the build.  A utility target is a command that
    * is run every time the target is built.
@@ -447,6 +475,12 @@ public:
     const std::string& sourceName, bool generated = false,
     cmSourceFileLocationKind kind = cmSourceFileLocationKind::Ambiguous);
 
+  /** Get a cmSourceFile pointer for a given source name and always mark the
+   * file as generated, if the name is not found, then create the source file
+   * and return it.
+   */
+  cmSourceFile* GetOrCreateGeneratedSource(const std::string& sourceName);
+
   void AddTargetObject(std::string const& tgtName, std::string const& objFile);
 
   /**
@@ -1028,13 +1062,12 @@ private:
   friend bool cmCMakePolicyCommand(std::vector<std::string> const& args,
                                    cmExecutionStatus& status);
   class IncludeScope;
-
   friend class IncludeScope;
-  class ListFileScope;
 
+  class ListFileScope;
   friend class ListFileScope;
-  class BuildsystemFileScope;
 
+  class BuildsystemFileScope;
   friend class BuildsystemFileScope;
 
   // CMP0053 == old
@@ -1053,6 +1086,8 @@ private:
 
   bool ValidateCustomCommand(const cmCustomCommandLines& commandLines) const;
 
+  void CreateGeneratedSources(const std::vector<std::string>& outputs);
+
   void CommitCustomCommandToTarget(
     cmTarget* target, const std::vector<std::string>& byproducts,
     const std::vector<std::string>& depends,
@@ -1075,8 +1110,7 @@ private:
     const cmImplicitDependsList& implicit_depends,
     const cmCustomCommandLines& commandLines);
 
-  void CommitUtilityCommand(cmTarget* target, const std::string& force,
-                            const std::string& forceCMP0049,
+  void CommitUtilityCommand(cmTarget* target, const cmUtilityOutput& force,
                             const char* workingDirectory,
                             const std::vector<std::string>& byproducts,
                             const std::vector<std::string>& depends,
@@ -1085,9 +1119,6 @@ private:
                             bool uses_terminal, bool command_expand_lists,
                             const std::string& job_pool);
 
-  void CreateGeneratedSource(const std::string& output);
-  void CreateGeneratedSources(const std::vector<std::string>& outputs);
-
   /**
    * See LinearGetSourceFileWithOutput for background information
    */
@@ -1112,12 +1143,7 @@ private:
   using OutputToSourceMap = std::unordered_map<std::string, SourceEntry>;
   OutputToSourceMap OutputToSource;
 
-  void UpdateOutputToSourceMap(std::vector<std::string> const& byproducts,
-                               cmTarget* target);
   void UpdateOutputToSourceMap(std::string const& byproduct, cmTarget* target);
-
-  void UpdateOutputToSourceMap(std::vector<std::string> const& outputs,
-                               cmSourceFile* source, bool byproduct);
   void UpdateOutputToSourceMap(std::string const& output, cmSourceFile* source,
                                bool byproduct);