Skip to content
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.

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

[Explicit Module Builds] Fix topological sort of inter-module dependency graph to account for Swift overlay dependencies #1438

Merged
merged 1 commit into from Sep 8, 2023

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Sep 7, 2023

No description provided.

@artemcm
Copy link
Contributor Author

artemcm commented Sep 7, 2023

@swift-ci test

Copy link
Contributor

@cachemeifyoucan cachemeifyoucan left a comment

Choose a reason for hiding this comment

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

LGTM other than one small question.

@@ -31,7 +31,7 @@ public enum ModuleDependencyId: Hashable {
}
}

internal var moduleNameForDiagnostic: String {
var moduleNameForDiagnostic: String {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use this in incremental build explanation remarks. Though the change here was not intentional so I reverted it.

…ncy graph

to account for Swift overlay dependencies
@artemcm artemcm force-pushed the FixTopoForExplicitOverlayDeps branch from bc4e652 to f4fc3b8 Compare September 7, 2023 19:40
@artemcm
Copy link
Contributor Author

artemcm commented Sep 7, 2023

@swift-ci test

@artemcm
Copy link
Contributor Author

artemcm commented Sep 7, 2023

@swift-ci test Windows platform

@artemcm artemcm merged commit f5df590 into apple:main Sep 8, 2023
3 checks passed
@artemcm artemcm deleted the FixTopoForExplicitOverlayDeps branch September 8, 2023 02:32
artemcm added a commit to artemcm/swift that referenced this pull request Sep 26, 2023
…the set of direct dependencies

Functionally reverts apple#67928.
We must still support potentially older drivers which are not ready for this change due to: apple/swift-driver#1438
artemcm added a commit to artemcm/swift that referenced this pull request Sep 26, 2023
…the set of direct dependencies

Functionally reverts apple#67928.
We must still support potentially older drivers which are not ready for this change due to: apple/swift-driver#1438
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants