Skip to content

Commit

Permalink
Remove IOsOverrides and related code
Browse files Browse the repository at this point in the history 
IOsOverrides was a partial implementation to achieve builds in Docker
containers, however the work was not complete and it the extra code
was complicating some basic use cases of setting defaults

This is an API breaking change and the changelog has been updated with
all the API changes in and around ICMakeProperties, including fixing
typos in WarnUninitialized methods.

This commit should be squashed into the parent commit, it is kept
temporarily separate for review reasons.
  • Loading branch information
@jonahgraham
jonahgraham committed Jan 23, 2025
1 parent afb8c41 commit 1218066
Show file tree
Hide file tree
Showing 16 changed files with 178 additions and 731 deletions.
9 changes: 9 additions & 0 deletions NewAndNoteworthy/CDT-12.0.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,15 @@ The minimum version of GLIBC required is now 2.31.
This version can be found in Ubuntu 20.04 and later, RHEL 9.0 and later and other distros as well.
CDT's native components will likely work with older versions of glibc too, assuming they provide the required APIs for Eclipse CDT.

# Core Build

## 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
15 changes: 13 additions & 2 deletions NewAndNoteworthy/CHANGELOG-API.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,20 @@ This section describes API removals that occurred in past releases, and upcoming
Below is the detailed descriptions of API changes and mitigation efforts API consumers need to take.

## API Changes in CDT 12.0.
### org.eclipse.cdt.cmake.core.properties.ICMakePropertiesController removed

The interface ICMakePropertiesController provided load and save methods; only the load method was called in CDT and not the save method, because of a partially implemented feature.
### org.eclipse.cdt.cmake.core.properties refactored

A significant simplification to the CMake build properties was completed, this included removing some API that was not used.
The following classes have been removed or modified in API breaking ways:

- org.eclipse.cdt.cmake.core.properties.ICMakePropertiesController removed
- org.eclipse.cdt.cmake.core.properties.IGeneralProperties removed
- org.eclipse.cdt.cmake.core.properties.IOsOverrides removed
- org.eclipse.cdt.cmake.core.properties.ICMakeProperties:
- new methods added to compensate for removal of IOsOverrides
- reset method removed
- spelling corrected for methods with Uninitialized in the name


## API Changes in CDT 11.5.

Expand Down
122 changes: 27 additions & 95 deletions ...pse.cdt.cmake.core.tests/src/org/eclipse/cdt/cmake/core/CMakeBuildConfigurationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@
import static org.mockito.Mockito.when;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import org.eclipse.cdt.cmake.core.properties.CMakeGenerator;
import org.eclipse.cdt.cmake.core.properties.ICMakeProperties;
import org.eclipse.cdt.cmake.core.properties.IOsOverrides;
import org.eclipse.cdt.core.CCProjectNature;
import org.eclipse.cdt.core.CProjectNature;
import org.eclipse.cdt.core.build.IToolChain;
Expand Down Expand Up @@ -57,86 +58,38 @@ public void setup() throws Exception {
}

/**
* Test for {@link IOsOverrides#setGenerator()}.
* Test for {@link ICMakeProperties#setGenerator()}.
*
* This test also verifies that what the ISV overrides in getCMakeProperties is what takes effect.
*/
@Test
public void getCMakePropertiesTestSetGenerator() throws Exception {
CMakeBuildConfigurationExtended cmBuildConfig = new CMakeBuildConfigurationExtended(buildConfig,
"cmBuildConfigName", mockToolchain) {
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
mockToolchain) {

@Override
public ICMakeProperties getCMakeProperties() {
ICMakeProperties properties = super.getCMakeProperties();

IOsOverrides windowsOverrides = properties.getWindowsOverrides();
windowsOverrides.setGenerator(CMakeGenerator.NMakeMakefiles);
IOsOverrides linuxOverrides = properties.getLinuxOverrides();
linuxOverrides.setGenerator(CMakeGenerator.UnixMakefiles);
return properties;
}
};

// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();

// Get overrides for Windows host and check the default value for getGenerator.
IOsOverrides windowsOverrides = cMakeProperties.getWindowsOverrides();
CMakeGenerator winGenerator = windowsOverrides.getGenerator();
assertThat(winGenerator, is(CMakeGenerator.NMakeMakefiles));

// Get overrides for Linux host and check the default value for getGenerator.
IOsOverrides linuxOverrides = cMakeProperties.getLinuxOverrides();
CMakeGenerator linuxGenerator = linuxOverrides.getGenerator();
assertThat(linuxGenerator, is(CMakeGenerator.UnixMakefiles));
}

/**
* Test for {@link IOsOverrides#setDefaultGenerator()}. Also tests that {@link ICMakeProperties#reset(boolean)} works as expected.
*/
@Test
public void getCMakePropertiesTestSetDefaultGenerator() throws Exception {
CMakeBuildConfigurationExtended cmBuildConfig = new CMakeBuildConfigurationExtended(buildConfig,
"cmBuildConfigName", mockToolchain) {

@Override
public ICMakeProperties getCMakeProperties() {
ICMakeProperties properties = super.getCMakeProperties();

IOsOverrides windowsOverrides = properties.getWindowsOverrides();
windowsOverrides.setDefaultGenerator(CMakeGenerator.NMakeMakefiles);
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);
properties.setGenerator(CMakeGenerator.WatcomWMake);
return properties;
}
};

// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();

// Get overrides for Windows host and check the default value for getGenerator.
IOsOverrides windowsOverrides = cMakeProperties.getWindowsOverrides();
CMakeGenerator winGenerator = windowsOverrides.getGenerator();
assertThat(winGenerator, is(CMakeGenerator.NMakeMakefiles));

// Get overrides for Linux host and check the default value for getGenerator.
IOsOverrides linuxOverrides = cMakeProperties.getLinuxOverrides();
CMakeGenerator linuxGenerator = linuxOverrides.getGenerator();
assertThat(linuxGenerator, is(CMakeGenerator.UnixMakefiles));
assertThat(cMakeProperties.getGenerator(), is(CMakeGenerator.WatcomWMake));
}

/**
* Test for {@link ICMakeProperties#setExtraArguments()}
* This is a different extraArguments to IOsOverrides#setExtraArguments().
* Presumably ICMakeProperties#setExtraArguments() are platform agnostic extra arguments, where as
* IOsOverrides#setExtraArguments() can be set different for Linux and Windows.
*
* This test also verifies that what the ISV overrides in getCMakeProperties is what takes effect.
*/
@Test
public void getCMakePropertiesTestSetExtraArguments() throws Exception {
// Create a C Build Configuration using the default build config and an arbitrary name
CMakeBuildConfigurationExtended cmBuildConfig = new CMakeBuildConfigurationExtended(buildConfig,
"cmBuildConfigName", mockToolchain) {
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
mockToolchain) {

@Override
public ICMakeProperties getCMakeProperties() {
Expand All @@ -153,46 +106,25 @@ public ICMakeProperties getCMakeProperties() {
}

/**
* Test for {@link IOsOverrides#setExtraArguments()}
* Test for {@link CMakeBuildConfiguration#getDefaultProperties()}
*/
@Test
public void getCMakePropertiesTestIOsOverridesSetExtraArguments() throws Exception {
public void getDefaultProperties() throws Exception {

This comment has been minimized.

Sign in to view

Copy link
@betamaxbandit

betamaxbandit Jan 24, 2025

Contributor

I like this! The tests are much simpler now :-)

IIUC another test should be added to set the extra arguments. Like this:

@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");
			defs.put(CMAKE_USE_UI_OVERRIDES, "true");
			return defs;
		}
	};
}
// Create a C Build Configuration using the default build config and an arbitrary name
CMakeBuildConfigurationExtended cmBuildConfig = new CMakeBuildConfigurationExtended(buildConfig,
"cmBuildConfigName", mockToolchain);
CMakeBuildConfiguration cmBuildConfig = new CMakeBuildConfiguration(buildConfig, "cmBuildConfigName",
mockToolchain) {

@Override
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;
}
};
// Call the new method on ICMakeBuildConfiguration to get the default CMake properties.
ICMakeProperties cMakeProperties = cmBuildConfig.getCMakeProperties();

// Get overrides for Windows host and check the default value for getExtraArguments.
IOsOverrides windowsOverrides = cMakeProperties.getWindowsOverrides();
List<String> winExtraArguments = windowsOverrides.getExtraArguments();
assertThat(winExtraArguments, contains("-Dtest0=0", "-Dtest1=1"));

// Get overrides for Linux host and check the default value for getExtraArguments.
IOsOverrides linuxOverrides = cMakeProperties.getLinuxOverrides();
List<String> linuxExtraArguments = linuxOverrides.getExtraArguments();
assertThat(linuxExtraArguments, contains("-DLinuxtest0=0", "-DLinuxtest1=1"));
}

private class CMakeBuildConfigurationExtended extends CMakeBuildConfiguration {

public CMakeBuildConfigurationExtended(IBuildConfiguration config, String name, IToolChain toolChain) {
super(config, name, toolChain);
}

@Override
public ICMakeProperties getCMakeProperties() {
// get the built-in CDT defaults
ICMakeProperties properties = super.getCMakeProperties();

IOsOverrides windowsOverrides = properties.getWindowsOverrides();
windowsOverrides.setExtraArguments(new ArrayList<>((List.of("-Dtest0=0", "-Dtest1=1"))));

IOsOverrides linuxOverrides = properties.getLinuxOverrides();
linuxOverrides.setExtraArguments(new ArrayList<>((List.of("-DLinuxtest0=0", "-DLinuxtest1=1"))));

return properties;
}
assertThat(cMakeProperties.getGenerator(), is(CMakeGenerator.WatcomWMake));
}

private IProject createCMakeProject() throws Exception {
Expand Down
39 changes: 0 additions & 39 deletions ....cdt.cmake.core.tests/src/org/eclipse/cdt/cmake/core/ExtendedCMakeBuildConfiguration.java
View file

This file was deleted.

98 changes: 0 additions & 98 deletions ...make.core.tests/src/org/eclipse/cdt/cmake/core/internal/CMakePropertiesEvolutionTest.java
View file

This file was deleted.

Loading

0 comments on commit 1218066

Please sign in to comment.