Merge topic 'swiftmodule-dependency-tracking'

1730d208b5 Add incremental Swift static lib build test
bf3a8ef6d5 Ninja: Swift: Add dependency edge to swiftmodule file
d0b469b7e0 Ninja: NFC: refactor swift module name computations

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: buildbot <buildbot@kitware.com>
Merge-request: !8084

6 files changed, 101 insertions, 25 deletions

diff --git a/Source/cmNinjaNormalTargetGenerator.cxx b/Source/cmNinjaNormalTargetGenerator.cxx
index d481b64..a1633ca 100644
--- a/Source/cmNinjaNormalTargetGenerator.cxx
+++ b/Source/cmNinjaNormalTargetGenerator.cxx
@@ -940,6 +940,37 @@ void cmNinjaNormalTargetGenerator::WriteNvidiaDeviceLinkStatement(
   this->WriteNvidiaDeviceLinkRule(usedResponseFile, config);
 }
 
+/// Get the target property if it exists, or return a default
+static std::string GetTargetPropertyOrDefault(cmGeneratorTarget const* target,
+                                              std::string const& property,
+                                              std::string defaultValue)
+{
+  if (cmValue name = target->GetProperty(property)) {
+    return *name;
+  }
+  return defaultValue;
+}
+
+/// Compute the swift module name for target
+static std::string GetSwiftModuleName(cmGeneratorTarget const* target)
+{
+  return GetTargetPropertyOrDefault(target, "Swift_MODULE_NAME",
+                                    target->GetName());
+}
+
+/// Compute the swift module path for the target
+/// The returned path will need to be converted to the generator path
+static std::string GetSwiftModulePath(cmGeneratorTarget const* target)
+{
+  std::string moduleName = GetSwiftModuleName(target);
+  std::string moduleDirectory = GetTargetPropertyOrDefault(
+    target, "Swift_MODULE_DIRECTORY",
+    target->LocalGenerator->GetCurrentBinaryDirectory());
+  std::string moduleFileName = GetTargetPropertyOrDefault(
+    target, "Swift_MODULE", moduleName + ".swiftmodule");
+  return moduleDirectory + "/" + moduleFileName;
+}
+
 void cmNinjaNormalTargetGenerator::WriteLinkStatement(
   const std::string& config, const std::string& fileConfig,
   bool firstForConfig)
@@ -1038,31 +1069,10 @@ void cmNinjaNormalTargetGenerator::WriteLinkStatement(
       return targetNames.Base;
     }();
 
-    vars["SWIFT_MODULE_NAME"] = [gt]() -> std::string {
-      if (cmValue name = gt->GetProperty("Swift_MODULE_NAME")) {
-        return *name;
-      }
-      return gt->GetName();
-    }();
-
-    vars["SWIFT_MODULE"] = [this](const std::string& module) -> std::string {
-      std::string directory =
-        this->GetLocalGenerator()->GetCurrentBinaryDirectory();
-      if (cmValue prop = this->GetGeneratorTarget()->GetProperty(
-            "Swift_MODULE_DIRECTORY")) {
-        directory = *prop;
-      }
-
-      std::string name = module + ".swiftmodule";
-      if (cmValue prop =
-            this->GetGeneratorTarget()->GetProperty("Swift_MODULE")) {
-        name = *prop;
-      }
-
-      return this->GetLocalGenerator()->ConvertToOutputFormat(
-        this->ConvertToNinjaPath(directory + "/" + name),
-        cmOutputConverter::SHELL);
-    }(vars["SWIFT_MODULE_NAME"]);
+    vars["SWIFT_MODULE_NAME"] = GetSwiftModuleName(gt);
+    vars["SWIFT_MODULE"] = this->GetLocalGenerator()->ConvertToOutputFormat(
+      this->ConvertToNinjaPath(GetSwiftModulePath(gt)),
+      cmOutputConverter::SHELL);
 
     vars["SWIFT_SOURCES"] = [this, config]() -> std::string {
       std::vector<cmSourceFile const*> sources;
@@ -1386,6 +1396,23 @@ void cmNinjaNormalTargetGenerator::WriteLinkStatement(
     }
   }
 
+  // Add dependencies on swiftmodule files when using the swift linker
+  if (this->TargetLinkLanguage(config) == "Swift") {
+    if (cmComputeLinkInformation* cli =
+          this->GeneratorTarget->GetLinkInformation(config)) {
+      for (auto const& dependency : cli->GetItems()) {
+        // Both the current target and the linked target must be swift targets
+        // in order for there to be a swiftmodule to depend on
+        if (dependency.Target &&
+            dependency.Target->GetLinkerLanguage(config) == "Swift") {
+          std::string swiftmodule =
+            this->ConvertToNinjaPath(GetSwiftModulePath(dependency.Target));
+          linkBuild.ImplicitDeps.emplace_back(swiftmodule);
+        }
+      }
+    }
+  }
+
   // Ninja should restat after linking if and only if there are byproducts.
   vars["RESTAT"] = byproducts.ExplicitOuts.empty() ? "" : "1";
 
diff --git a/Tests/RunCMake/Swift/IncrementalSwift-second-result.txt b/Tests/RunCMake/Swift/IncrementalSwift-second-result.txt
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/Tests/RunCMake/Swift/IncrementalSwift-second-result.txt
@@ -0,0 +1 @@
+1
diff --git a/Tests/RunCMake/Swift/IncrementalSwift-second-stderr.txt b/Tests/RunCMake/Swift/IncrementalSwift-second-stderr.txt
new file mode 100644
index 0000000..7a882f8
--- /dev/null
+++ b/Tests/RunCMake/Swift/IncrementalSwift-second-stderr.txt
@@ -0,0 +1,2 @@
+ninja explain: A.swiftmodule is dirty
+ninja explain: libB.a is dirty
diff --git a/Tests/RunCMake/Swift/IncrementalSwift-second-stdout.txt b/Tests/RunCMake/Swift/IncrementalSwift-second-stdout.txt
new file mode 100644
index 0000000..bb08a49
--- /dev/null
+++ b/Tests/RunCMake/Swift/IncrementalSwift-second-stdout.txt
@@ -0,0 +1,3 @@
+.*Linking Swift static library libA.a
+.*Linking Swift static library libB.a
+FAILED: libB.a CMakeFiles/B.dir/b.swift.o B.swiftmodule
diff --git a/Tests/RunCMake/Swift/IncrementalSwift.cmake b/Tests/RunCMake/Swift/IncrementalSwift.cmake
new file mode 100644
index 0000000..092269f
--- /dev/null
+++ b/Tests/RunCMake/Swift/IncrementalSwift.cmake
@@ -0,0 +1,22 @@
+enable_language(Swift)
+
+# Write initial files to build directory
+# The files are generated into the build directory to avoid dirtying the source
+# directory. This is done because the source files are changed during the test
+# to ensure correct incremental build behavior.
+file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/a.swift
+  "let number: Int = 32\n"
+  "public func callA() -> Int { return number }\n")
+file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/b.swift
+  "import A\n"
+  "public func callB() -> Int { return callA() + 1 }\n")
+file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/exec.swift
+  "import B\n"
+  "print(callB())\n")
+
+add_library(A STATIC ${CMAKE_CURRENT_BINARY_DIR}/a.swift)
+add_library(B STATIC ${CMAKE_CURRENT_BINARY_DIR}/b.swift)
+target_link_libraries(B PRIVATE A)
+
+add_executable(exec ${CMAKE_CURRENT_BINARY_DIR}/exec.swift)
+target_link_libraries(exec PRIVATE B)
diff --git a/Tests/RunCMake/Swift/RunCMakeTest.cmake b/Tests/RunCMake/Swift/RunCMakeTest.cmake
index 7f4464d..5537c01 100644
--- a/Tests/RunCMake/Swift/RunCMakeTest.cmake
+++ b/Tests/RunCMake/Swift/RunCMakeTest.cmake
@@ -24,6 +24,27 @@ elseif(RunCMake_GENERATOR STREQUAL Ninja)
       run_cmake_command(NoWorkToDo-build ${CMAKE_COMMAND} --build .)
       run_cmake_command(NoWorkToDo-nowork ${CMAKE_COMMAND} --build . -- -d explain)
     endblock()
+
+    # Test that intermediate static libraries are rebuilt when the public
+    # interface of their dependency changes
+    block()
+      set(IncrementalSwift_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/IncrementalSwift-build)
+      set(IncrementalSwift_TEST_NO_CLEAN 1)
+      set(IncrementalSwift_TEST_OUTPUT_MERGE 1)
+      # Since files are modified during test, the files are created in the cmake
+      # file into the build directory
+      run_cmake(IncrementalSwift)
+      run_cmake_command(IncrementalSwift-first ${CMAKE_COMMAND} --build ${IncrementalSwift_TEST_BINARY_DIR})
+
+      # Modify public interface of libA requiring rebuild of libB
+      file(WRITE ${IncrementalSwift_TEST_BINARY_DIR}/a.swift
+        "public func callA() -> Float { return 32.0 }\n")
+
+      # Note: We still expect this to fail, but instead of failure at link time,
+      # it should fail while re-compiling libB because the function changed
+      run_cmake_command(IncrementalSwift-second ${CMAKE_COMMAND} --build ${IncrementalSwift_TEST_BINARY_DIR} -- -d explain)
+    endblock()
+
   endif()
 elseif(RunCMake_GENERATOR STREQUAL "Ninja Multi-Config")
   if(CMAKE_Swift_COMPILER)