Skip to content

Commit 50cd03a

Browse files
committed
Refactor CMake build settings
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 eclipse-cdt#1055 Part of eclipse-cdt#1000
1 parent 1218066 commit 50cd03a

File tree

15 files changed

+570
-319
lines changed

15 files changed

+570
-319
lines changed

NewAndNoteworthy/CDT-12.0.md

+8-2
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,19 @@ CDT's native components will likely work with older versions of glibc too, assum
1414

1515
# Core Build
1616

17+
## More CMake build settings are now available in the user interface
18+
19+
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.
20+
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.
21+
Build settings can be customized by unchecking "Use default CMake settings".
22+
23+
TODO: Before release add the final screenshot for the build settings here. I am not including it now because the UI keeps changing.
24+
1725
## Default build system generator for CMake changed to Ninja on all platforms
1826

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

22-
TODO: Before release add the final screenshot for the build settings here. I am not including it now because the UI keeps changing.
23-
2430
# Managed Build
2531

2632
## New *C Project* and new *C++ Project* available via *New C/C++ Project* wizard

NewAndNoteworthy/CHANGELOG-API.md

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ The following classes have been removed or modified in API breaking ways:
2222
- new methods added to compensate for removal of IOsOverrides
2323
- reset method removed
2424
- spelling corrected for methods with Uninitialized in the name
25+
- setWarnUnused renamed to setWarnUnusedVars and isWarnUnused renamed to isWarnUnusedVars
2526

2627

2728
## API Changes in CDT 11.5.

cmake/org.eclipse.cdt.cmake.core.tests/src/org/eclipse/cdt/cmake/core/CMakeBuildConfigurationTests.java

+160-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import static org.hamcrest.MatcherAssert.assertThat;
1414
import static org.hamcrest.Matchers.contains;
1515
import static org.hamcrest.Matchers.is;
16+
import static org.hamcrest.Matchers.nullValue;
1617
import static org.mockito.Mockito.mock;
1718
import static org.mockito.Mockito.when;
1819

@@ -22,12 +23,14 @@
2223
import java.util.Map;
2324

2425
import org.eclipse.cdt.cmake.core.properties.CMakeGenerator;
26+
import org.eclipse.cdt.cmake.core.properties.ICMakeGenerator;
2527
import org.eclipse.cdt.cmake.core.properties.ICMakeProperties;
2628
import org.eclipse.cdt.core.CCProjectNature;
2729
import org.eclipse.cdt.core.CProjectNature;
2830
import org.eclipse.cdt.core.build.IToolChain;
2931
import org.eclipse.cdt.core.testplugin.ResourceHelper;
3032
import org.eclipse.cdt.core.testplugin.util.BaseTestCase5;
33+
import org.eclipse.cdt.utils.CommandLineUtil;
3134
import org.eclipse.core.resources.IBuildConfiguration;
3235
import org.eclipse.core.resources.IProject;
3336
import org.eclipse.core.resources.IProjectDescription;
@@ -118,7 +121,6 @@ public void getDefaultProperties() throws Exception {
118121
public Map<String, String> getDefaultProperties() {
119122
var defs = new HashMap<>(super.getDefaultProperties());
120123
defs.put(CMAKE_GENERATOR, CMakeGenerator.WatcomWMake.getCMakeName());
121-
defs.put(CMAKE_USE_UI_OVERRIDES, "true");
122124
return defs;
123125
}
124126
};
@@ -127,6 +129,163 @@ public Map<String, String> getDefaultProperties() {
127129
assertThat(cMakeProperties.getGenerator(), is(CMakeGenerator.WatcomWMake));
128130
}
129131

132+
@Test
133+
public void getDefaultPropertiesTestExtraArgs() throws Exception {
134+
// Create a C Build Configuration using the default build config and an arbitrary name
135+
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
136+
mockToolchain) {
137+
@Override
138+
public Map<String, String> getDefaultProperties() {
139+
var defs = new HashMap<>(super.getDefaultProperties());
140+
defs.put(CMAKE_ARGUMENTS, "-Dtest0=0 -Dtest1=1");
141+
return defs;
142+
}
143+
};
144+
// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
145+
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();
146+
List<String> extraArguments = cMakeProperties.getExtraArguments();
147+
assertThat(extraArguments, contains("-Dtest0=0", "-Dtest1=1"));
148+
}
149+
150+
/**
151+
* Test that a custom cmake generator can be entered and auto-created
152+
*/
153+
@Test
154+
public void customCMakeGeneratorEntryAuto() throws Exception {
155+
// Create a C Build Configuration using the default build config and an arbitrary name
156+
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
157+
mockToolchain) {
158+
@Override
159+
public Map<String, String> getDefaultProperties() {
160+
var defs = new HashMap<>(super.getDefaultProperties());
161+
// A custom generator for a custom cmake version
162+
defs.put(CMAKE_GENERATOR, "My Personal Generator");
163+
return defs;
164+
}
165+
};
166+
167+
// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
168+
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();
169+
assertThat(cMakeProperties.getGenerator().getCMakeName(), is("My Personal Generator"));
170+
assertThat(cMakeProperties.getGenerator().getIgnoreErrOption(), is(nullValue()));
171+
assertThat(cMakeProperties.getGenerator().getMakefileName(), is(nullValue()));
172+
}
173+
174+
/**
175+
* Test that a custom cmake generator can be entered and manually-created
176+
*/
177+
@Test
178+
public void customCMakeGeneratorEntryManual() throws Exception {
179+
// Create a C Build Configuration using the default build config and an arbitrary name
180+
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
181+
mockToolchain) {
182+
@Override
183+
public Map<String, String> getDefaultProperties() {
184+
var defs = new HashMap<>(super.getDefaultProperties());
185+
// A custom generator for a custom cmake version
186+
defs.put(CMAKE_GENERATOR, "My Personal Generator");
187+
return defs;
188+
}
189+
190+
@Override
191+
public ICMakeProperties getCMakeProperties() {
192+
ICMakeProperties properties = super.getCMakeProperties();
193+
if ("My Personal Generator".equals(properties.getGenerator().getCMakeName())) {
194+
var generator = new ICMakeGenerator() {
195+
@Override
196+
public String getMakefileName() {
197+
return "MyMak.mak";
198+
}
199+
200+
@Override
201+
public String getIgnoreErrOption() {
202+
return "-mycustom";
203+
}
204+
205+
@Override
206+
public String getCMakeName() {
207+
return "My Personal Generator";
208+
}
209+
};
210+
properties.setGenerator(generator);
211+
}
212+
return properties;
213+
}
214+
};
215+
216+
// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
217+
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();
218+
assertThat(cMakeProperties.getGenerator().getCMakeName(), is("My Personal Generator"));
219+
assertThat(cMakeProperties.getGenerator().getIgnoreErrOption(), is("-mycustom"));
220+
assertThat(cMakeProperties.getGenerator().getMakefileName(), is("MyMak.mak"));
221+
}
222+
223+
/**
224+
* Test all and clean targets and cmake command have working defaults
225+
*/
226+
@Test
227+
public void targetsAndCommandDefaults() throws Exception {
228+
// Create a C Build Configuration using the default build config and an arbitrary name
229+
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
230+
mockToolchain);
231+
232+
// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
233+
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();
234+
assertThat(cMakeProperties.getCommand(), is("cmake"));
235+
assertThat(cMakeProperties.getAllTarget(), is("all"));
236+
assertThat(cMakeProperties.getCleanTarget(), is("clean"));
237+
}
238+
239+
/**
240+
* Test all and clean targets and cmake command can be overridden
241+
*/
242+
@Test
243+
public void targetsAndCommand() throws Exception {
244+
// Create a C Build Configuration using the default build config and an arbitrary name
245+
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
246+
mockToolchain) {
247+
@Override
248+
public Map<String, String> getDefaultProperties() {
249+
var defs = new HashMap<>(super.getDefaultProperties());
250+
defs.put(CMAKE_BUILD_COMMAND, "mycmake");
251+
defs.put(CMAKE_ALL_TARGET, "myall");
252+
defs.put(CMAKE_CLEAN_TARGET, "myclean");
253+
return defs;
254+
}
255+
};
256+
257+
// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
258+
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();
259+
assertThat(cMakeProperties.getCommand(), is("mycmake"));
260+
assertThat(cMakeProperties.getAllTarget(), is("myall"));
261+
assertThat(cMakeProperties.getCleanTarget(), is("myclean"));
262+
}
263+
264+
/**
265+
* Test that extra arguments parse correctly, e.g. handles ".
266+
*
267+
* Note that this test is minimal here as the real functionality is in {@link CommandLineUtil}
268+
* and all the special cases are tested in CommandLineUtilTest.
269+
*/
270+
@Test
271+
public void extraArgumentsParseCorrectly() throws Exception {
272+
// Create a C Build Configuration using the default build config and an arbitrary name
273+
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
274+
mockToolchain) {
275+
@Override
276+
public Map<String, String> getDefaultProperties() {
277+
var defs = new HashMap<>(super.getDefaultProperties());
278+
defs.put(CMAKE_ARGUMENTS, "-Da=\"something with space and quotes\" \"-Danother=quoted\"");
279+
return defs;
280+
}
281+
};
282+
283+
// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
284+
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();
285+
assertThat(cMakeProperties.getExtraArguments(),
286+
is(List.of("-Da=something with space and quotes", "-Danother=quoted")));
287+
}
288+
130289
private IProject createCMakeProject() throws Exception {
131290
// Create a plain Eclipse project
132291
IProject project = ResourceHelper.createProject(this.getName());

0 commit comments

Comments
 (0)