Skip to content

Replace SwiftDriverExecutor with SPMSwiftDriverExecutor #2946

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
Sep 23, 2020

Conversation

owenv
Copy link
Contributor

@owenv owenv commented Sep 20, 2020

SPM does not need or use the full functionality of SwiftDriverExecutor, which we're planning to move to a separate library so that the SwiftDriver planning library no longer depends on LLBuild. Introduce a minimal SPMSwiftDriverExecutor for the processes the integrated driver needs to launch during planning.

@owenv
Copy link
Contributor Author

owenv commented Sep 20, 2020

@swift-ci test

@owenv
Copy link
Contributor Author

owenv commented Sep 20, 2020

@swift-ci please test self hosted

1 similar comment
@owenv
Copy link
Contributor Author

owenv commented Sep 20, 2020

@swift-ci please test self hosted

@owenv
Copy link
Contributor Author

owenv commented Sep 20, 2020

@swift-ci please smoke test


try job.verifyInputsNotModified(since: recordedInputModificationDates,
fileSystem: fileSystem)
assert(!job.requiresInPlaceExecution,
Copy link
Member

Choose a reason for hiding this comment

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

I don't see how this can fail now, but it seems fragile. Should we throw an error here rather than crash, given the likelihood that someone can slip an argument through to the Swift driver that would cause in-place execution? Crashing SwiftPM seems unfortunate there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea! I changed this to throw an error instead

SPM does not need or use the full functionality of SwiftDriverExecutor,
which we're planning to move to a separate library so that the SwiftDriver
planning lib no longer depends on LLBuild. Introduce a miinimal SPMSwiftDriverExecutor
for the processes the integrated driver needs to launch during planning.
@owenv owenv force-pushed the driver-library-split branch from 0fd6eca to 2248570 Compare September 22, 2020 04:13
@owenv
Copy link
Contributor Author

owenv commented Sep 22, 2020

@swift-ci smoke test

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

Thanks!

@owenv owenv changed the base branch from master to main September 23, 2020 03:35
@owenv
Copy link
Contributor Author

owenv commented Sep 23, 2020

@DougGregor do you mind helping merge this when you have a chance? Thanks!

@DougGregor DougGregor merged commit 93b9142 into swiftlang:main Sep 23, 2020
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