Bug 505868: Split clean command when cleaning lots of files

This change overcomes the Cannot run program "rm": Command line
too long error when there are hundreds to thousands of files.

This change only applies to the interal builder.

Change-Id: Idc32067e27d76e3b438b2b1a07376859c7c8d1e4

build/org.eclipse.cdt.managedbuilder.core.tests/suite/org/eclipse/cdt/managedbuilder/testplugin/AbstractBuilderTest.java

@@ -26,6 +26,7 @@ import org.eclipse.cdt.core.settings.model.ICConfigurationDescription;
 import org.eclipse.cdt.core.settings.model.ICProjectDescription;
 import org.eclipse.cdt.core.settings.model.ICProjectDescriptionManager;
 import org.eclipse.core.resources.IBuildConfiguration;
+import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IFolder;
 import org.eclipse.core.resources.IMarker;
 import org.eclipse.core.resources.IProject;
@@ -126,7 +127,14 @@ public abstract class AbstractBuilderTest extends TestCase {
 	}
 
 	protected Collection<IResource> getProjectBuildExeResources(String projectName, String cfgName, String obj) throws CoreException {
-		return getProjectBuildExeResources(projectName, cfgName, new String[]{obj});
+		return getProjectBuildExeResources(projectName, cfgName, obj, true);
+	}
+
+	/**
+	 * The externalBuilder is true for when makefiles are generated, or false for internal builder
+	 */
+	protected Collection<IResource> getProjectBuildExeResources(String projectName, String cfgName, String obj, boolean externalBuilder) throws CoreException {
+		return getProjectBuildExeResources(projectName, cfgName, new String[]{obj}, externalBuilder);
 	}
 
 	protected Collection<IResource> getProjectBuildLibResources(String projectName, String cfgName, String obj) throws CoreException {
@@ -137,11 +145,22 @@ public abstract class AbstractBuilderTest extends TestCase {
 		return getProjectBuildSharedLibResources(projectName, cfgName, new String[]{obj});
 	}
 
-	protected Collection<IResource> getProjectBuildExeResources(String projectName, String cfgName, String[] objs) throws CoreException {
+	protected IFile getProjectExe(String projectName, String cfgName) throws CoreException {
-		Collection<IResource> resources = getProjectBuildResources(projectName, cfgName, objs);
 		IProject project = getWorkspace().getRoot().getProject(projectName);
 		IFolder buildDir = project.getFolder(cfgName);
-		resources.add(buildDir.getFile(projectName + (WINDOWS ? ".exe" : "")));
+		return buildDir.getFile(projectName + (WINDOWS ? ".exe" : ""));
+	}
+
+	protected Collection<IResource> getProjectBuildExeResources(String projectName, String cfgName, String[] objs) throws CoreException {
+		return getProjectBuildExeResources(projectName, cfgName, objs, true);
+	}
+
+	/**
+	 * The externalBuilder is true for when makefiles are generated, or false for internal builder
+	 */
+	protected Collection<IResource> getProjectBuildExeResources(String projectName, String cfgName, String[] objs, boolean externalBuilder) throws CoreException {
+		Collection<IResource> resources = getProjectBuildResources(projectName, cfgName, objs, externalBuilder);
+		resources.add(getProjectExe(projectName, cfgName));
 		return resources;
 	}
 
@@ -166,18 +185,33 @@ public abstract class AbstractBuilderTest extends TestCase {
 	 * The object files expected to be output can also be specified.
 	 */
 	protected Collection<IResource> getProjectBuildResources(String projectName, String cfgName, String[] objs) throws CoreException {
+		return getProjectBuildResources(projectName, cfgName, objs, true);
+	}
+
+	/**
+	 * Returns an array of resources expected to be generated by building a project configuration.
+	 * The object files expected to be output can also be specified.
+	 * The externalBuilder is true for when makefiles are generated, or false for internal builder
+	 */
+	protected Collection<IResource> getProjectBuildResources(String projectName, String cfgName, String[] objs, boolean externalBuilder) throws CoreException {
 		IProject project = getWorkspace().getRoot().getProject(projectName);
 		IFolder buildDir = project.getFolder(cfgName);
 		Collection<IResource> resources = new LinkedHashSet<IResource>();
 		resources.add(buildDir);
-		resources.add(buildDir.getFile("makefile"));
+		if (externalBuilder) {
-		resources.add(buildDir.getFile("objects.mk"));
+			resources.add(buildDir.getFile("makefile"));
-		resources.add(buildDir.getFile("sources.mk"));
+			resources.add(buildDir.getFile("objects.mk"));
+			resources.add(buildDir.getFile("sources.mk"));
+		}
 		for (String obj : objs) {
-			resources.add(buildDir.getFile(obj + ".d"));
+			if (externalBuilder) {
+				resources.add(buildDir.getFile(obj + ".d"));
+			}
 			resources.add(buildDir.getFile(obj + ".o"));
 			// Add subdir.mk in the same directory
-			resources.add(buildDir.getFile(new Path(obj).removeLastSegments(1).append("subdir.mk")));
+			if (externalBuilder) {
+				resources.add(buildDir.getFile(new Path(obj).removeLastSegments(1).append("subdir.mk")));
+			}
 			// If the parent of the obj doesn't exist, then ensure we're expecting that too...
 			IPath p = new Path(obj).removeLastSegments(1);
 			while (p.segmentCount() > 0) {

build/org.eclipse.cdt.managedbuilder.core.tests/tests/org/eclipse/cdt/managedbuilder/core/tests/ManagedBuildClean.java

@@ -0,0 +1,82 @@
+/*******************************************************************************
+ * Copyright (c) 20116 Kichwa Coders Ltd and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Jonah Graham (Kichwa Coders) - Initial API and implementation
+ *******************************************************************************/
+package org.eclipse.cdt.managedbuilder.core.tests;
+
+import java.util.Collection;
+
+import org.eclipse.cdt.managedbuilder.testplugin.AbstractBuilderTest;
+import org.eclipse.cdt.managedbuilder.testplugin.ManagedBuildTestHelper;
+import org.eclipse.core.resources.IFile;
+import org.eclipse.core.resources.IProject;
+import org.eclipse.core.resources.IResource;
+import org.eclipse.core.resources.IWorkspaceDescription;
+import org.eclipse.core.resources.IncrementalProjectBuilder;
+import org.eclipse.core.resources.ResourcesPlugin;
+import org.eclipse.core.runtime.CoreException;
+
+public class ManagedBuildClean extends AbstractBuilderTest {
+	private static final String PROJ_PATH = "testCleanProjects";
+	private IProject fInternalBuilderProject;
+	private IProject fExternalBuilderProject;
+
+	@Override
+	protected void setUp() throws Exception {
+		super.setUp();
+		IWorkspaceDescription wsDescription = ResourcesPlugin.getWorkspace().getDescription();
+		wsDescription.setAutoBuilding(false);
+		ResourcesPlugin.getWorkspace().setDescription(wsDescription);
+		assertNotNull("Cannot create testCleanInternal project",
+				fInternalBuilderProject = ManagedBuildTestHelper.loadProject("testCleanInternal", PROJ_PATH));
+		assertNotNull("Cannot create testCleanExternal project",
+				fExternalBuilderProject = ManagedBuildTestHelper.loadProject("testCleanExternal", PROJ_PATH));
+	}
+
+	@Override
+	protected void tearDown() throws Exception {
+		super.tearDown();
+		ManagedBuildTestHelper.removeProject(fInternalBuilderProject.getName());
+	}
+
+	public void testCleanInternal() throws Exception {
+		helperTestClean(fInternalBuilderProject, false);
+	}
+
+	public void testCleanExternal() throws Exception {
+		helperTestClean(fExternalBuilderProject, true);
+	}
+
+	private void helperTestClean(IProject project, boolean externalBuilder) throws CoreException {
+
+		// do a build and ensure files are present
+		project.build(IncrementalProjectBuilder.FULL_BUILD, null);
+		Collection<IResource> resources = getProjectBuildExeResources(project.getName(), "Debug",
+				"src/" + project.getName(), externalBuilder);
+		for (IResource resource : resources) {
+			assertTrue("Resource not found: " + resource, resource.exists());
+		}
+
+		// do a clean and make sure files are gone
+		project.build(IncrementalProjectBuilder.CLEAN_BUILD, null);
+		for (IResource resource : resources) {
+			if (!(resource instanceof IFile)) {
+				// Only files are removed by clean, not folders
+				continue;
+			}
+			if (externalBuilder
+					&& (resource.getName().endsWith(".mk") || resource.getName().equals("makefile"))) {
+				// makefiles are not removed when cleaning
+				continue;
+			}
+			assertFalse("Resource not deleted: " + resource, resource.exists());
+		}
+	}
+
+}

build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/buildmodel/BuildStep.java

@@ -46,6 +46,21 @@ import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.Path;
 
 public class BuildStep implements IBuildStep {
+	/**
+	 * When an argument of a command is joined into a String for preparation for running with exec, the
+	 * argument may need surrounding with quotes and have spaces between each argument. This padding allows
+	 * for that when constructing long commands.
+	 */
+	private static final int PER_ARGUMENT_PADDING = 3;
+	/**
+	 * On Windows XP and above, the maximum command line length is 8191, on Linux it is at least 131072, but
+	 * that includes the environment. We want to limit the invocation of a single command to this number of
+	 * characters, and we want to ensure that the number isn't so low as to slow down operation.
+	 * 
+	 * Doing each rm in its own command would be very slow, especially on Windows.
+	 */
+	private static final int MAX_CLEAN_LENGTH = 6000;
+
 	private List<BuildIOType> fInputTypes = new ArrayList<BuildIOType>();
 	private List<BuildIOType> fOutputTypes = new ArrayList<BuildIOType>();
 	private ITool fTool;
@@ -230,47 +245,81 @@ public class BuildStep implements IBuildStep {
 			cwd = calcCWD();
 
 		if (fTool == null) {
-			String step = null;
+			if (this == fBuildDescription.getCleanStep()) {
-			String appendToLastStep = null;
-			if (this == fBuildDescription.getInputStep()) {
-				step = fBuildDescription.getConfiguration().getPrebuildStep();
-			} else if (this == fBuildDescription.getOutputStep()) {
-				step = fBuildDescription.getConfiguration().getPostbuildStep();
-			} else if (this == fBuildDescription.getCleanStep()) {
-				step = fBuildDescription.getConfiguration().getCleanCommand();
-
-				IBuildResource[] generated = fBuildDescription.getResources(true);
-
-				if (generated.length != 0) {
-					StringBuilder buf = new StringBuilder();
-					for (int i = 0; i < generated.length; i++) {
-						buf.append(' ');
-
-						IPath rel = BuildDescriptionManager.getRelPath(cwd, generated[i].getLocation());
-						buf.append(rel.toString());
-					}
-					appendToLastStep = buf.toString();
-				}
-			}
-
-			if (step != null && (step = step.trim()).length() > 0) {
-				step = resolveMacros(step, resolveAll);
-				if (step != null && (step = step.trim()).length() > 0) {
-					String commands[] = step.split(";"); //$NON-NLS-1$
-
-					if (appendToLastStep != null && commands.length != 0) {
-						commands[commands.length - 1] = commands[commands.length - 1] + appendToLastStep;
-					}
 
+				String cleanCmd = fBuildDescription.getConfiguration().getCleanCommand();
+				if (cleanCmd != null && (cleanCmd = cleanCmd.trim()).length() > 0) {
 					List<IBuildCommand> list = new ArrayList<IBuildCommand>();
-					for (int i = 0; i < commands.length; i++) {
+					cleanCmd = resolveMacros(cleanCmd, resolveAll);
-						IBuildCommand cmds[] = createCommandsFromString(commands[i], cwd, getEnvironment());
+					String commands[] = cleanCmd.split(";"); //$NON-NLS-1$
-						for (int j = 0; j < cmds.length; j++) {
+					for (int i = 0; i < commands.length - 1; i++) {
-							list.add(cmds[j]);
+						list.add(createCommandFromString(commands[0], cwd, getEnvironment()));
-						}
 					}
+
+					List<String> cleanCmdArgs = convertStringToArguments(commands[commands.length - 1]);
+					final int initialLen = cleanCmdArgs.stream()
+							.mapToInt(w -> w.length() + PER_ARGUMENT_PADDING).sum();
+					IPath cleanCmdPath = new Path(cleanCmdArgs.get(0));
+					Map<String, String> env = getEnvironment();
+
+					IBuildResource[] resources = fBuildDescription.getResources(true);
+
+					List<String> args = new ArrayList<>();
+					args.addAll(cleanCmdArgs.subList(1, cleanCmdArgs.size()));
+					int totalLen = initialLen;
+					for (IBuildResource resource : resources) {
+						IPath resLoc = BuildDescriptionManager.getRelPath(cwd, resource.getLocation());
+						String path = resLoc.toString();
+						int pathLen = path.length() + PER_ARGUMENT_PADDING;
+
+						if (totalLen + pathLen > MAX_CLEAN_LENGTH && totalLen != initialLen) {
+							// adding new path takes us over limit, emit what we have...
+							BuildCommand buildCommand = new BuildCommand(cleanCmdPath,
+									args.toArray(new String[args.size()]), env, cwd, this);
+							list.add(buildCommand);
+
+							// ...and restart
+							totalLen = initialLen;
+							args.clear();
+							args.addAll(cleanCmdArgs.subList(1, cleanCmdArgs.size()));
+						}
+
+						args.add(path);
+						totalLen += pathLen;
+					}
+
+					// add remaining files
+					BuildCommand buildCommand = new BuildCommand(cleanCmdPath,
+							args.toArray(new String[args.size()]), env, cwd, this);
+					list.add(buildCommand);
+
 					return list.toArray(new BuildCommand[list.size()]);
 				}
+
+			} else {
+				String step = null;
+				if (this == fBuildDescription.getInputStep()) {
+					step = fBuildDescription.getConfiguration().getPrebuildStep();
+				} else if (this == fBuildDescription.getOutputStep()) {
+					step = fBuildDescription.getConfiguration().getPostbuildStep();
+				}
+
+				if (step != null && (step = step.trim()).length() > 0) {
+					step = resolveMacros(step, resolveAll);
+					if (step != null && (step = step.trim()).length() > 0) {
+						String commands[] = step.split(";"); //$NON-NLS-1$
+
+						List<IBuildCommand> list = new ArrayList<IBuildCommand>();
+						for (int i = 0; i < commands.length; i++) {
+							IBuildCommand cmds[] = createCommandsFromString(commands[i], cwd,
+									getEnvironment());
+							for (int j = 0; j < cmds.length; j++) {
+								list.add(cmds[j]);
+							}
+						}
+						return list.toArray(new BuildCommand[list.size()]);
+					}
+				}
 			}
 			return new IBuildCommand[0];
 		}
@@ -354,7 +403,29 @@ public class BuildStep implements IBuildStep {
 	}
 
 	protected IBuildCommand[] createCommandsFromString(String cmd, IPath cwd, Map<String, String> env) {
-		char arr[] = cmd.toCharArray();
+		IBuildCommand buildCommand = createCommandFromString(cmd, cwd, env);
+		return new IBuildCommand[] { buildCommand };
+	}
+
+	protected IBuildCommand createCommandFromString(String cmd, IPath cwd, Map<String, String> env) {
+		List<String> list = convertStringToArguments(cmd);
+
+		IPath c = new Path(list.remove(0));
+		String[] args = list.toArray(new String[list.size()]);
+
+		BuildCommand buildCommand = new BuildCommand(c, args, env, cwd, this);
+		return buildCommand;
+	}
+
+	/**
+	 * Convert string to arguments, first argument is command
+	 *
+	 * @param commandLine
+	 *            to parse
+	 * @return arguments as a list
+	 */
+	protected List<String> convertStringToArguments(String commandLine) {
+		char arr[] = commandLine.toCharArray();
 		char expect = 0;
 		char prev = 0;
 		// int start = 0;
@@ -400,11 +471,7 @@ public class BuildStep implements IBuildStep {
 
 		if (buf.length() > 0)
 			list.add(buf.toString());
-
+		return list;
-		IPath c = new Path(list.remove(0));
-		String[] args = list.toArray(new String[list.size()]);
-
-		return new IBuildCommand[] { new BuildCommand(c, args, env, cwd, this) };
 	}
 
 	private BuildResource[] getPrimaryResources(boolean input) {