[Dependency Scanner] Add a scanner for explicit placeholder module dependencies #33032

artemcm · 2020-07-21T19:19:10Z

This PR adds support for the dependency scanner to accept a set of modules that are treated as placeholder dependencies.

Placeholder dependencies are swift modules that the scanner will not be able to locate in its search paths and which are the responsibility of the scanner's client to ensure are provided. Today, encountering an import of a module that cannot be discovered in a search path is a failure.

The motivation for this change comes from building SwiftPM packages with explicit module builds. Building a SwiftPM package with multiple inter-dependent targets requires that each target’s build job must have its target dependencies’ modules passed into it as explicit module dependencies. Because the build planning stage occurs before any of the targets have been built, each target's module scanning action cannot be made aware of other targets. To support this, SwiftPM must explicitly pass down target dependencies to each target's build planning action (by the driver), which will forward said dependencies to the scanner. Upon encountering an import statement of a module known to be a placeholder dependency, instead of searching for the module and discovering its dependencies, the scanner will discover it in an explicit placeholder-dependency-module-map JSON file and note its presence and reflect it accordingly in the resulting dependency graph.

This PR:

Adds an option: -placeholder-dependency-module-map-file that accepts a module map of placeholder module dependencies of the same format as -explicit-swift-module-map-file.
Refactors parsing of the module map JSON files into a standalone utility used by explicit-swift-module-map code and placeholder-dependency-module-map code.
Adds a new dependency kind (ModuleDependencyKind case: .swiftPlaceholder) to model placeholder module dependencies.
Adds PlaceholderSwiftModuleStubScanner used by the dependency scanning action to discover this new kind of dependencies.

nkcsgexi

The change makes sense to me. Can we rename external dependencies to placeholder dependencies? Because what makes these modules different is that they are not ready when the scanning happens.

artemcm · 2020-07-21T19:48:35Z

The change makes sense to me. Can we rename external dependencies to placeholder dependencies? Because what makes these modules different is that they are not ready when the scanning happens.

Yeah, I think placeholder is a better name.

nkcsgexi · 2020-07-21T20:38:56Z

test/ScanDependencies/Inputs/ModuleDependencyGraph.swift

-      case .swift(let name): return name
-      case .clang(let name): return name
+    case .swift(let name): return name
+    case .swiftExternal(let name): return name


Another question: why does the client side need to know whether a module is a placeholder dependency? can we blend them in other module dependencies in the JSON output?

I wrestled with doing this a little bit, here is my rationale for making it a distinct dependency kind:

Using swift-driver as an example, when the scanner produces a dependency graph for the swift-driver, it is not enough to have the placeholder just be another swift module in it. One of the things the client will also need (and will be supplied by SwiftPM in this case), is a full dependency graph of the placeholder itself, to get its transitive dependencies and to unique all such transitive dependencies with the modules already in the dependency graph of the main module. Therefore, once the scanner produces this graph, the client will go through all placeholders and substitute their leaf nodes in this graph with a full dependency tree, replacing placeholders with "actual" dependencies.

It made sense to model such nodes as a distinct kind of entity in the graph instead of relying on the client searching for placeholders by-name, but I am curious what you think.

go through all placeholders and substitute their leaf nodes in this graph with a full dependency tree

If we need to transform the graph any way, can we just scan at the point when actual dependencies are ready? avoiding placeholder dependencies altogether?

This particular aspect of this design comes from the way SwiftPM (and I suspect other build systems?) interact with the compiler/driver. SwiftPM forms a build manifest for LLBuild ahead of time, for all targets; instead of planning & building a specific target, then requesting a build plan/manifest for the next target, etc. So this approach is meant to fit with that model. Hence, the driver needs to plan a build for a target/module when its dependencies have not yet been built.

I am also not sure how we would handle transitive dependencies in that case. If we have a target that is a dependency that has been pre-built, I think we would still need its dependency graph to get transitive dependency modules?

An important point to keeping SwiftPM's design w.r.t. planning a full package manifest ahead of time for all targets maximizes the parallelism we can extract from the full package build graph. Building/planning targets one-by-one would limit this, to a degree.

It makes a lot of sense now. Thank you, Artem! Could you document this in the source code explaining the post-processing steps clients will do for these placeholder dependencies and why they are necessary?

btw, the post-processing steps make the term placeholder more proper :)

btw, the post-processing steps make the term placeholder more proper :)

It also lends to nicer APIs in the client, resolvePlaceholders is a very natural-sounding operation.

We actually don't need the synthesized module path for these placeholders, right? we could use some internal ID to facilitate find and replacement. But it's minor though.

artemcm · 2020-07-23T04:57:57Z

@swift-ci please test

artemcm · 2020-07-23T17:14:19Z

@swift-ci please test

swift-ci · 2020-07-23T17:17:14Z

Build failed
Swift Test Linux Platform
Git Sha - aec6ac0a1e9250483c6f4aa5480fd583a2d82df2

swift-ci · 2020-07-23T17:24:32Z

Build failed
Swift Test OS X Platform
Git Sha - aec6ac0a1e9250483c6f4aa5480fd583a2d82df2

artemcm · 2020-07-23T18:02:22Z

@swift-ci please test OS X Platform

artemcm · 2020-07-23T18:04:08Z

@swift-ci please smoke test

artemcm · 2020-07-23T18:15:44Z

@swift-ci please test Windows Platform

swift-ci · 2020-07-23T19:29:20Z

Build failed
Swift Test OS X Platform
Git Sha - 4b2a44f231a084a938483cebc541595fcb00567c

artemcm · 2020-07-24T19:14:34Z

@swift-ci please test

swift-ci · 2020-07-24T19:17:00Z

Build failed
Swift Test Linux Platform
Git Sha - 4b2a44f231a084a938483cebc541595fcb00567c

swift-ci · 2020-07-24T19:18:00Z

Build failed
Swift Test OS X Platform
Git Sha - 4b2a44f231a084a938483cebc541595fcb00567c

With: `-external-dependency-module-map-file`

…pendenciesKind

…map parser.

artemcm · 2020-07-27T18:29:06Z

@swift-ci please test

swift-ci · 2020-07-27T18:31:53Z

Build failed
Swift Test Linux Platform
Git Sha - 483fafa86e371d1365f0fcb2d198e603df4abba5

swift-ci · 2020-07-27T18:33:12Z

Build failed
Swift Test OS X Platform
Git Sha - 483fafa86e371d1365f0fcb2d198e603df4abba5

artemcm · 2020-07-28T01:49:05Z

@swift-ci please test Linux platform

swift-ci · 2020-07-28T03:05:36Z

Build failed
Swift Test Linux Platform
Git Sha - 6cf9d898e335f033c7591833c666423345a9e2a3

artemcm · 2020-07-28T04:31:09Z

@swift-ci please test Linux platform

artemcm · 2020-07-28T04:32:14Z

@swift-ci please test OS X platform

swift-ci · 2020-07-28T04:33:59Z

Build failed
Swift Test Linux Platform
Git Sha - 6cf9d898e335f033c7591833c666423345a9e2a3

swift-ci · 2020-07-28T04:34:42Z

Build failed
Swift Test OS X Platform
Git Sha - 6cf9d898e335f033c7591833c666423345a9e2a3

artemcm · 2020-07-28T18:45:53Z

@swift-ci please test

swift-ci · 2020-07-28T18:48:11Z

Build failed
Swift Test Linux Platform
Git Sha - 8fc01c1804f3aa4f9f373d4e45da43e040dfaeb1

artemcm · 2020-07-28T18:48:38Z

@swift-ci please test Windows platform

swift-ci · 2020-07-28T18:49:01Z

Build failed
Swift Test OS X Platform
Git Sha - 8fc01c1804f3aa4f9f373d4e45da43e040dfaeb1

artemcm · 2020-07-28T18:49:17Z

@swift-ci please test Windows platform

artemcm requested review from DougGregor and nkcsgexi July 21, 2020 19:19

nkcsgexi reviewed Jul 21, 2020

View reviewed changes

artemcm changed the title ~~[Dependency Scanner] Add a scanner for explicit external module dependencies~~ [Dependency Scanner] Add a scanner for explicit placeholder module dependencies Jul 21, 2020

artemcm force-pushed the ExplicitPackageBuilds branch 3 times, most recently from 09070b1 to aec6ac0 Compare July 22, 2020 20:52

swiftlang deleted a comment from swift-ci Jul 23, 2020

artemcm mentioned this pull request Jul 23, 2020

[Explicit Module Builds] Support explicit module builds with external dependencies. swiftlang/swift-driver#181

Merged

artemcm force-pushed the ExplicitPackageBuilds branch from aec6ac0 to 4b2a44f Compare July 23, 2020 17:14

artemcm mentioned this pull request Jul 24, 2020

[Explicit Module Builds] Support explicit builds of packages witn inter-dependent Swift targets swiftlang/swift-package-manager#2827

Merged

artemcm force-pushed the ExplicitPackageBuilds branch from 4b2a44f to 483fafa Compare July 24, 2020 19:14

artemcm added 4 commits July 27, 2020 09:24

Factor out Explicit Module Map parsing into a standalone utility.

1c1a263

Add option to accept external module dependency map as input

c52982c

With: `-external-dependency-module-map-file`

Add new dependency kind (swiftExternal) to model external dependencies.

b5af62f

Add an external module dependency test

b85f547

artemcm added 3 commits July 27, 2020 09:24

Refactor ModuleDependenciesStorageBase to self-identify with ModuleDe…

7a736f6

…pendenciesKind

Keep the memory allocator local to module loaders, instead of module …

1d49d22

…map parser.

Rename external dependencies to placeholder dependencies.

746e89b

artemcm force-pushed the ExplicitPackageBuilds branch from 483fafa to 41f01fa Compare July 27, 2020 18:28

artemcm force-pushed the ExplicitPackageBuilds branch from 41f01fa to 6cf9d89 Compare July 28, 2020 01:48

artemcm force-pushed the ExplicitPackageBuilds branch from 6cf9d89 to 8fc01c1 Compare July 28, 2020 04:30

Add in-source rationale for why Placeholder dependencies are needed.

965ca69

artemcm force-pushed the ExplicitPackageBuilds branch from 8fc01c1 to 965ca69 Compare July 28, 2020 18:34

artemcm merged commit 413bbab into swiftlang:master Jul 28, 2020

[Dependency Scanner] Add a scanner for explicit placeholder module dependencies #33032

[Dependency Scanner] Add a scanner for explicit placeholder module dependencies #33032

Uh oh!

Conversation

artemcm commented Jul 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nkcsgexi left a comment

Choose a reason for hiding this comment

Uh oh!

artemcm commented Jul 21, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nkcsgexi Jul 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

artemcm Jul 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

artemcm commented Jul 23, 2020

Uh oh!

artemcm commented Jul 23, 2020

Uh oh!

swift-ci commented Jul 23, 2020

Uh oh!

swift-ci commented Jul 23, 2020

Uh oh!

artemcm commented Jul 23, 2020

Uh oh!

artemcm commented Jul 23, 2020

Uh oh!

artemcm commented Jul 23, 2020

Uh oh!

swift-ci commented Jul 23, 2020

Uh oh!

artemcm commented Jul 24, 2020

Uh oh!

swift-ci commented Jul 24, 2020

Uh oh!

swift-ci commented Jul 24, 2020

Uh oh!

artemcm commented Jul 27, 2020

Uh oh!

swift-ci commented Jul 27, 2020

Uh oh!

swift-ci commented Jul 27, 2020

Uh oh!

artemcm commented Jul 28, 2020

Uh oh!

swift-ci commented Jul 28, 2020

Uh oh!

artemcm commented Jul 28, 2020

Uh oh!

artemcm commented Jul 28, 2020

Uh oh!

swift-ci commented Jul 28, 2020

Uh oh!

swift-ci commented Jul 28, 2020

Uh oh!

artemcm commented Jul 28, 2020

Uh oh!

swift-ci commented Jul 28, 2020

Uh oh!

artemcm commented Jul 21, 2020 •

edited

Loading

nkcsgexi Jul 21, 2020 •

edited

Loading

artemcm Jul 21, 2020 •

edited

Loading