Improve stability of CMake generator selection #747
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The cms client normally started a pick call in the UI. This modification moves the questions infront of the start. This removes the required callback from cms client.
The prefered generators are extracted by the cmake drive from configuration, kit files or settings. This modification moves the aggregation of different prefered generator out of the driver into the cmake tools. This allows to defines exact preferred generators for testing (no hidden dependency). The driver only checks the prefered generator list, if there is more then one then the first existing generator is selected. - Fix wrong preferred generator definition in test - Extend debug information
The error of invalid generators/kits is a parallel event based dead of the backend driver (Cmake-server client). This modification moves the generator selection and test from the backend driver part to the UI accessable part of the driver. This makes exception visible for Promise and exception handling.
- Replace driver creation by only update existing driver if it is present - Clear active kit if it is broken - Extend test for linux
…r_selection_error_handling
|
#707 is about ready to go, if you can respond to my feedback, we can get that one committed so there will be less churn to review in this PR. |
bobbrow
previously approved these changes
Sep 7, 2019
src/cmake-tools.ts
Outdated
| @@ -271,25 +271,25 @@ export class CMakeTools implements vscode.Disposable, api.CMakeToolsAPI { | |||
| } | |||
|
|
|||
| let drv: CMakeDriver; | |||
| const preferredGenerators = this.getPreferredGenerators(); | |||
| const preferedGenerators = this.getPreferredGenerators(); | |||
There was a problem hiding this comment.
The spelling was correct before. Now it is incorrect. Please revert.
bobbrow
approved these changes
Sep 12, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change addresses item #512
This changes behavior and stability of the extension
The following changes are proposed:
The purpose of this change
Issue #512 reports that the extension crashes completely if there was a generator selection problem.
The cmake generator is checked at start of the cmake client when the cmake server protocol requires the information. If there is no useable build system (generator), then there cmake-server client throws an error. But there is no way to return this error into the cmake-server-driver instance. The driver instance is broken, and cmake-tools does not know it and will not create a new instance.
The simplest solution was to move the generator selection to the cmake-tools context so that the driver does not have to select the generator. It only uses it like other build information.
This kind of error is only possible in cmake-server-api driver, legacy driver and file-api driver need the generator before executing the cmake configure step.
Additional information