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

do not parse cmake.preferredGenerators, instead just hand it over to cmake via -G option #33

Closed
MinnieTheMoocher opened this issue Sep 19, 2016 · 17 comments

Comments

@MinnieTheMoocher
Copy link

I just tried to set an invalid value in my user settings:
"cmake.preferredGenerators": [ "xxxVisual Studio 14 2015"]
and did a "CMake: Configure" step.
I wanted to see if the plugin is agnostic to the contents of that variable
and just properly passes it to cmake (in case that the cmake developers
add new generators).

This led to the strange effect that

  • VSCode showed me an error message that that generator is unknown
  • but anyway launched a cmake "configure" step
  • which fell back to using Visual Studio 14 2015 (because it probably found that installed on my machine)

That's different from what I expected the tool to do:

  • it should not try to analyze that string on its own. Why should it?
  • instead, it should just pass that to cmake and let that one deal with it
  • if cmake finds the generator to be invalid, it should terminate with error message
@MinnieTheMoocher MinnieTheMoocher changed the title problem with cmake.preferredGenerators do not parse cmake.preferredGenerators, instead just hand it over to cmake via -G option Sep 19, 2016
@vector-of-bool
Copy link
Contributor

Part of the reason to parse it is because of the "Preferred" idea. If one generator is unavailable, try the next one in the list. I tried to determine the availability myself, but it seems I can't quite get it right (See #1). If I delegate to CMake to try each generator, it would be more reliable, but it could significantly slow down the ability to configure the first time in the case that one or more generators in preferredGenerators is unavailable.

For example, the default first generator is Ninja because if a user has it installed, they definitely want to use it; but for users that don't have it installed, they will fall back to using the Makefile generator for their platform, and most users never even need to know that the "preferred generators" setting is even a thing.

@MinnieTheMoocher
Copy link
Author

MinnieTheMoocher commented Sep 21, 2016

That's right. But by hardcoding the generator check, you tie your plugin to only work with certain cmake versions, and not with others. For example, the generator "Visual Studio 10" recently has been changed to become "Visual Studio 10 2010", and the destination platform suffices (32bit vs 64bit build) only recently were added. Thus, if you rely on some generator name syntax, you lose compatibility of your plugin.
My expectation is that it should work with as much cmake versions as possible.
Your idea of trying out various generators is a good one IMHO,
I just suggest that this should be done without parsing the preferredGenerators list.
I currently still cannot see the necessity to do that:
Can't you just pass the generators to cmake in the preferred order
and this way try them, without parsing them first?

@tapir
Copy link

tapir commented Sep 29, 2016

I agree. Preferred feature is not needed at all. What generally developers want is just to set generator per project basis. So a cmake.Generator field for the project settings with valid generators is the only thing needed. Which should be passed directly with -G to cmake

@m-j-w
Copy link

m-j-w commented Sep 29, 2016

Important thing first: That's a really useful VS Code extension you've written there. Thanks a lot for that!

However, I'd also like to join and argue against the current parsing approach, although I understand the rationale behind. Unfortunately, this restricts the vast range of possible cmake generator configurations far too much.

Best example is in fact the Visual Studio generators, to which the extension currently defaults on Windows. However, the full configuration not only requires the -G option to be given, but also the -T for the toolset (to be able to switch to Clang/C2) and the -A for the platform (to cross-compile e.g. for ARM).

For myself, it's also not possible to switch to Unix Makefiles using Cygwin or MSYS2 on Windows since the Unix Makefile generator is not accepted. See also #40.

Thus, I'd suggest to keep the current possibility of trying to figure out a default with the preferredGenerators, but also to add a cmakeConfigureArguments and cmakeBuildArguments or similar. If the preferredGenerators are empty, then the extension shouldn't figure out anything, not provide a -G xxx at all, and simply leave that to the user through the additional flags, or even completely leave it to CMake.

@vector-of-bool
Copy link
Contributor

Ah! I didn't even consider the -T argument.

I'll probably go the route of allowing the user specify their generator explicitly, and falling back to the current logic if it is unspecified. I'll also add options specifically for the toolchain and platform, and something generic like cmakeConfigureArguments and cmakeBuildArguments.

Does that sound good to everyone?

@tapir
Copy link

tapir commented Sep 29, 2016

I'm gonna push it a little bit further if I may.
How about OS specific settings?

Integrated terminal does it this way:

    "terminal.integrated.shell.linux": "/bin/bash",

    // The command line arguments to use when on the Linux terminal.
    "terminal.integrated.shellArgs.linux": [],

    // The path of the shell that the terminal uses on OS X.
    "terminal.integrated.shell.osx": "/bin/bash",

    // The command line arguments to use when on the OS X terminal.
    "terminal.integrated.shellArgs.osx": [],

That would completely let the user customize cmake generators and toolchains for every OS in a single settings file.

@m-j-w
Copy link

m-j-w commented Sep 29, 2016

As a first step, that's probably too much. A future way after the simple additional arguments could be then to enable build-configuration-configurations ;-) Something like all the settings that you would pass to cmake, the shell to use, debugger settings, build type, etc. and then choose from those similarly as choosing between Debug and Release. But that's certainly way more effort to implement in a really user-friendly way.
I'd be happy with those additional arguments for now.

@vector-of-bool
Copy link
Contributor

Thanks for all the suggestions!

I have a pretty common use case where I want to be able to flip back and forth between a few sets of configuration settings, so that may be something coming soon.

I'll take a look at this some more later when I get some time.

@grisevg
Copy link

grisevg commented Sep 29, 2016

+1 for the ability to pass any flags, but that would also mean we'd need custom build command.
If you generate Visual Studio solution, you then probably want to run msbuild.exe on it and not make

@vector-of-bool
Copy link
Contributor

@grisevg: The build command is handled already for Visual Studio solutions. For this extension, it uses cmake --build <build-dir>, and CMake will automatically find the best way to run the build, either by invoking ninja, make, msbuild, etc. based on the project generator.

CMake also lets you pass options to the underlying build tool using the -- option. I'll probably tweak it so that a user can specify what those options will be.

@grisevg
Copy link

grisevg commented Sep 29, 2016

@vector-of-bool oh sweet, didn't know about that! No reason then not to support any generator passed as string, right? Or even better, just allow any custom flags.

@vector-of-bool
Copy link
Contributor

The 0.5.4 release should address most of the concerns mentioned in this thread. Any feedback/comments/complaints/questions are welcome. If this particular issue is solved for everyone, I'll close it.

I've opened up this issue to track build variants, as discussed above.

@vector-of-bool
Copy link
Contributor

Now support forcing generator based on platform. Should fix most concerns. I'll close this unless anyone else would like to bring up more concerns

@vector-of-bool
Copy link
Contributor

Great news: next cmake version will support explicitly asking for capabilities, including what generators are supported. I feel that the upcoming vscode and cmake updates will really open up the door to some great opportunities regarding what can be done with this extension. Stay tuned...

@tapir
Copy link

tapir commented Oct 6, 2016

I couldn't make cmake to believe it was in a mingw environement. When I run cmake from MSYS2 shell (command line of mingw-w64) it works OK but when vscode-cmake-tools runs it, it can't see gcc, libs, include etc... Which is of course not the fault of vscode-cmake-tools but is there something that can be done to overcome this?

@vector-of-bool
Copy link
Contributor

It's possible it is an environment issue. Have you tried running vscode using the msys2 shell?

@m-j-w
Copy link

m-j-w commented Oct 6, 2016

@tapir: it certainly is a environment issue. the MSYS2 shell sets up the path correctly for the MSYS2 provided CMake to be able to find your compilers.
@vector-of-bool, that's actually a nice work around, works quite well. However, some of the decision logic with the generator still is weird. Sometimes it doesn't get emitted as command line argument. I'll try to figure out what's happening there...

For all other readers a quick tip as a starting point: To be able to use CMake Tools mostly seamlessly with MSYS2/MinGW64 or Cygwin64 on Windows: Launch your MSYS2 or Cygwin shell (e.g. bash), and start Visual Studio Code from in there by simply typing:

$ /c/Program\ Files\ \(x86\)/Microsoft\ VS\ Code/bin/code

On Cygwin you might need to prepend /cygdrive to the above. Further, defining a shell command alias might simplify things there a bit.
Then it'll naturally find the MSYS2 or Cygwin provided CMake and compilers.

Still, setting the generator to "Unix Makefiles" or "Ninja" might be necessary either by giving the CMake Tools setting in your projects .vscode/settings.json the "cmake.generator" = "Unix Makefiles" or "cmake.configureArgs": ["-G", "Unix Makefiles"] along with the usual CMAKE_CXX_COMPILER arguments for the compiler to choose.
Otherwise MSYS2 CMake appears to default to the Visual Studio build system.

@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

No branches or pull requests

5 participants