Skip to content

Commit

Permalink
Refactor CMake build settings
Browse files Browse the repository at this point in the history
Add support for all generators to CMake build settings

Correct command line option for CMake's --warn-unused-vars

Rename clean command to clean target to better reflect its use as
the argument passed to cmake's --target command line.

Add all target for the argument passed to cmake's --target
command line when doing a normal build.

Clarify usage of UI overrides and change the UI to be "use defaults"
(i.e. invert the checkbox). This is a **breaking** change as it means
user projects that were using UI overrides will revert to using defaults.
This is done on purpose as so many little things have changed in CMake
settings, that reverting to defaults on upgrade seems like a logical
decision. In addition *use defaults* matches the other GUIs in Eclipse,
for example the MBS build command settings.

Populate all default in getDefaultProperties() so that all CMake build
settings are displayed as used (greyed out) and can be used as a starting
point when editing settings.

Simplify some of the code in CMakeBuildTab, especially:
- removing the duplicated code between initializeFrom and
restoreProperties
- use Map's getOrDefault instead of a get, followed by a null check.

Remove getUseDefaultCommand/setUseDefaultCommand as they don't appear
in the GUI and it makes it impossible for the GUI command to be used.
It is also redundant as use ui overrides is a global use default for
all settings, so having a second use default seems unneeded at this point.

Cleanup and simplify getCMakeProperties.

Fix parsing of extra args so that quoted strings work.

Remove doubled-up extra args added to command line.

Refactored manual tests document and brought it up to date.

Fixes #1055
Part of #1000
  • Loading branch information
jonahgraham committed Jan 24, 2025
1 parent 1218066 commit 50cd03a
Show file tree
Hide file tree
Showing 15 changed files with 570 additions and 319 deletions.
10 changes: 8 additions & 2 deletions NewAndNoteworthy/CDT-12.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,19 @@ CDT's native components will likely work with older versions of glibc too, assum

# Core Build

## More CMake build settings are now available in the user interface

The CMake build setting GUI has been updated to include more CMake settings, and some of the settings that did not used to do the correct thing have been updated for more consistent behavior.
The way these settings are saved has been slightly modified, meaning workspaces with CMake projects from before CDT 12 will have their build settings restored to defaults.
Build settings can be customized by unchecking "Use default CMake settings".

TODO: Before release add the final screenshot for the build settings here. I am not including it now because the UI keeps changing.

## Default build system generator for CMake changed to Ninja on all platforms

The default for CMake's build system generator is now Ninja on all platforms.
Users who want to use other build system generators can select their desired generator in the build settings.

TODO: Before release add the final screenshot for the build settings here. I am not including it now because the UI keeps changing.

# Managed Build

## New *C Project* and new *C++ Project* available via *New C/C++ Project* wizard
Expand Down
1 change: 1 addition & 0 deletions NewAndNoteworthy/CHANGELOG-API.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ The following classes have been removed or modified in API breaking ways:
- new methods added to compensate for removal of IOsOverrides
- reset method removed
- spelling corrected for methods with Uninitialized in the name
- setWarnUnused renamed to setWarnUnusedVars and isWarnUnused renamed to isWarnUnusedVars


## API Changes in CDT 11.5.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand All @@ -22,12 +23,14 @@
import java.util.Map;

import org.eclipse.cdt.cmake.core.properties.CMakeGenerator;
import org.eclipse.cdt.cmake.core.properties.ICMakeGenerator;
import org.eclipse.cdt.cmake.core.properties.ICMakeProperties;
import org.eclipse.cdt.core.CCProjectNature;
import org.eclipse.cdt.core.CProjectNature;
import org.eclipse.cdt.core.build.IToolChain;
import org.eclipse.cdt.core.testplugin.ResourceHelper;
import org.eclipse.cdt.core.testplugin.util.BaseTestCase5;
import org.eclipse.cdt.utils.CommandLineUtil;
import org.eclipse.core.resources.IBuildConfiguration;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IProjectDescription;
Expand Down Expand Up @@ -118,7 +121,6 @@ public void getDefaultProperties() throws Exception {
public Map<String, String> getDefaultProperties() {
var defs = new HashMap<>(super.getDefaultProperties());
defs.put(CMAKE_GENERATOR, CMakeGenerator.WatcomWMake.getCMakeName());
defs.put(CMAKE_USE_UI_OVERRIDES, "true");
return defs;
}
};
Expand All @@ -127,6 +129,163 @@ public Map<String, String> getDefaultProperties() {
assertThat(cMakeProperties.getGenerator(), is(CMakeGenerator.WatcomWMake));
}

@Test
public void getDefaultPropertiesTestExtraArgs() throws Exception {
// Create a C Build Configuration using the default build config and an arbitrary name
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
mockToolchain) {
@Override
public Map<String, String> getDefaultProperties() {
var defs = new HashMap<>(super.getDefaultProperties());
defs.put(CMAKE_ARGUMENTS, "-Dtest0=0 -Dtest1=1");
return defs;
}
};
// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();
List<String> extraArguments = cMakeProperties.getExtraArguments();
assertThat(extraArguments, contains("-Dtest0=0", "-Dtest1=1"));
}

/**
* Test that a custom cmake generator can be entered and auto-created
*/
@Test
public void customCMakeGeneratorEntryAuto() throws Exception {
// Create a C Build Configuration using the default build config and an arbitrary name
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
mockToolchain) {
@Override
public Map<String, String> getDefaultProperties() {
var defs = new HashMap<>(super.getDefaultProperties());
// A custom generator for a custom cmake version
defs.put(CMAKE_GENERATOR, "My Personal Generator");
return defs;
}
};

// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();
assertThat(cMakeProperties.getGenerator().getCMakeName(), is("My Personal Generator"));
assertThat(cMakeProperties.getGenerator().getIgnoreErrOption(), is(nullValue()));
assertThat(cMakeProperties.getGenerator().getMakefileName(), is(nullValue()));
}

/**
* Test that a custom cmake generator can be entered and manually-created
*/
@Test
public void customCMakeGeneratorEntryManual() throws Exception {
// Create a C Build Configuration using the default build config and an arbitrary name
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
mockToolchain) {
@Override
public Map<String, String> getDefaultProperties() {
var defs = new HashMap<>(super.getDefaultProperties());
// A custom generator for a custom cmake version
defs.put(CMAKE_GENERATOR, "My Personal Generator");
return defs;
}

@Override
public ICMakeProperties getCMakeProperties() {
ICMakeProperties properties = super.getCMakeProperties();
if ("My Personal Generator".equals(properties.getGenerator().getCMakeName())) {
var generator = new ICMakeGenerator() {
@Override
public String getMakefileName() {
return "MyMak.mak";
}

@Override
public String getIgnoreErrOption() {
return "-mycustom";
}

@Override
public String getCMakeName() {
return "My Personal Generator";
}
};
properties.setGenerator(generator);
}
return properties;
}
};

// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();
assertThat(cMakeProperties.getGenerator().getCMakeName(), is("My Personal Generator"));
assertThat(cMakeProperties.getGenerator().getIgnoreErrOption(), is("-mycustom"));
assertThat(cMakeProperties.getGenerator().getMakefileName(), is("MyMak.mak"));
}

/**
* Test all and clean targets and cmake command have working defaults
*/
@Test
public void targetsAndCommandDefaults() throws Exception {
// Create a C Build Configuration using the default build config and an arbitrary name
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
mockToolchain);

// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();
assertThat(cMakeProperties.getCommand(), is("cmake"));
assertThat(cMakeProperties.getAllTarget(), is("all"));
assertThat(cMakeProperties.getCleanTarget(), is("clean"));
}

/**
* Test all and clean targets and cmake command can be overridden
*/
@Test
public void targetsAndCommand() throws Exception {
// Create a C Build Configuration using the default build config and an arbitrary name
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
mockToolchain) {
@Override
public Map<String, String> getDefaultProperties() {
var defs = new HashMap<>(super.getDefaultProperties());
defs.put(CMAKE_BUILD_COMMAND, "mycmake");
defs.put(CMAKE_ALL_TARGET, "myall");
defs.put(CMAKE_CLEAN_TARGET, "myclean");
return defs;
}
};

// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();
assertThat(cMakeProperties.getCommand(), is("mycmake"));
assertThat(cMakeProperties.getAllTarget(), is("myall"));
assertThat(cMakeProperties.getCleanTarget(), is("myclean"));
}

/**
* Test that extra arguments parse correctly, e.g. handles ".
*
* Note that this test is minimal here as the real functionality is in {@link CommandLineUtil}
* and all the special cases are tested in CommandLineUtilTest.
*/
@Test
public void extraArgumentsParseCorrectly() throws Exception {
// Create a C Build Configuration using the default build config and an arbitrary name
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
mockToolchain) {
@Override
public Map<String, String> getDefaultProperties() {
var defs = new HashMap<>(super.getDefaultProperties());
defs.put(CMAKE_ARGUMENTS, "-Da=\"something with space and quotes\" \"-Danother=quoted\"");
return defs;
}
};

// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();
assertThat(cMakeProperties.getExtraArguments(),
is(List.of("-Da=something with space and quotes", "-Danother=quoted")));
}

private IProject createCMakeProject() throws Exception {
// Create a plain Eclipse project
IProject project = ResourceHelper.createProject(this.getName());
Expand Down
Loading

0 comments on commit 50cd03a

Please sign in to comment.