Skip to content

Commit b9f56ca

Browse files
jonahgrahamjonahgraham
committed
Fix many of the review comments.
1 parent 852b0a8 commit b9f56ca

File tree

8 files changed

+42
-56
lines changed

8 files changed

+42
-56
lines changed

‎build/org.eclipse.cdt.managedbuilder.core.tests/resources/builderTests/regressions/helloworldCPP.zip

-250 Bytes
Binary file not shown.

‎build/org.eclipse.cdt.managedbuilder.core.tests/tests/org/eclipse/cdt/managedbuilder/core/tests/CompilationDatabaseGenerationTest.java

+24-43
Original file line numberDiff line numberDiff line change
@@ -15,61 +15,54 @@
1515
import static org.junit.jupiter.api.Assertions.assertTrue;
1616

1717
import java.io.FileReader;
18-
import java.io.IOException;
1918

20-
import org.eclipse.cdt.managedbuilder.core.ManagedBuilderCorePlugin;
2119
import org.eclipse.cdt.managedbuilder.core.jsoncdb.CompilationDatabaseInformation;
20+
import org.eclipse.cdt.managedbuilder.internal.core.CommonBuilder;
2221
import org.eclipse.cdt.managedbuilder.testplugin.AbstractBuilderTest;
2322
import org.eclipse.cdt.managedbuilder.testplugin.ManagedBuildTestHelper;
2423
import org.eclipse.core.resources.IFile;
2524
import org.eclipse.core.resources.IProject;
2625
import org.eclipse.core.resources.IncrementalProjectBuilder;
27-
import org.eclipse.core.runtime.CoreException;
2826
import org.eclipse.core.runtime.preferences.InstanceScope;
2927
import org.eclipse.jface.preference.IPreferenceStore;
3028
import org.eclipse.ui.preferences.ScopedPreferenceStore;
31-
import org.junit.Ignore;
32-
import org.junit.Test;
29+
import org.junit.jupiter.api.AfterEach;
30+
import org.junit.jupiter.api.Test;
3331

3432
import com.google.gson.Gson;
3533
import com.google.gson.JsonArray;
3634
import com.google.gson.JsonElement;
37-
import com.google.gson.JsonIOException;
3835

3936
public class CompilationDatabaseGenerationTest extends AbstractBuilderTest {
4037

4138
/**
4239
* Tests generation of compile_commands.json in "build" folder
43-
* @throws CoreException
4440
*/
4541
@Test
46-
public void testCompilationDatabaseGeneration() throws CoreException {
42+
public void testCompilationDatabaseGeneration() throws Exception {
4743
setWorkspace("regressions");
4844
final IProject app = loadProject("helloworldC");
49-
isGenerateFileOptionEnabled(true);
45+
setGenerateFileOptionEnabled(true);
5046
app.build(IncrementalProjectBuilder.FULL_BUILD, null);
5147
IFile compilationDatabase = app.getFile("build/compile_commands.json");
5248
assertTrue(compilationDatabase.exists());
5349
}
5450

5551
/**
5652
* Tests format for compile_commands.json. JSON array is expected, containing an element for the c file
57-
* @throws JsonIOException
58-
* @throws CoreException
5953
*/
6054
@Test
61-
public void testJsonFormat() throws JsonIOException, CoreException {
55+
public void testJsonFormat() throws Exception {
6256
setWorkspace("regressions");
6357
final IProject app = loadProject("helloworldC");
64-
isGenerateFileOptionEnabled(true);
58+
setGenerateFileOptionEnabled(true);
6559
app.build(IncrementalProjectBuilder.FULL_BUILD, null);
6660
IFile commandsFile = app.getFile("build/compile_commands.json");
6761
if (commandsFile.exists()) {
6862

6963
try (FileReader reader = new FileReader(commandsFile.getLocation().toFile())) {
7064
Gson gson = new Gson();
7165
JsonArray jsonArray = gson.fromJson(reader, JsonArray.class);
72-
System.out.println(jsonArray);
7366
for (JsonElement element : jsonArray) {
7467
CompilationDatabaseInformation compileCommand = gson.fromJson(element,
7568
CompilationDatabaseInformation.class);
@@ -80,23 +73,20 @@ public void testJsonFormat() throws JsonIOException, CoreException {
8073
assertTrue(compileCommand.file().endsWith("src/helloworldC.c"));
8174
}
8275

83-
} catch (IOException e) {
84-
assertTrue(false);
8576
}
8677

8778
}
8879
}
8980

9081
/**
9182
* Test that compile_commands.json is correctly generated when more than one .c file is present as a source file
92-
* @throws CoreException
9383
*/
9484
@Test
95-
public void testMultipleFiles() throws CoreException {
85+
public void testMultipleFiles() throws Exception {
9686
setWorkspace("regressions");
9787
final IProject app = loadProject("helloworldC");
9888
IFile aFile = ManagedBuildTestHelper.createFile(app, "src/newFile.c");
99-
isGenerateFileOptionEnabled(true);
89+
setGenerateFileOptionEnabled(true);
10090
app.build(IncrementalProjectBuilder.FULL_BUILD, null);
10191
IFile commandsFile = app.getFile("build/compile_commands.json");
10292
int numberOfElementsFound = 0;
@@ -120,31 +110,24 @@ public void testMultipleFiles() throws CoreException {
120110
assertEquals(2, numberOfElementsFound);
121111
assertTrue(helloworldCIsPresent);
122112
assertTrue(newFileIsPresent);
123-
} catch (IOException e) {
124-
assertTrue(false);
125113
}
126-
127114
}
128115

129116
/**
130117
* Tests that cpp files are handled by compile_commands.json file generator
131-
* @throws CoreException
132118
*/
133119
@Test
134-
@Ignore("This will be temporary skipped due to builder error")
135-
public void isCPPFileAllowed() throws CoreException {
120+
public void isCPPFileAllowed() throws Exception {
136121
setWorkspace("regressions");
137122
final IProject app = loadProject("helloworldCPP");
138-
isGenerateFileOptionEnabled(true);
123+
setGenerateFileOptionEnabled(true);
139124
app.build(IncrementalProjectBuilder.FULL_BUILD, null);
140-
System.out.println(app.getLocation());
141125
IFile commandsFile = app.getFile("build/compile_commands.json");
142126
if (commandsFile.exists()) {
143127

144128
try (FileReader reader = new FileReader(commandsFile.getLocation().toFile())) {
145129
Gson gson = new Gson();
146130
JsonArray jsonArray = gson.fromJson(reader, JsonArray.class);
147-
System.out.println(jsonArray);
148131
for (JsonElement element : jsonArray) {
149132
CompilationDatabaseInformation compileCommand = gson.fromJson(element,
150133
CompilationDatabaseInformation.class);
@@ -155,35 +138,33 @@ public void isCPPFileAllowed() throws CoreException {
155138
assertTrue(compileCommand.file().endsWith("src/helloworldCPP.cpp"));
156139
}
157140

158-
} catch (IOException e) {
159-
assertTrue(false);
160141
}
161142
}
162143
}
163144

164145
/**
165146
* Tests that compilation database is not generated when feature is disabled
166-
* @throws CoreException
167147
*/
168148
@Test
169-
public void testCompilationDatabaseGenerationNotEnabled() throws CoreException {
149+
public void testCompilationDatabaseGenerationNotEnabled() throws Exception {
170150
setWorkspace("regressions");
171151
final IProject app = loadProject("helloworldC");
172-
isGenerateFileOptionEnabled(false);
152+
setGenerateFileOptionEnabled(false);
173153
app.build(IncrementalProjectBuilder.FULL_BUILD, null);
174154
IFile compilationDatabase = app.getFile("build/compile_commands.json");
175155
assertFalse(compilationDatabase.exists());
176156
}
177157

178-
public static boolean isGenerateFileOptionEnabled(boolean value) {
179-
try {
180-
IPreferenceStore preferenceStore = new ScopedPreferenceStore(InstanceScope.INSTANCE,
181-
"org.eclipse.cdt.managedbuilder.ui"); //$NON-NLS-1$
182-
preferenceStore.setDefault("generateFile", value);
183-
return preferenceStore.getBoolean("generateFile");
184-
} catch (Exception e) {
185-
ManagedBuilderCorePlugin.log(e);
186-
}
187-
return false;
158+
private static void setGenerateFileOptionEnabled(boolean value) {
159+
IPreferenceStore preferenceStore = new ScopedPreferenceStore(InstanceScope.INSTANCE,
160+
"org.eclipse.cdt.managedbuilder.ui");
161+
preferenceStore.setValue(CommonBuilder.COMPILATION_DATABASE_ENABLEMENT, value);
162+
}
163+
164+
@AfterEach
165+
private void restoreDefaultForGenerateFile() {
166+
IPreferenceStore preferenceStore = new ScopedPreferenceStore(InstanceScope.INSTANCE,
167+
"org.eclipse.cdt.managedbuilder.ui");
168+
preferenceStore.setToDefault(CommonBuilder.COMPILATION_DATABASE_ENABLEMENT);
188169
}
189170
}

‎build/org.eclipse.cdt.managedbuilder.core/.project

+5
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@
2525
<arguments>
2626
</arguments>
2727
</buildCommand>
28+
<buildCommand>
29+
<name>org.eclipse.pde.ds.core.builder</name>
30+
<arguments>
31+
</arguments>
32+
</buildCommand>
2833
</buildSpec>
2934
<natures>
3035
<nature>org.eclipse.jdt.core.javanature</nature>

‎build/org.eclipse.cdt.managedbuilder.core/META-INF/MANIFEST.MF

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ Export-Package: org.eclipse.cdt.build.core.scannerconfig,
1616
org.eclipse.cdt.managedbuilder.envvar,
1717
org.eclipse.cdt.managedbuilder.internal.buildmodel;x-friends:="org.eclipse.cdt.managedbuilder.ui",
1818
org.eclipse.cdt.managedbuilder.internal.core;x-friends:="org.eclipse.cdt.managedbuilder.ui,org.eclipse.cdt.managedbuilder.headlessbuilderapp",
19+
org.eclipse.cdt.managedbuilder.internal.core.jsoncdb.generator;x-internal:=true,
1920
org.eclipse.cdt.managedbuilder.internal.dataprovider;x-internal:=true,
2021
org.eclipse.cdt.managedbuilder.internal.envvar;x-internal:=true,
2122
org.eclipse.cdt.managedbuilder.internal.language.settings.providers;x-friends:="org.eclipse.cdt.managedbuilder.ui",

‎build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/CommonBuilder.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public class CommonBuilder extends ACBuilder implements IIncrementalProjectBuild
9797
private static final String NEWLINE = System.getProperty("line.separator"); //$NON-NLS-1$
9898
private static final String TRACE_FOOTER = "]: "; //$NON-NLS-1$
9999
private static final String TRACE_HEADER = "GeneratedmakefileBuilder trace ["; //$NON-NLS-1$
100-
private static final String COMPILATION_DATABASE_ENABLEMENT = "generateFile"; //$NON-NLS-1$
100+
public static final String COMPILATION_DATABASE_ENABLEMENT = "generateCBDFile"; //$NON-NLS-1$
101101
public static boolean VERBOSE = false;
102102

103103
private static final int PROGRESS_MONITOR_SCALE = 100;

‎build/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/internal/ui/compilationdatabase/CompilationDatabaseGeneratorBlock.java

+6-11
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
********************************************************************************/
1010
package org.eclipse.cdt.managedbuilder.internal.ui.compilationdatabase;
1111

12+
import org.eclipse.cdt.managedbuilder.internal.core.CommonBuilder;
1213
import org.eclipse.cdt.managedbuilder.ui.properties.ManagedBuilderUIPlugin;
1314
import org.eclipse.cdt.ui.dialogs.AbstractCOptionPage;
1415
import org.eclipse.cdt.ui.dialogs.ICOptionContainer;
@@ -33,15 +34,14 @@
3334
*/
3435
public class CompilationDatabaseGeneratorBlock extends AbstractCOptionPage {
3536

36-
private static final String PREF_PAGE_ID = "org.eclipse.cdt.managedbuilder.internal.ui.compilationdatabase.JsonCdbGeneratorPreferencePage"; //$NON-NLS-1$
37-
private final String ENABLE_FILE_GENERATION = "generateFile"; //$NON-NLS-1$
37+
private static final String PREF_PAGE_ID = "org.eclipse.cdt.managedbuilder.ui.compilationdatabase.JsonCdbPreferencePage"; //$NON-NLS-1$
38+
private final String ENABLE_FILE_GENERATION = CommonBuilder.COMPILATION_DATABASE_ENABLEMENT;
3839
private Button generateFileCheckbox;
3940
private IPreferenceStore preferenceStore;
4041
private PreferenceScopeBlock fPrefScopeBlock;
4142

42-
protected CompilationDatabaseGeneratorBlock() {
43+
public CompilationDatabaseGeneratorBlock() {
4344
preferenceStore = ManagedBuilderUIPlugin.getDefault().getPreferenceStore();
44-
performDefaults();
4545
}
4646

4747
@Override
@@ -66,21 +66,16 @@ protected void onPreferenceScopeChange() {
6666
generateFileCheckbox = new Button(cdbGeneratorOptions, SWT.CHECK);
6767
generateFileCheckbox.setSelection(preferenceStore.getBoolean(ENABLE_FILE_GENERATION));
6868
generateFileCheckbox.setText(Messages.JsonCdbGeneratorPreferencePage_generateCompilationdatabase);
69-
generateFileCheckbox.addListener(SWT.Selection, e -> {
70-
boolean newValue = generateFileCheckbox.getSelection();
71-
preferenceStore.setValue(ENABLE_FILE_GENERATION, newValue);
72-
});
73-
7469
}
7570

7671
@Override
7772
public void performDefaults() {
78-
preferenceStore.setDefault(ENABLE_FILE_GENERATION, false);
73+
preferenceStore.setToDefault(ENABLE_FILE_GENERATION);
7974
}
8075

8176
@Override
8277
public void performApply(IProgressMonitor monitor) throws CoreException {
83-
this.performApply(monitor);
78+
preferenceStore.setValue(ENABLE_FILE_GENERATION, generateFileCheckbox.getSelection());
8479
}
8580

8681
private IProject getProject() {

‎build/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/internal/ui/compilationdatabase/JsonCdbGeneratorPreferencePage.java

+3
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@
99
********************************************************************************/
1010
package org.eclipse.cdt.managedbuilder.internal.ui.compilationdatabase;
1111

12+
import org.eclipse.cdt.managedbuilder.ui.properties.ManagedBuilderUIPlugin;
1213
import org.eclipse.cdt.ui.dialogs.ICOptionContainer;
1314
import org.eclipse.core.resources.IProject;
1415
import org.eclipse.core.runtime.CoreException;
1516
import org.eclipse.core.runtime.NullProgressMonitor;
17+
import org.eclipse.core.runtime.Status;
1618
import org.eclipse.jface.preference.PreferencePage;
1719
import org.eclipse.swt.SWT;
1820
import org.eclipse.swt.layout.GridData;
@@ -72,6 +74,7 @@ public boolean performOk() {
7274
try {
7375
fOptionBlock.performApply(new NullProgressMonitor());
7476
} catch (CoreException e) {
77+
ManagedBuilderUIPlugin.log(Status.error("Failed to save JSON Compilation Database Generator settings", e)); //$NON-NLS-1$
7578
}
7679
return true;
7780
}

‎build/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/internal/ui/compilationdatabase/JsonCdbGeneratorPropertyPage.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.eclipse.core.runtime.IAdaptable;
1818
import org.eclipse.core.runtime.NullProgressMonitor;
1919
import org.eclipse.core.runtime.Preferences;
20+
import org.eclipse.core.runtime.Status;
2021
import org.eclipse.swt.SWT;
2122
import org.eclipse.swt.layout.GridLayout;
2223
import org.eclipse.swt.widgets.Composite;
@@ -54,7 +55,7 @@ public boolean performOk() {
5455
try {
5556
optionPage.performApply(new NullProgressMonitor());
5657
} catch (CoreException e) {
57-
ManagedBuilderUIPlugin.log(e);
58+
ManagedBuilderUIPlugin.log(Status.error("Failed to save JSON Compilation Database Generator settings", e)); //$NON-NLS-1$
5859
}
5960
return true;
6061
}

0 commit comments

Comments
 (0)