Skip to content

Refactor Commands module #5807

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 1 commit into from
Nov 2, 2022
Merged

Refactor Commands module #5807

merged 1 commit into from
Nov 2, 2022

Conversation

neonichu
Copy link
Contributor

@neonichu neonichu commented Oct 12, 2022

This is the initial step in reducing the first stage of the bootstrap build. Currently, the initial bootstrap build using CMake is essentially building the entirety of SwiftPM and that is (mostly) because the Commands module pulls in everything that SwiftPM possibly offers.

This introduces a new CoreCommands module which contains the necessary parts for providing a minimal build command using the native build system. This can be used to provide a new swift-bootstrap executable which is a minimal version of swift-build, but actually doing so will be a separate PR.

The Windows build currently relies entirely on CMake, so initally we will not be able to profit much from the minimal build, but this will get us closer to doing so in the future.

@neonichu neonichu added the WIP Work in progress label Oct 12, 2022
@neonichu neonichu self-assigned this Oct 12, 2022
@tomerd
Copy link
Contributor

tomerd commented Oct 12, 2022

very cool!

@neonichu neonichu force-pushed the swift-bootstrap-cmd branch from 6d2c99f to 14c4329 Compare October 14, 2022 00:59
@neonichu
Copy link
Contributor Author

@swift-ci please smoke test

@neonichu
Copy link
Contributor Author

swiftlang/sourcekit-lsp#650
@swift-ci please smoke test

@neonichu
Copy link
Contributor Author

@neonichu neonichu removed the WIP Work in progress label Oct 14, 2022
@stevapple
Copy link
Contributor

stevapple commented Oct 14, 2022

Bootstrapping SwiftPM on Windows with "debug" configuration is possible, but in release build it will hit swiftlang/swift-llbuild#844, so this is currently unusable. It’s nice to see SwiftPM being bootstrapped on Windows, in align with other platforms.

@neonichu
Copy link
Contributor Author

swiftlang/swift#61574
swiftlang/sourcekit-lsp#650
@swift-ci please smoke test macOS

@neonichu
Copy link
Contributor Author

fatal error encountered while reading from module 'CxxVectorSum'

Looks like this is an unrelated issue on the CI right now? I also saw it on TSC PRs.

@neonichu
Copy link
Contributor Author

swiftlang/swift#61574
@swift-ci please smoke test macOS

@tomerd tomerd added the WIP Work in progress label Oct 17, 2022
neonichu added a commit that referenced this pull request Oct 27, 2022
This is an initial step of unwinding the direct dependency of individual commands to a concrete build system implementation, through two major changes:
- Adding a new target `DriverSupport` which contains integration with swift-driver that is independent of a concrete build system being used (today mostly `checkSupportedFrontendFlags(flags:fileSystem)` but there could be more in the future)
- Adding a new `BuildSystemProvider` API which handles instantiation of a concrete build system. This will e.g. allow creating a `SwiftCommand` which doesn't depend on the `XCBuildSupport` module (which is going to help with #5807) and also provides a way to generally clean up (almost) all callsites from directly interacting with a concrete build system.
@neonichu
Copy link
Contributor Author

Split out #5846 to address the concerns about build system support.

neonichu added a commit that referenced this pull request Oct 27, 2022
This is an initial step of unwinding the direct dependency of individual commands to a concrete build system implementation, through two major changes:
- Adding a new target `DriverSupport` which contains integration with swift-driver that is independent of a concrete build system being used (today mostly `checkSupportedFrontendFlags(flags:fileSystem)` but there could be more in the future)
- Adding a new `BuildSystemProvider` API which handles instantiation of a concrete build system. This will e.g. allow creating a `SwiftCommand` which doesn't depend on the `XCBuildSupport` module (which is going to help with #5807) and also provides a way to generally clean up (almost) all callsites from directly interacting with a concrete build system.
neonichu added a commit that referenced this pull request Oct 27, 2022
This is an initial step of unwinding the direct dependency of individual commands to a concrete build system implementation, through two major changes:
- Adding a new target `DriverSupport` which contains integration with swift-driver that is independent of a concrete build system being used (today mostly `checkSupportedFrontendFlags(flags:fileSystem)` but there could be more in the future)
- Adding a new `BuildSystemProvider` API which handles instantiation of a concrete build system. This will e.g. allow creating a `SwiftCommand` which doesn't depend on the `XCBuildSupport` module (which is going to help with #5807) and also provides a way to generally clean up (almost) all callsites from directly interacting with a concrete build system.
neonichu added a commit that referenced this pull request Oct 27, 2022
This is an initial step of unwinding the direct dependency of individual commands to a concrete build system implementation, through two major changes:
- Adding a new target `DriverSupport` which contains integration with swift-driver that is independent of a concrete build system being used (today mostly `checkSupportedFrontendFlags(flags:fileSystem)` but there could be more in the future)
- Adding a new `BuildSystemProvider` API which handles instantiation of a concrete build system. This will e.g. allow creating a `SwiftCommand` which doesn't depend on the `XCBuildSupport` module (which is going to help with #5807) and also provides a way to generally clean up (almost) all callsites from directly interacting with a concrete build system.
neonichu added a commit that referenced this pull request Oct 27, 2022
This is an initial step of unwinding the direct dependency of individual commands to a concrete build system implementation, through two major changes:
- Adding a new target `DriverSupport` which contains integration with swift-driver that is independent of a concrete build system being used (today mostly `checkSupportedFrontendFlags(flags:fileSystem)` but there could be more in the future)
- Adding a new `BuildSystemProvider` API which handles instantiation of a concrete build system. This will e.g. allow creating a `SwiftCommand` which doesn't depend on the `XCBuildSupport` module (which is going to help with #5807) and also provides a way to generally clean up (almost) all callsites from directly interacting with a concrete build system.
@neonichu neonichu force-pushed the swift-bootstrap-cmd branch from 14c4329 to 73137a8 Compare November 1, 2022 04:32
@neonichu neonichu changed the title Start reducing first stage bootstrap build Refactor Commands module Nov 1, 2022
@neonichu neonichu removed the WIP Work in progress label Nov 1, 2022
@neonichu neonichu marked this pull request as ready for review November 1, 2022 04:33
@neonichu
Copy link
Contributor Author

neonichu commented Nov 1, 2022

Simplified this PR a little bit for easier review and updated it based on the refactoring I have done separately.

Once this lands, there will be two eventual follow-up PRs:

  • a pretty simple PR that introduces the swift-bootstrap command and uses it for the bootstrap script
  • a PR which changes the bootstrap process and CMake configuration to actually be more minimal. This requires us to move the Windows build to do bootstrapping first, so it'll take us a few steps to get there.

@neonichu
Copy link
Contributor Author

neonichu commented Nov 1, 2022

@swift-ci please smoke test

@neonichu neonichu force-pushed the swift-bootstrap-cmd branch from 73137a8 to 20b354a Compare November 1, 2022 06:33
@neonichu
Copy link
Contributor Author

neonichu commented Nov 1, 2022

@swift-ci please smoke test

observabilityScope: customObservabilityScope ?? swiftTool.observabilityScope
)
},
], uniquingKeysWith: { a, b in
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a bit unclear to me, we are merging a map of 1?

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're merging with the providers from defaultBuildSystemProvider above, right now this doesn't have an effect, but I wanted to keep it forward compatible.

Copy link
Contributor

@tomerd tomerd left a comment

Choose a reason for hiding this comment

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

very nice

This is the initial step in reducing the first stage of the bootstrap build. Currently, the initial bootstrap build using CMake is essentially building the entirety of SwiftPM and that is (mostly) because the `Commands` module pulls in everything that SwiftPM possibly offers.

This introduces a new `CoreCommands` module which contains the necessary parts for providing a minimal build command using the native build system. This can be used to provide a new `swift-bootstrap` executable which is a minimal version of `swift-build`, but actually doing so will be a separate PR.

The Windows build currently relies entirely on CMake, so initally we will not be able to profit much from the minimal build, but this will get us closer to doing so in the future.
@neonichu neonichu force-pushed the swift-bootstrap-cmd branch from 20b354a to fab38b1 Compare November 1, 2022 22:36
@neonichu
Copy link
Contributor Author

neonichu commented Nov 1, 2022

@swift-ci please smoke test

@neonichu neonichu merged commit 2a5650d into main Nov 2, 2022
@neonichu neonichu deleted the swift-bootstrap-cmd branch November 2, 2022 00:35
CodaFi added a commit that referenced this pull request Nov 2, 2022
This was removed in #5807
before projects had time to migrate. Restore this option until they do.
CodaFi added a commit that referenced this pull request Nov 2, 2022
This was removed in #5807
before projects had time to migrate. Restore this option until they do.

rdar://101841334
@CodaFi CodaFi mentioned this pull request Nov 2, 2022
CodaFi added a commit that referenced this pull request Nov 2, 2022
CodaFi added a commit that referenced this pull request Nov 2, 2022
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.

3 participants