Skip to content

[Explicit Module Builds] Emit a JSON file that specifies details of explicit Swift module dependencies #133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jul 1, 2020
Merged

[Explicit Module Builds] Emit a JSON file that specifies details of explicit Swift module dependencies #133

merged 2 commits into from
Jul 1, 2020

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Jun 19, 2020

This PR implements the interface expected by the frontend, as built by @nkcsgexi in:
swiftlang/swift#32355
swiftlang/swift#32450

Swift module's explicit Swift module dependencies are now encoded in a JSON file, passed with -explicit-swift-module-map-file. The key benefit is that now the frontend doesn't need to deserialize the .swiftmodule file just to collect the module's name.

Resolves rdar://problem/64533451

@artemcm artemcm requested review from DougGregor and nkcsgexi June 19, 2020 23:31
@artemcm
Copy link
Contributor Author

artemcm commented Jun 19, 2020

@swift-ci please test

cltnschlosser
cltnschlosser reviewed Jun 20, 2020
View reviewed changes
@artemcm artemcm force-pushed the JSONSwiftDependencies branch from f4eb16b to d9a63cc Compare June 22, 2020 16:30
nkcsgexi
nkcsgexi reviewed Jun 22, 2020
View reviewed changes
Copy link
Contributor

@nkcsgexi nkcsgexi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adopting this so fast!

Sources/SwiftDriver/Explicit Module Builds/SwiftModuleArtifacts.swift Outdated
self.sourceInfoPath = sourceInfoPath
}

private enum CodingKeys : String, CodingKey {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't need the CodingKeys if we use these keys as the field names.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mostly just wanted to stick to the Swift naming convention (Types start with an uppercase letter, variables start with a lowercase latter), but I don't have a strong preference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair. How about we change the compiler-side to use keys conforming to the Swift naming convention and remove CodingKeys?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that'd be great!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the change: swiftlang/swift#32507

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkcsgexi , I've addressed the comments, but the change now has to wait for swiftlang/swift#32507 to be picked up by the next master snapshot toolchain.

@artemcm artemcm force-pushed the JSONSwiftDependencies branch 2 times, most recently from 2b83afd to d0a0da6 Compare June 22, 2020 23:27
@artemcm
Copy link
Contributor Author

artemcm commented Jun 22, 2020

@swift-ci please test

@artemcm artemcm force-pushed the JSONSwiftDependencies branch 3 times, most recently from c4b99b8 to ade77cd Compare June 23, 2020 23:42
nkcsgexi
nkcsgexi approved these changes Jun 24, 2020
View reviewed changes
Copy link
Contributor

@nkcsgexi nkcsgexi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @artemcm !

@artemcm artemcm force-pushed the JSONSwiftDependencies branch 2 times, most recently from c8b256a to defa4fc Compare June 25, 2020 21:04
@DougGregor
Copy link
Member

@swift-ci test

DougGregor
DougGregor approved these changes Jun 27, 2020
View reviewed changes
@artemcm artemcm force-pushed the JSONSwiftDependencies branch from defa4fc to 5cb5a9c Compare June 28, 2020 01:41
@artemcm
Copy link
Contributor Author

artemcm commented Jun 28, 2020

We haven't had a new snapshot toolchain on swift.org for a few days now, and this change has a dependency on one of Xi's changes that was made after the most recent one one was created. So this will need to wait a little bit until it can be tested in the CI.

@artemcm artemcm force-pushed the JSONSwiftDependencies branch from 5cb5a9c to b817ead Compare June 29, 2020 22:06
@artemcm
Copy link
Contributor Author

artemcm commented Jun 29, 2020

@swift-ci please test

1 similar comment
@artemcm
Copy link
Contributor Author

artemcm commented Jun 30, 2020

@swift-ci please test

@artemcm
Copy link
Contributor Author

artemcm commented Jun 30, 2020

@swift-ci test

@artemcm
Copy link
Contributor Author

artemcm commented Jul 1, 2020

@swift-ci please test

artemcm added 2 commits July 1, 2020 11:17
@artemcm
[Explicit Module Builds] Emit a JSON file that specifies details of e…
56f0660 
…xplicit Swift module dependencies

This PR implements the interface expected by the frontend, as built in:
swiftlang/swift#32355
swiftlang/swift#32450

Swift module's explicit Swift module dependencies are now encoded in a JSON file, passed with `-explicit-swift-module-map-file`

Resolves rdar://problem/64533451
@artemcm
[Explicit Module Builds] Refactor clang module dependency commandLine…
33480f2 
…/input gathering to match swift
@artemcm artemcm force-pushed the JSONSwiftDependencies branch from b817ead to 33480f2 Compare July 1, 2020 18:17
@artemcm
Copy link
Contributor Author

artemcm commented Jul 1, 2020

@swift-ci please test

1 similar comment
@artemcm
Copy link
Contributor Author

artemcm commented Jul 1, 2020

@swift-ci please test

@artemcm artemcm merged commit 9384f94 into swiftlang:master Jul 1, 2020
@artemcm artemcm deleted the JSONSwiftDependencies branch January 20, 2021 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cltnschlosser cltnschlosser cltnschlosser left review comments

@DougGregor DougGregor DougGregor approved these changes

@nkcsgexi nkcsgexi nkcsgexi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@artemcm @DougGregor @cltnschlosser @nkcsgexi