Skip to content
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

Add build and configure commands to show cmake commands without running them #1767

Merged
merged 16 commits into from
Oct 11, 2021

Conversation

xisui-MSFT
Copy link
Collaborator

@xisui-MSFT xisui-MSFT commented Apr 14, 2021

Added configure and build commands to log corresponding cmake commands without running them.

According to Erika, these commands are mainly in response to feedback (primarily from VS users) that they didn't understand how settings map to the actual CMake command line, and the goal is to provide transparency so users can see how specific settings map to the CLI.

Current implementation requires cmake installed on the machine, since cmake-tools.ts and driver.ts aren't in a good state to get rid of this requirement, and I think it's okay as the target scenario isn't checking configure/build commands without cmake installed.

@bobbrow bobbrow added this to the 1.8.0 milestone Apr 14, 2021
@bobbrow
Copy link
Member

bobbrow commented Jul 2, 2021

It's not really a "dry run" so much as it is just logging the command line. A dry run tells you everything that would happen if that command was run.

I would prefer we come up with a better name for the command. "CMake: Log Configure Command" or something like that would be more accurate, I think. You might also want to put focus on the output window when the command is run.

@bobbrow bobbrow modified the milestones: 1.8.0, 1.9.0 Jul 9, 2021
@xisui-MSFT xisui-MSFT changed the title Add dry run commands Add build and configure commands to log cmake commands without running them Jul 9, 2021
@elahehrashedi elahehrashedi self-requested a review July 22, 2021 01:24
@elahehrashedi
Copy link
Contributor

elahehrashedi commented Jul 22, 2021

I am getting this error while testing the branch:
image

I think it is related to the latest regression we had, as your branch is up to date with the develop: #2006

I will test an older version of your branch i.e. d51d531 .
I have two questions:

  1. do you also see the same error while testing your latest version or not? just wondering.
  2. Is it like a Demo or a Log? In my mind log build command would have given me some info about the build command that was run previously. While this is more like giving insight into what a build command would do. I appreciate some info.

@xisui-MSFT
Copy link
Collaborator Author

  1. I haven't tested this for a long time. I'll test it today.
  2. This is a great feedback. I have the same feeling as well. I'll change it to "show config/build command".

@xisui-MSFT
Copy link
Collaborator Author

@elahehrashedi I tested the latest changes, and didn't see the issue in your comment.

@xisui-MSFT xisui-MSFT changed the title Add build and configure commands to log cmake commands without running them Add build and configure commands to show cmake commands without running them Jul 22, 2021
@elahehrashedi
Copy link
Contributor

Copying the error message here:

Error: Failed to get build command
    at CMakeTools.runBuild (d:\vscode-cmake-tools\src\cmake-tools.ts:1276:15)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at d:\vscode-cmake-tools\src\extension.ts:1521:21

@xisui-MSFT
Copy link
Collaborator Author

@elahehrashedi The problem should be fixed now. Can you please test it?

@elahehrashedi
Copy link
Contributor

@andreeis I have tested the code and it is working. We can have this checked in for 1.9.

@andreeis
Copy link
Contributor

andreeis commented Oct 6, 2021

Not sure how important this is, but for showConfigureCommand we write more (unuseful?) lines than for showBuildCommand.

[main] Configuring folder: Project1
[main] Saving open files before configure/build
[driver] Run _refreshExpansions
[driver] Run _refreshExpansions cb
[driver] Start configure
[driver] Runnnig pre-configure checks and steps
[driver] Run _refreshExpansions
[driver] Run _refreshExpansions cb
[cmakefileapi-driver] "c:/Program Files/CMake/bin/cmake.exe" --no-warn-unused-cli -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=TRUE -Hc:/Work/Projects/CMakeTools/GitHub2145/Project1 -Bc:/Work/Projects/CMakeTools/GitHub2145/Project1/build -G "Visual Studio 16 2019" -T host=x64 -A x64
[extension] [8858] cmake.showConfigureCommand finished (returned 0)

I am approving and will follow up whether we should hide the lines that are not the cmake configure command itself. It would just be querying in a few extra places for the boolean showCommandOnly or the trigger value and it would also require passing these further to some functions that do write these lines and which had no reason yet to have this parameter added to their prototype.

src/cmake-tools.ts Outdated Show resolved Hide resolved
@andreeis andreeis merged commit d28a4f8 into microsoft:develop Oct 11, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants