Skip to content

[Traits] Integrate traits into module loading #7689

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

Merged
merged 5 commits into from
Jun 20, 2024

Conversation

FranzBusch
Copy link
Member

Motivation

In my previous PR I landed the APIs for package traits. We now have to take this information into consideration when evaluating the module graph.

Modification

This PR implements the various trait features in the module graph loading stage. Including resolving optional dependencies, enabled traits of dependencies, build settings based on traits and passing defines for traits.

This PR also contains an exhaustive test fixtures which we can use to check if all trait functionality is working as expected.

@FranzBusch
Copy link
Member Author

@swift-ci please test

# Motivation
In my previous PR I landed the APIs for package traits. We now have to take this information into consideration when evaluating the module graph.

# Modification
This PR implements the various trait features in the module graph loading stage. Including resolving optional dependencies, enabled traits of dependencies, build settings based on traits and passing defines for traits.

This PR also contains an exhaustive test fixtures which we can use to check if all trait functionality is working as expected.
@FranzBusch FranzBusch force-pushed the fb-moduels-and-loading branch from 4451034 to 5e4c28b Compare June 20, 2024 13:02
@FranzBusch
Copy link
Member Author

@swift-ci please test

@FranzBusch FranzBusch force-pushed the fb-moduels-and-loading branch from 5e4c28b to 28ca7a4 Compare June 20, 2024 14:05
@FranzBusch
Copy link
Member Author

@swift-ci please test

Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

Overall SGTM, my only nit is for the error message and gratuitous use of public instead of package.

@FranzBusch
Copy link
Member Author

@swift-ci please test

@FranzBusch
Copy link
Member Author

@swift-ci test windows

@FranzBusch FranzBusch force-pushed the fb-moduels-and-loading branch from 305afc8 to 25f57e1 Compare June 20, 2024 15:29
@FranzBusch
Copy link
Member Author

@swift-ci please test

@FranzBusch
Copy link
Member Author

@swift-ci test windows

Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

Splendid, thank you!

@MaxDesiatov MaxDesiatov enabled auto-merge (squash) June 20, 2024 15:35
@MaxDesiatov
Copy link
Contributor

@swift-ci test windows

@FranzBusch
Copy link
Member Author

@swift-ci please test

@FranzBusch
Copy link
Member Author

@swift-ci test windows

1 similar comment
@MaxDesiatov
Copy link
Contributor

@swift-ci test windows

@MaxDesiatov MaxDesiatov added pending evolution proposal enhancement package manifests changes to package manifest APIs modules graph Modules dependency resolution labels Jun 20, 2024
@MaxDesiatov MaxDesiatov merged commit 7746e69 into swiftlang:main Jun 20, 2024
5 checks passed
@FranzBusch FranzBusch deleted the fb-moduels-and-loading branch June 20, 2024 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement modules graph Modules dependency resolution package manifests changes to package manifest APIs pending evolution proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants