-
Notifications
You must be signed in to change notification settings - Fork 206
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 API to set CMake generator default (eg Ninja) #1046
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,7 @@ | |
import org.eclipse.cdt.cmake.core.internal.CommandDescriptorBuilder.CommandDescriptor; | ||
import org.eclipse.cdt.cmake.core.internal.IOsOverridesSelector; | ||
import org.eclipse.cdt.cmake.core.properties.CMakeGenerator; | ||
import org.eclipse.cdt.cmake.core.properties.CMakePropertiesFactory; | ||
import org.eclipse.cdt.cmake.core.properties.ICMakeProperties; | ||
import org.eclipse.cdt.cmake.core.properties.ICMakePropertiesController; | ||
import org.eclipse.cdt.cmake.core.properties.IOsOverrides; | ||
|
@@ -72,10 +73,11 @@ | |
import org.osgi.service.prefs.Preferences; | ||
|
||
/** | ||
* @since 1.6 | ||
* @since 2.0 | ||
*/ | ||
public class CMakeBuildConfiguration extends CBuildConfiguration { | ||
public class CMakeBuildConfiguration extends CBuildConfiguration implements ICMakeBuildConfiguration { | ||
|
||
private static final String CMAKE_PROPERTIES_INITIALSED = "cmake.properties.initialised"; //$NON-NLS-1$ | ||
public static final String CMAKE_USE_UI_OVERRIDES = "cmake.use.ui.overrides"; //$NON-NLS-1$ | ||
public static final boolean CMAKE_USE_UI_OVERRIDES_DEFAULT = false; | ||
public static final String CMAKE_GENERATOR = "cmake.generator"; //$NON-NLS-1$ | ||
|
@@ -91,6 +93,8 @@ public class CMakeBuildConfiguration extends CBuildConfiguration { | |
// lazily instantiated.. | ||
private ICMakePropertiesController pc; | ||
|
||
ICMakeProperties defaultProperties; | ||
|
||
private Map<IResource, IScannerInfo> infoPerResource; | ||
/** | ||
* whether one of the CMakeLists.txt files in the project has been modified and saved by the | ||
|
@@ -112,6 +116,7 @@ public CMakeBuildConfiguration(IBuildConfiguration config, String name) throws C | |
|
||
ICMakeToolChainManager manager = Activator.getService(ICMakeToolChainManager.class); | ||
toolChainFile = manager.getToolChainFileFor(getToolChain()); | ||
setDefaultCMakeProperties(); | ||
} | ||
|
||
public CMakeBuildConfiguration(IBuildConfiguration config, String name, IToolChain toolChain) { | ||
|
@@ -122,6 +127,30 @@ public CMakeBuildConfiguration(IBuildConfiguration config, String name, IToolCha | |
ICMakeToolChainFile toolChainFile, String launchMode) { | ||
super(config, name, toolChain, launchMode); | ||
this.toolChainFile = toolChainFile; | ||
// // moule | ||
setDefaultCMakeProperties(); | ||
} | ||
|
||
private void setDefaultCMakeProperties() { | ||
// Only set defaults on initial construction | ||
if (getProperty(CMAKE_PROPERTIES_INITIALSED) == null) { | ||
ICMakeProperties properties = CMakePropertiesFactory.getProperties(); | ||
IOsOverrides windowsOverrides = properties.getWindowsOverrides(); | ||
windowsOverrides.setDefaultGenerator(CMakeGenerator.UnixMakefiles); | ||
|
||
IOsOverrides linuxOverrides = properties.getLinuxOverrides(); | ||
linuxOverrides.setDefaultGenerator(CMakeGenerator.UnixMakefiles); | ||
// reset(true) causes the CMake generator to be reset to it's default value. | ||
properties.reset(true); | ||
setDefaultCMakeProperties(properties); | ||
} | ||
} | ||
|
||
@Override | ||
public void setDefaultCMakeProperties(ICMakeProperties defaultProperties) { | ||
this.defaultProperties = defaultProperties; | ||
setProperty(CMAKE_GENERATOR, getOsOverrides(defaultProperties).getGenerator().getCMakeName()); | ||
setProperty(CMAKE_PROPERTIES_INITIALSED, Boolean.TRUE.toString()); | ||
} | ||
|
||
/** | ||
|
@@ -162,7 +191,8 @@ public IProject[] build(int kind, Map<String, String> args, IConsole console, IP | |
runCMake = true; | ||
} | ||
|
||
ICMakeProperties cmakeProperties = getPropertiesController().load(); | ||
// ICMakeProperties cmakeProperties = getPropertiesController().load(); | ||
ICMakeProperties cmakeProperties = getPropertiesController().load(defaultProperties); | ||
runCMake |= !Files.exists(buildDir.resolve("CMakeCache.txt")); //$NON-NLS-1$ | ||
|
||
// Causes CMAKE_BUILD_TYPE to be set according to the launch mode | ||
|
@@ -303,7 +333,8 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti | |
|
||
project.deleteMarkers(ICModelMarker.C_MODEL_PROBLEM_MARKER, false, IResource.DEPTH_INFINITE); | ||
|
||
ICMakeProperties cmakeProperties = getPropertiesController().load(); | ||
// ICMakeProperties cmakeProperties = getPropertiesController().load(); | ||
ICMakeProperties cmakeProperties = getPropertiesController().load(defaultProperties); | ||
CommandDescriptorBuilder cmdBuilder = new CommandDescriptorBuilder(cmakeProperties, | ||
new SimpleOsOverridesSelector()); | ||
CommandDescriptor command = cmdBuilder.makeCMakeBuildCommandline(getCleanCommand()); | ||
|
@@ -405,7 +436,8 @@ private ICMakePropertiesController getPropertiesController() { | |
@SuppressWarnings("unchecked") | ||
public <T> T getAdapter(Class<T> adapter) { | ||
if (ICMakePropertiesController.class.equals(adapter)) { | ||
return (T) pc; | ||
// return (T) pc; | ||
return (T) getPropertiesController(); | ||
} | ||
return super.getAdapter(adapter); | ||
} | ||
|
@@ -582,6 +614,15 @@ public void shutdown() { | |
} | ||
} // CMakeIndexerInfoConsumer | ||
|
||
private IOsOverrides getOsOverrides(ICMakeProperties cmakeProperties) { | ||
if (Platform.OS_WIN32.equals(Platform.getOS())) { | ||
return cmakeProperties.getWindowsOverrides(); | ||
|
||
} else { | ||
return cmakeProperties.getLinuxOverrides(); | ||
} | ||
} | ||
|
||
/** | ||
* Supports OS overrides and also User Interface (UI) overrides, provided in the CMakeBuildTab. The UI | ||
* overrides are stored in the intermediary CBuildConfiguration Preferences (CBuildConfiguration.getSettings()) | ||
|
@@ -591,17 +632,9 @@ private class SimpleOsOverridesSelector implements IOsOverridesSelector { | |
|
||
@Override | ||
public IOsOverrides getOsOverrides(ICMakeProperties cmakeProperties) { | ||
IOsOverrides overrides; | ||
// get overrides. Simplistic approach ATM, probably a strategy might fit better. | ||
// see comment in CMakeIndexerInfoConsumer#getFileForCMakePath() | ||
final String os = Platform.getOS(); | ||
if (Platform.OS_WIN32.equals(os)) { | ||
overrides = cmakeProperties.getWindowsOverrides(); | ||
} else { | ||
// fall back to linux, if OS is unknown | ||
overrides = cmakeProperties.getLinuxOverrides(); | ||
} | ||
return getUiOrOsOverrides(overrides); | ||
return getUiOrOsOverrides(getOsOverrides(cmakeProperties)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately this change gave me a stack overflow. I created a new CMake project and hit build (in the launch bar) and I got: !ENTRY org.eclipse.core.jobs 4 2 2025-01-19 10:41:21.640
!MESSAGE An internal error occurred during: "Building Active Configuration".
!STACK 0
java.lang.StackOverflowError
at org.eclipse.cdt.cmake.core.CMakeBuildConfiguration$SimpleOsOverridesSelector.getOsOverrides(CMakeBuildConfiguration.java:637)
at org.eclipse.cdt.cmake.core.CMakeBuildConfiguration$SimpleOsOverridesSelector.getOsOverrides(CMakeBuildConfiguration.java:637)
at org.eclipse.cdt.cmake.core.CMakeBuildConfiguration$SimpleOsOverridesSelector.getOsOverrides(CMakeBuildConfiguration.java:637)
-- repeating -- There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apologies, that was some clumsy refactoring last thing on a Friday when I needed to finish work. |
||
} | ||
|
||
private IOsOverrides getUiOrOsOverrides(IOsOverrides osOverrides) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package org.eclipse.cdt.cmake.core; | ||
|
||
import org.eclipse.cdt.cmake.core.properties.ICMakeProperties; | ||
import org.eclipse.cdt.core.build.ICBuildConfiguration; | ||
|
||
/** | ||
* @since 2.0 | ||
*/ | ||
public interface ICMakeBuildConfiguration { | ||
/** | ||
* Provides a CBuildConfiguration with a set of default CMake properties. The ICMakeProperties can then, for | ||
* example, provide a default value for the CMake generator. | ||
* The CMake generator can then be set using {@link ICBuildConfiguration#setProperty(String, String)}, using | ||
* {@link CMakeBuildConfiguration#CMAKE_GENERATOR}. The CMake generator property can then be accessed using | ||
* the {@link ICBuildConfiguration#getProperty(String)}. | ||
*/ | ||
void setDefaultCMakeProperties(ICMakeProperties defaultProperties); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package org.eclipse.cdt.cmake.core.properties; | ||
|
||
import org.eclipse.cdt.cmake.core.internal.properties.CMakePropertiesBean; | ||
|
||
public class CMakePropertiesFactory { | ||
public static ICMakeProperties getProperties() { | ||
return new CMakePropertiesBean(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't ok - you shouldn't have non-final methods called by constructor.
Can you change this around so that the interface has
getDefaultCMakeProperties
that extenders can override if they don't like the built-in defaults?Here is a diff showing what I mean, but it isn't tested, so may require fixing up
expand for patch contents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addendum,
extenders can do something like this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And finally - maybe ;) - I think calling:
from within this method is incorrect as that method has side effects by changing project preferences too. It may be best to call
setProperty
on the return value ofgetDefaultCMakeProperties
:and then in build/clean: