Skip to content

Refactor build system support #5846

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
Oct 27, 2022
Merged

Refactor build system support #5846

merged 1 commit into from
Oct 27, 2022

Conversation

neonichu
Copy link
Contributor

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 Refactor Commands module #5807) and also provides a way to generally clean up (almost) all callsites from directly interacting with a concrete build system.

@neonichu neonichu requested a review from abertelrud as a code owner October 27, 2022 00:19
@neonichu neonichu self-assigned this Oct 27, 2022
@neonichu neonichu requested review from tomerd and elsh as code owners October 27, 2022 00:19
@neonichu
Copy link
Contributor Author

@swift-ci please smoke test

@neonichu
Copy link
Contributor Author

@swift-ci please smoke test macOS

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.

looks great, some small comments and ideas

@neonichu
Copy link
Contributor Author

Looks like there's an issue to be addressed with the CMake build, probably a missing dependency:

/Users/ec2-user/jenkins/workspace/swift-package-manager-PR-macos-smoke-test/branch-main/swiftpm/Sources/DriverSupport/Frontend.swift:13:8: error: no such module 'SwiftDriver'
import SwiftDriver

@neonichu
Copy link
Contributor Author

Cannot repro the CMake issue in a local build 🤔

@neonichu
Copy link
Contributor Author

Cannot repro the CMake issue in a local build 🤔

Ah, that's because I was on the wrong branch 🤣

@neonichu
Copy link
Contributor Author

@swift-ci please smoke test

@neonichu neonichu force-pushed the refactor-build-system-support branch from 81d3ac5 to 6fa8141 Compare October 27, 2022 17:48
@neonichu
Copy link
Contributor Author

@swift-ci please smoke test

@neonichu neonichu force-pushed the refactor-build-system-support branch from 6fa8141 to 10af1c1 Compare October 27, 2022 21:42
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 refactor-build-system-support branch from 10af1c1 to 151804b Compare October 27, 2022 21:44
@neonichu
Copy link
Contributor Author

@swift-ci please smoke test

@neonichu
Copy link
Contributor Author

@swift-ci please smoke test macOS

@neonichu neonichu merged commit b30ba0b into main Oct 27, 2022
@neonichu neonichu deleted the refactor-build-system-support branch October 27, 2022 23:45
neonichu added a commit that referenced this pull request Oct 31, 2022
This was broken in #5846 which accidentally used the default value instead of passing along the global option.

rdar://101681661
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.

2 participants