Skip to content

Create BuiltInBuildSystem in BuildSystemAdapter #1653

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 4 commits into from
Sep 10, 2024

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Sep 10, 2024

We currently create the BuiltInBuildSystem while constructing a Workspace and then pass it into the BuildSystemManager, which will do some final wiring up, setting itself as the message handler and delegate of the build system.

This PR moves the creation of the BuiltInBuildSystem into the BuiltInBuildSystemAdapter. That way, the creation of a BuiltInBuildSystem is more like the initialization of a BSP server.

To do so, we need to split the creation of a build system (now happening in BuiltInBuildSystemAdapter) and determining what kind of build system to use for a workspace (still happening during workspace creation). I have found that this split was generally beneficial and made the code easier to reason about.

…orkspace creation

This allows us to create the build system from a `BuiltInBuildSystemAdapter` when it receives an `InitializeRequest`, which will be done in a follow-up commit.
This way we create the `BuiltInBuildSystem` at the same time that we create the `BuildSystemManager`, which gets us one step closer to creating the `BuiltInBuildSystem` from the `BuiltInBuildSystemAdapter`.
This moves the creation another step closer to creating the `BuiltInBuildSystem` inside `BuiltInBuildSystemAdapter`.
This finalizes the move of `BuiltInBuildSystem` creation into `BuiltInBuildSystemAdapter` and means that we can set the message handler of the `BuiltInBuildSystem` during initialization instead of using a setter method.
@ahoppen
Copy link
Member Author

ahoppen commented Sep 10, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Sep 10, 2024

Will address review comments in follow-up PRs.

@ahoppen ahoppen merged commit 9473aeb into swiftlang:main Sep 10, 2024
3 checks passed
@ahoppen ahoppen deleted the build-system-creation branch September 10, 2024 13:37
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.

1 participant