From 10428dd53afc4d794d711cdde9e970ca9b9bf732 Mon Sep 17 00:00:00 2001 From: Morten Kristiansen Date: Sun, 25 Dec 2016 15:58:07 +0100 Subject: [PATCH] [Bug 340300] Fix parallel make when using pre-build steps Pre-build will always run in parallel to compilation of source files unless each and every source file depends on pre-build. Also, when parallel build was enabled, the "Make build target" under "Workbench Build Behavior" was ignored. Instead of doing make -j all, Eclipse ran make -j pre-build main-build. While the intentions are good, make will attempt to build pre-build and main-build in parallel for previous stated reasons. This patch changes two things: 1. Eclipse will consistently respect the "Workbench Build Behavior" for both single- and multi-process build. 2. The generated makefile is changed to guarantee pre-build is run first. Changed from all: pre-build main-build to all: $(MAKE) --no-print-directory pre-build $(MAKE) --no-print-directory main-build Change-Id: Icf3a1057ee3b3cc8a04a433820492a4f469e17dd Signed-off-by: Morten Kristiansen --- .../preAndPostBuildSteps/Benchmarks/makefile | 4 +- .../core/ExternalBuildRunner.java | 47 +++++-------------- .../makegen/gnu/GnuMakefileGenerator.java | 20 ++++---- 3 files changed, 26 insertions(+), 45 deletions(-) diff --git a/build/org.eclipse.cdt.managedbuilder.core.tests/resources/test30Projects/preAndPostBuildSteps/Benchmarks/makefile b/build/org.eclipse.cdt.managedbuilder.core.tests/resources/test30Projects/preAndPostBuildSteps/Benchmarks/makefile index abd0edb2928..67a38214181 100644 --- a/build/org.eclipse.cdt.managedbuilder.core.tests/resources/test30Projects/preAndPostBuildSteps/Benchmarks/makefile +++ b/build/org.eclipse.cdt.managedbuilder.core.tests/resources/test30Projects/preAndPostBuildSteps/Benchmarks/makefile @@ -34,7 +34,9 @@ endif # Add inputs and outputs from these tool invocations to the build variables # All Target -all: pre-build main-build +all: + $(MAKE) --no-print-directory pre-build + $(MAKE) --no-print-directory main-build # Main-build Target main-build: preAndPostBuildSteps diff --git a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/ExternalBuildRunner.java b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/ExternalBuildRunner.java index fd54bf98905..41dab02d436 100644 --- a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/ExternalBuildRunner.java +++ b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/ExternalBuildRunner.java @@ -160,43 +160,22 @@ public class ExternalBuildRunner extends AbstractBuildRunner { } protected String[] getTargets(int kind, IBuilder builder) { - String targetsArray[] = null; - - if(kind != IncrementalProjectBuilder.CLEAN_BUILD && !builder.isCustomBuilder() && builder.isManagedBuildOn()){ - IConfiguration cfg = builder.getParent().getParent(); - String preBuildStep = cfg.getPrebuildStep(); - try { - preBuildStep = ManagedBuildManager.getBuildMacroProvider().resolveValueToMakefileFormat( - preBuildStep, - "", //$NON-NLS-1$ - " ", //$NON-NLS-1$ - IBuildMacroProvider.CONTEXT_CONFIGURATION, - cfg); - } catch (BuildMacroException e) { - } - - if(preBuildStep != null && preBuildStep.length() != 0){ - targetsArray = new String[]{"pre-build", "main-build"}; //$NON-NLS-1$ //$NON-NLS-2$ - } + String targets = ""; //$NON-NLS-1$ + switch (kind) { + case IncrementalProjectBuilder.AUTO_BUILD : + targets = builder.getAutoBuildTarget(); + break; + case IncrementalProjectBuilder.INCREMENTAL_BUILD : // now treated as the same! + case IncrementalProjectBuilder.FULL_BUILD : + targets = builder.getIncrementalBuildTarget(); + break; + case IncrementalProjectBuilder.CLEAN_BUILD : + targets = builder.getCleanBuildTarget(); + break; } - if(targetsArray == null){ - String targets = ""; //$NON-NLS-1$ - switch (kind) { - case IncrementalProjectBuilder.AUTO_BUILD : - targets = builder.getAutoBuildTarget(); - break; - case IncrementalProjectBuilder.INCREMENTAL_BUILD : // now treated as the same! - case IncrementalProjectBuilder.FULL_BUILD : - targets = builder.getIncrementalBuildTarget(); - break; - case IncrementalProjectBuilder.CLEAN_BUILD : - targets = builder.getCleanBuildTarget(); - break; - } + String targetsArray[] = CommandLineUtil.argumentsToArray(targets); - targetsArray = CommandLineUtil.argumentsToArray(targets); - } return targetsArray; } diff --git a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/makegen/gnu/GnuMakefileGenerator.java b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/makegen/gnu/GnuMakefileGenerator.java index 67fbf4825a0..28cba267ac4 100644 --- a/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/makegen/gnu/GnuMakefileGenerator.java +++ b/build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/makegen/gnu/GnuMakefileGenerator.java @@ -1348,8 +1348,10 @@ public class GnuMakefileGenerator implements IManagedBuilderMakefileGenerator2 { // and neither conditions apply if we are building for it .... } */ - // If a prebuild step exists, redefine the all target to be - // all: {pre-build} main-build + // If a prebuild step exists, redefine the all target to be + // all: + // $(MAKE) pre-build + // $(MAKE) main-build // and then reset the "traditional" all target to main-build // This will allow something meaningful to happen if the generated // makefile is @@ -1361,17 +1363,15 @@ public class GnuMakefileGenerator implements IManagedBuilderMakefileGenerator2 { // Add the comment for the "All" target buffer.append(COMMENT_SYMBOL).append(WHITESPACE).append(ManagedMakeMessages.getResourceString(ALL_TARGET)).append(NEWLINE); - buffer.append(defaultTarget).append(WHITESPACE); - buffer.append(PREBUILD).append(WHITESPACE); - - // Reset defaultTarget for now and for subsequent use, below - defaultTarget = MAINBUILD; - buffer.append(defaultTarget); + // Invoke make multiple times to ensure pre-build is executed before main-build + buffer.append(defaultTarget).append(NEWLINE); + buffer.append(TAB).append(MAKE).append(WHITESPACE).append(NO_PRINT_DIR).append(WHITESPACE).append(PREBUILD).append(NEWLINE); + buffer.append(TAB).append(MAKE).append(WHITESPACE).append(NO_PRINT_DIR).append(WHITESPACE).append(MAINBUILD).append(NEWLINE); + buffer.append(NEWLINE); // Update the defaultTarget, main-build, by adding a colon, which is // needed below - defaultTarget = defaultTarget.concat(COLON); - buffer.append(NEWLINE).append(NEWLINE); + defaultTarget = MAINBUILD.concat(COLON); // Add the comment for the "main-build" target buffer.append(COMMENT_SYMBOL).append(WHITESPACE).append(ManagedMakeMessages.getResourceString(MAINBUILD_TARGET)).append(NEWLINE);