Merge pull request swiftlang#77686 from artemcm/NoInPackageDepsForNonPackageInterface

Ignore in-package transitive dependencies when building from non-package textual interface

Author: artemcm

include/swift/AST/SearchPathOptions.h

@@ -567,6 +567,15 @@ class SearchPathOptions {
   /// dependencies in the scanner itself.
   bool ScannerModuleValidation = false;
 
+  /// Whether this compilation should attempt to resolve in-package
+  /// imports of its module dependencies.
+  ///
+  /// Source compilation and 'package' textual interface compilation both
+  /// require that package-only imports of module dependencies be resolved.
+  /// Otherwise, compilation of non-package textual interfaces, even if
+  /// "in-package", must not require package-only module dependencies.
+  bool ResolveInPackageModuleDependencies = false;
+
   /// Return all module search paths that (non-recursively) contain a file whose
   /// name is in \p Filenames.
   SmallVector<const ModuleSearchPath *, 4>

lib/Frontend/CompilerInvocation.cpp

@@ -2131,6 +2131,7 @@ static bool validateSwiftModuleFileArgumentAndAdd(const std::string &swiftModule
 static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args,
                                 DiagnosticEngine &Diags,
                                 const CASOptions &CASOpts,
+                                const FrontendOptions &FrontendOpts,
                                 StringRef workingDirectory) {
   using namespace options;
   namespace path = llvm::sys::path;
@@ -2299,6 +2300,16 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args,
   Opts.ScannerModuleValidation |= Args.hasFlag(OPT_scanner_module_validation,
                                                OPT_no_scanner_module_validation,
                                                CASOpts.EnableCaching);
+  bool buildingFromInterface =
+      FrontendOptions::doesActionBuildModuleFromInterface(
+          FrontendOpts.RequestedAction);
+  auto firstInputPath =
+      FrontendOpts.InputsAndOutputs.hasInputs()
+          ? FrontendOpts.InputsAndOutputs.getFilenameOfFirstInput()
+          : "";
+  Opts.ResolveInPackageModuleDependencies |=
+      !buildingFromInterface ||
+      StringRef(firstInputPath).ends_with(".package.swiftinterface");
 
   std::optional<std::string> forceModuleLoadingMode;
   if (auto *A = Args.getLastArg(OPT_module_load_mode))
@@ -3759,7 +3770,7 @@ bool CompilerInvocation::parseArgs(
   ParseSymbolGraphArgs(SymbolGraphOpts, ParsedArgs, Diags, LangOpts);
 
   if (ParseSearchPathArgs(SearchPathOpts, ParsedArgs, Diags,
-                          CASOpts, workingDirectory)) {
+                          CASOpts, FrontendOpts, workingDirectory)) {
     return true;
   }
 

lib/Serialization/ModuleFile.cpp

@@ -314,7 +314,8 @@ ModuleFile::getTransitiveLoadingBehavior(const Dependency &dependency,
 
   return Core->getTransitiveLoadingBehavior(
       dependency.Core, ctx.LangOpts.ImportNonPublicDependencies,
-      isPartialModule, ctx.LangOpts.PackageName, forTestable);
+      isPartialModule, ctx.LangOpts.PackageName,
+      ctx.SearchPathOpts.ResolveInPackageModuleDependencies, forTestable);
 }
 
 bool ModuleFile::mayHaveDiagnosticsPointingAtBuffer() const {
@@ -1418,4 +1419,4 @@ StringRef SerializedASTFile::getPublicModuleName() const {
 
 llvm::VersionTuple SerializedASTFile::getSwiftInterfaceCompilerVersion() const {
   return File.getSwiftInterfaceCompilerVersion();
-}
+}

lib/Serialization/ModuleFileSharedCore.cpp

@@ -1853,6 +1853,7 @@ ModuleFileSharedCore::getTransitiveLoadingBehavior(
                                           bool importNonPublicDependencies,
                                           bool isPartialModule,
                                           StringRef packageName,
+                                          bool resolveInPackageModuleDependencies,
                                           bool forTestable) const {
   if (isPartialModule) {
     // Keep the merge-module behavior for legacy support. In that case
@@ -1892,6 +1893,9 @@ ModuleFileSharedCore::getTransitiveLoadingBehavior(
   }
 
   if (dependency.isPackageOnly()) {
+    if (!resolveInPackageModuleDependencies)
+      return ModuleLoadingBehavior::Ignored;
+
     // Package dependencies are usually loaded only for import from the same
     // package.
     if ((!packageName.empty() && packageName == getModulePackageName()) ||

lib/Serialization/ModuleFileSharedCore.h

@@ -681,7 +681,8 @@ class ModuleFileSharedCore {
   /// those non-public dependencies.
   ModuleLoadingBehavior getTransitiveLoadingBehavior(
       const Dependency &dependency, bool importNonPublicDependencies,
-      bool isPartialModule, StringRef packageName, bool forTestable) const;
+      bool isPartialModule, StringRef packageName,
+      bool resolveInPackageModuleDependencies, bool forTestable) const;
 };
 
 template <typename T, typename RawData>

lib/Serialization/SerializedModuleLoader.cpp

@@ -455,7 +455,9 @@ SerializedModuleLoaderBase::getImportsOfModule(
         loadedModuleFile.getTransitiveLoadingBehavior(
             dependency,
             /*importPrivateDependencies*/ false,
-            /*isPartialModule*/ false, packageName, isTestableImport);
+            /*isPartialModule*/ false, packageName,
+            /*resolveInPackageModuleDependencies */ true,
+            isTestableImport);
     if (dependencyTransitiveBehavior > transitiveBehavior)
       continue;
 

test/ModuleInterface/no-package-dependencies-on-non-package-interface.swift

@@ -0,0 +1,59 @@
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/clang-module-cache)
+// RUN: %empty-directory(%t/Modules)
+// RUN: %empty-directory(%t/PackageModules)
+// RUN: %empty-directory(%t/PackageModules/Foo.swiftmodule)
+// RUN: %empty-directory(%t/Modules/Bar.swiftmodule)
+// RUN: %empty-directory(%t/TextualInterfaces)
+// RUN: %empty-directory(%t/TextualInterfaces/Bar.swiftmodule)
+// RUN: split-file %s %t
+
+// Step 1: Build Foo Swift binary module
+// RUN: %target-swift-frontend -emit-module %t/Foo.swift -emit-module-path %t/PackageModules/Foo.swiftmodule/%target-swiftmodule-name -module-name Foo
+
+// Step 2: Build Bar Swift binary module and textual interface with a package-only import
+// RUN: %target-swift-frontend -emit-module %t/Bar.swift -emit-module-path %t/Modules/Bar.swiftmodule/%target-swiftmodule-name -module-name Bar -enable-library-evolution -emit-module-interface-path %t/TextualInterfaces/Bar.swiftmodule/%target-swiftinterface-name -emit-private-module-interface-path %t/TextualInterfaces/Bar.swiftmodule/%target-private-swiftinterface-name -emit-package-module-interface-path %t/TextualInterfaces/Bar.swiftmodule/%target-package-swiftinterface-name -I %t/PackageModules/ -package-name BarTest
+
+// Step 3: Now that Bar has been built, remove package-only dependency 'Foo' so that any clients of 'Bar' fail to build if they search for it
+// RUN: rm -rf %t/PackageModules/*
+
+
+// Test 1: Build a textual interface client which imports Bar and is in Bar's package but not being built from a package interface, therefore it must not import Bar's package-only dependencies
+// RUN: %target-swift-frontend -compile-module-from-interface -explicit-interface-module-build %t/Client.swiftinterface -o %t/Modules/Client.swiftmodule -module-name Client -I %t/Modules/ -package-name BarTest 
+
+// Test 2: Build a textual interface client which imports Bar and is in Bar's package and is being built from a package interface, therefore it must import Bar's package-only dependencies
+// RUN: not %target-swift-frontend -compile-module-from-interface -explicit-interface-module-build %t/Client.package.swiftinterface -o %t/Modules/Client.package.swiftmodule -module-name Client -I %t/Modules/ -package-name BarTest &> %t/error.txt
+// RUN %FileCheck --check-prefix=CHECK-MISSING-FOO %s < %t/error.txt
+
+// Test 3: Build a source client which imports Bar but is not in Bar's package, therefore it must not import Bar's package-only dependencies
+// RUN: %target-swift-frontend -emit-module %t/Client.swift -emit-module-path %t/Modules/SourceClient.swiftmodule/%target-swiftmodule-name -module-name Client -I %t/Modules/
+
+// Test 4: Build a source client which imports Bar but and is in Bar's package, therefore it must import Bar's package-only dependencies
+// RUN: not %target-swift-frontend -emit-module %t/Client.swift -emit-module-path %t/Modules/SourceClient.swiftmodule/%target-swiftmodule-name -module-name Client -I %t/Modules/ -package-name BarTest &> %t/source_error.txt
+// RUN %FileCheck --check-prefix=CHECK-MISSING-FOO %s < %t/source_error.txt
+
+// CHECK-MISSING-FOO: error: missing required module 'Foo'
+
+//--- Foo.swift
+public func foo() {}
+
+//--- Bar.swift
+package import Foo
+
+//--- Client.swiftinterface
+// swift-interface-format-version: 1.0
+// swift-module-flags: -swift-version 5 -enable-library-evolution -module-name Client
+import Bar
+public func test() {}
+
+//--- Client.package.swiftinterface
+// swift-interface-format-version: 1.0
+// swift-module-flags: -swift-version 5 -enable-library-evolution -module-name Client -package-name BarTest
+import Bar
+public func test() {}
+
+//--- Client.swift
+import Bar
+
+
+

test/lit.cfg

@@ -2936,6 +2936,7 @@ config.substitutions.append(('%target-swiftdoc-name', target_specific_module_tri
 config.substitutions.append(('%target-swiftsourceinfo-name', target_specific_module_triple + '.swiftsourceinfo'))
 config.substitutions.append(('%target-swiftinterface-name', target_specific_module_triple + '.swiftinterface'))
 config.substitutions.append(('%target-private-swiftinterface-name', target_specific_module_triple + '.private.swiftinterface'))
+config.substitutions.append(('%target-package-swiftinterface-name', target_specific_module_triple + '.package.swiftinterface'))
 
 config.substitutions.append(('%target-object-format', config.target_object_format))
 config.substitutions.append(('%{target-shared-library-prefix}', config.target_shared_library_prefix))