Different behavior between extension and CMake cli, when choosing the default generator in presets

[jochil]

Brief Issue Summary

As the generator field is optional in a preset using CMake directly falls back to the default generator discovery procedure. I would expect the vscode extension to do the same... but it falls back to Ninja as generator, according to observed behavior and this code block:

vscode-cmake-tools/src/preset.ts

Lines 707 to 712 in 01c4796

	if (!preset.generator) {
	const defaultValue = preferredGeneratorName ?? 'Ninja';
	log.debug(localize('generator.undefined', 'Configure preset {0}: No generator specified, using default value {1}', preset.name, `"${defaultValue}"`));
	preset.generator = defaultValue;
	}

From the CMake docs at https://cmake.org/cmake/help/latest/manual/cmake-presets.7.html:

An optional string representing the generator to use for the preset. If generator is not specified, it must be inherited from the inherits preset (unless this preset is hidden). In version 3 or above, this field may be omitted to fall back to regular generator discovery procedure.

I would like to know if there is a reason (I might have overlooked) for this behavior... if not I would be happy to create a PR 🙂

CMake Tools Diagnostics

No response

Debug Log

No response

Additional Information

No response

[elahehrashedi]

preferredGeneratorName is originally being determined in this function:

vscode-cmake-tools/src/cmakeProject.ts

Lines 558 to 572 in 01c4796

	private getPreferredGenerators(): CMakeGenerator[] {
	// User can override generator with a setting
	const userGenerator = this.workspaceContext.config.generator;
	if (userGenerator) {
	log.debug(localize('using.user.generator', 'Using generator from user configuration: {0}', userGenerator));
	return [{
	name: userGenerator,
	platform: this.workspaceContext.config.platform || undefined,
	toolset: this.workspaceContext.config.toolset || undefined
	}];
	}
	const userPreferred = this.workspaceContext.config.preferredGenerators.map(g => ({ name: g }));
	return userPreferred;
	}

Which searches for alternatives in user configuration. If it is still not defined, the extension will uses Ninja as the last alternative.

[bobbrow]

What should be happening with presets is that if the generator is not defined somewhere in the inheritance, the preset should be considered invalid and not available for use. In that case, we shouldn't be hitting any fallback code.

Are you seeing incorrect behavior done by the extension? And if so, can you share your repro?

[jochil]

Thanks for your answers!

if the generator is not defined somewhere in the inheritance, the preset should be considered invalid and not available for use

I think this is not correct, at least from what I got from the CMake docs. The generator field is optional...if it is missing and no generator is defined in the preset inheritance it is on the cmake command to figure out the generator.
For example, if I run these presets https://github.com/CodeIntelligenceTesting/cifuzz/blob/main/tools/cmake/cifuzz/share/CMakePresets.json on my command line (cmake --preset="cifuzz (Fuzzing)" or cmake --build --preset="cifuzz (Fuzzing)" my system uses Make as a generator. If I run the presets in the exact same project with the extension, the following command gets generated:

[proc] Executing command: /usr/bin/cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -DCIFUZZ_ENGINE=libfuzzer "-DCIFUZZ_SANITIZERS=address;undefined" -DCIFUZZ_TESTING:BOOL=ON -DCMAKE_BUILD_RPATH_USE_ORIGIN:BOOL=ON -S/home/jochen/dev/workspace/ci/cifuzz/examples/cmake -B/home/jochen/dev/workspace/ci/cifuzz/examples/cmake/.cifuzz-build/libfuzzer/address+undefined -G Ninja

... which explicitly uses -G Ninja.
Without the -G parameter it would behave exactly like executing the preset directly via cmake, which IMO should be the goal here

[bobbrow]

I think this is not correct, at least from what I got from the CMake docs. The generator field is optional...if it is missing and no generator is defined in the preset inheritance it is on the cmake command to figure out the generator.

Sorry, I mispoke. You are correct. The behavior changed in v3. We might not have fixed the extension to deal with that yet. I don't believe the extension has a path to correctly handle unset generators yet either. For the time being we recommend you set it explicitly if possible.

[jochil]

Sorry, I mispoke. You are correct. The behavior changed in v3. We might not have fixed the extension to deal with that yet. I don't believe the extension has a path to correctly handle unset generators yet either. For the time being we recommend you set it explicitly if possible.

Thanks... for my personal workflow, setting it explicitly would be fine, but we are recommending this extension in our project at https://github.com/CodeIntelligenceTesting/cifuzz and would like having consistent behavior for our users. Long story short: Would you prefer handling the issue yourself or would it be fine for you if I start working on a PR?

[bobbrow]

We would accept a PR if you are willing to work on this.

[jochil]

@bobbrow I am working on the PR (main...jochil:vscode-cmake-tools:v3-preset-default-generator) but I have a few questions:

1. The unit tests are not running on my system... I get a lot of this Error: Error starting up cmake-server. Are there any instructions on how to setup the test environment?
2. What way would you prefer to cover this change with a test?
3. I only found the default generator behavior for configure presets, it seems like for build/test presets it isn't handled... Am I right or am I overseeing something?

[bobbrow]

1. Sorry about that. We need to write a testing guide. To make the cmake-server tests work, you need to install cmake version 3.18.2 and put it in the path ahead of your normal version of cmake
2. The first place that comes to mind would be adding another preset to test/extension-tests/single-root-UI/project-folder/CMakeUserPresets.json and adding a test case in test/extension-tests/single-root-UI/test/configure-and-build-presets.test.ts. I think that would be sufficient.
3. I'm not sure I understand the question. The build presets shouldn't need a generator. Are you saying that a build preset linked to a configure preset that doesn't have a generator won't work? Does it work in the terminal, but not the extension?

[jochil]

@bobbrow thanks for the answers... with 3. I was wrong of course, no need for a generator here 🙂
I created the PR: #2748

Thanks a lot for the support

[bobbrow]

Awesome! Thanks for the help! We'll try to take a look within the next few days.