Work for bug 52647.

build/org.eclipse.cdt.managedbuilder.core/ChangeLog

@@ -1,3 +1,27 @@
+2004-02-23 Sean Evoy
+	Fix for bug 52647.
+	In 1.2, the target stored the raw, overridden build command the user 
+	specified on the property page. This string may or may not have included 
+	arguments to make. The managed build info was responsible for parsing the 
+	command from the arguments and returning both to the makefile generator. 
+	The problem was that the logic was too light-weight to really parse a 
+	complex command line. That logic has been refactored to the property page itself, 
+	so the price of parsing is payed once.
+	
+	The Target and its public interface have been reworked to set and get the 
+	arguments for make. This is treated as a project-level setting. It cannot 
+	be defined in a manifest for now. There is also a capability to reset and 
+	test the args when checking for an overridden make command in a target.
+	* src/org/eclipse/cdt/managedbuilder/core/ITarget.java 
+	* src/org/eclipse/cdt/managedbuilder/internal/core/Target.java
+
+	The arguments are now passed to the spawner that launches make correctly. 
+	* src/org/eclipse/cdt/managedbuilder/internal/core/GeneratedMakefileBuilder.java
+	
+	The ManagedBuildInfo is off the hook now. Rather than performing any 
+	parsing or foo-fa-raw, it simply delegates the lookup to the target.
+	* src/org/eclipse/cdt/managedbuilder/internal/core/ManagedBuildInfo.java
+
 2004-02-17 Sean Evoy
 	Fix for critical bug 44163.
 	The managed build info would become confused when the project it was associated

build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/core/ITarget.java

@@ -26,6 +26,7 @@ public interface ITarget extends IBuildObject {
 	public static final String IS_ABSTRACT = "isAbstract";	//$NON-NLS-1$
 	public static final String IS_TEST = "isTest";	//$NON-NLS-1$
 	public static final String MAKE_COMMAND = "makeCommand";	//$NON-NLS-1$
+	public static final String MAKE_ARGS = "makeArguments";	//$NON-NLS-1$
 	public static final String OS_LIST = "osList";	//$NON-NLS-1$
 	public static final String PARENT = "parent";	//$NON-NLS-1$
 	
@@ -95,6 +96,15 @@ public interface ITarget extends IBuildObject {
 	 */
 	public String getDefaultExtension();	
 
+	
+	/**
+	 * Answers the command line arguments to pass to the make utility used 
+	 * by the receiver to build a project.
+	 * 
+	 * @return
+	 */
+	public String getMakeArguments();
+	
 	/**
 	 * Answers the name of the make utility for the target.
 	 *  
@@ -193,6 +203,14 @@ public interface ITarget extends IBuildObject {
 	 */
 	public void setArtifactName(String name);
 
+	/**
+	 * Sets the arguments to be passed to the make utility used by the 
+	 * receiver to produce a build goal.
+	 * 
+	 * @param makeArgs
+	 */
+	public void setMakeArguments(String makeArgs);
+
 	/**
 	 * Sets the make command for the receiver to the value in the argument.
 	 * 

build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/GeneratedMakefileBuilder.java

@@ -335,9 +335,10 @@ public class GeneratedMakefileBuilder extends ACBuilder {
 
 				// Get the arguments to be passed to make from build model
 				ArrayList makeArgs = new ArrayList();
-				String args = info.getMakeArguments();
+				String arg = info.getMakeArguments();
-				if (args.length() > 0) {
+				String[] args = arg.split("\\s");
-					makeArgs.add(args);
+				for (int i = 0; i < args.length; ++i) {
+					makeArgs.add(args[i]);
 				}
 				makeArgs.addAll(Arrays.asList(getMakeTargets(fullBuild)));
 				String[] makeTargets = (String[]) makeArgs.toArray(new String[makeArgs.size()]);

build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/ManagedBuildInfo.java

@@ -19,15 +19,15 @@ import java.util.List;
 import java.util.ListIterator;
 import java.util.Map;
 
+import org.eclipse.cdt.core.CCProjectNature;
+import org.eclipse.cdt.core.CProjectNature;
+import org.eclipse.cdt.core.parser.IScannerInfo;
 import org.eclipse.cdt.managedbuilder.core.BuildException;
 import org.eclipse.cdt.managedbuilder.core.IConfiguration;
 import org.eclipse.cdt.managedbuilder.core.IManagedBuildInfo;
 import org.eclipse.cdt.managedbuilder.core.IOption;
 import org.eclipse.cdt.managedbuilder.core.ITarget;
 import org.eclipse.cdt.managedbuilder.core.ITool;
-import org.eclipse.cdt.core.CCProjectNature;
-import org.eclipse.cdt.core.CProjectNature;
-import org.eclipse.cdt.core.parser.IScannerInfo;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IResource;
 import org.eclipse.core.runtime.CoreException;
@@ -47,6 +47,11 @@ public class ManagedBuildInfo implements IManagedBuildInfo, IScannerInfo {
 	private Map defaultConfigurations;
 	private ITarget defaultTarget;
 	
+	/**
+	 * Create a new managed build information for the IResource specified in the argument
+	 * 
+	 * @param owner
+	 */
 	public ManagedBuildInfo(IResource owner) {
 		targetMap = new HashMap();
 		targets = new ArrayList();
@@ -54,13 +59,20 @@ public class ManagedBuildInfo implements IManagedBuildInfo, IScannerInfo {
 		this.owner = owner;
 	}
 	
+	/**
+	 * Reads the build information from the project file and creates the 
+	 * internal representation of the build settings for the project.
+	 * 
+	 * @param owner
+	 * @param element
+	 */
 	public ManagedBuildInfo(IResource owner, Element element) {
 		this(owner);
+		Node child = element.getFirstChild();
 		
 		// The id of the default configuration
 		String defaultTargetId = null;
 		List configIds = new ArrayList();
-		Node child = element.getFirstChild();
 		while (child != null) {
 			if (child.getNodeName().equals("target")) {
 				new Target(this, (Element)child);
@@ -495,46 +507,14 @@ public class ManagedBuildInfo implements IManagedBuildInfo, IScannerInfo {
 	 * @see org.eclipse.cdt.core.build.managed.IManagedBuildInfo#getMakeArguments()
 	 */
 	public String getMakeArguments() {
-		String arguments = new String();		
+		return getDefaultTarget().getMakeArguments();
-		
-		// The make command may or may not have any flags
-		ITarget target = getDefaultTarget();
-		String command = target.getMakeCommand();
-		
-		// If it does, the flags will be everything between the '-' and the next space
-		int indexOfArgs = command.indexOf('-');
-		if (indexOfArgs != - 1) {
-			try {
-				String argsAndTargs = command.substring(indexOfArgs);
-				int indexOfTargs = argsAndTargs.indexOf(' ');
-				arguments = (indexOfTargs != -1) ? 
-							argsAndTargs.substring(0, indexOfTargs) : 
-							argsAndTargs;
-				// Make sure the arg list does not contain f or C
-				
-			} catch (IndexOutOfBoundsException e) {
-			}
-		}
-		
-		return arguments.trim();
 	}
 
 	/* (non-Javadoc)
 	 * @see org.eclipse.cdt.core.build.managed.IManagedBuildInfo#getMakeCommand()
 	 */
 	public String getMakeCommand() {
-		String command = new String();
+		return getDefaultTarget().getMakeCommand();
-		ITarget target = getDefaultTarget();
-		command = target.getMakeCommand();
-		
-		// There may actually be arguments, so just get everything up to the first '-'
-		int indexOfArgs = command.indexOf('-');
-		if (indexOfArgs != -1) {
-			// Return ecverything up to the first argument as the command
-			return command.substring(0, indexOfArgs).trim();
-		} else {
-			return command.trim();
-		}
 	}
 
 	/* (non-Javadoc)

build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/core/Target.java

@@ -42,6 +42,7 @@ public class Target extends BuildObject implements ITarget {
 	private String extension;
 	private boolean isAbstract = false;
 	private boolean isTest = false;
+	private String makeArguments;
 	private String makeCommand;
 	private IResource owner;
 	private ITarget parent;
@@ -214,10 +215,13 @@ public class Target extends BuildObject implements ITarget {
 		// Get the clean command
 		cleanCommand = element.getAttribute(CLEAN_COMMAND);
 		
-		// Get the make command
+		// Get the make command and arguments
 		if (element.hasAttribute(MAKE_COMMAND)) {
 			makeCommand = element.getAttribute(MAKE_COMMAND);
 		}
+		if(element.hasAttribute(MAKE_ARGS)) {
+			makeArguments = element.getAttribute(MAKE_ARGS);
+		}
 	
 		Node child = element.getFirstChild();
 		while (child != null) {
@@ -249,6 +253,7 @@ public class Target extends BuildObject implements ITarget {
 	 */
 	public void resetMakeCommand() {
 		makeCommand = null;
+		makeArguments = null;
 	}
 	
 	/**
@@ -272,6 +277,9 @@ public class Target extends BuildObject implements ITarget {
 		if (makeCommand != null) {
 			element.setAttribute(MAKE_COMMAND, makeCommand);
 		}
+		if (makeArguments != null) {
+			element.setAttribute(MAKE_ARGS, makeArguments);
+		}
 				
 		if (configurations != null)
 			for (int i = 0; i < configurations.size(); ++i) {
@@ -282,6 +290,22 @@ public class Target extends BuildObject implements ITarget {
 			}
 	}
 
+	/* (non-Javadoc)
+	 * @see org.eclipse.cdt.managedbuilder.core.ITarget#getMakeArguments()
+	 */
+	public String getMakeArguments() {
+		if (makeArguments == null) {
+			// See if it is defined in my parent
+			if (parent != null) {
+				return parent.getMakeArguments();
+			} else { 
+				// No parent and no user setting
+				return new String("");
+			}
+		}
+		return makeArguments;
+	}
+
 	/* (non-Javadoc)
 	 * @see org.eclipse.cdt.core.build.managed.ITarget#getMakeCommand()
 	 */
@@ -376,7 +400,9 @@ public class Target extends BuildObject implements ITarget {
 	 * @see org.eclipse.cdt.managedbuilder.core.ITarget#hasMakeCommandOverride()
 	 */
 	public boolean hasOverridenMakeCommand() {
-		return (makeCommand != null && !makeCommand.equals(parent.getMakeCommand()));
+		// We answer true if the make command or the flags are different
+		return ((makeCommand != null && !makeCommand.equals(parent.getMakeCommand())) 
+			|| (makeArguments != null && !makeArguments.equals(parent.getMakeArguments())));
 	}
 
 	/**
@@ -551,6 +577,15 @@ public class Target extends BuildObject implements ITarget {
 		}
 	}
 
+	/* (non-Javadoc)
+	 * @see org.eclipse.cdt.managedbuilder.core.ITarget#setMakeArguments(java.lang.String)
+	 */
+	public void setMakeArguments(String makeArgs) {
+		if (makeArgs != null && !getMakeArguments().equals(makeArgs)) {
+			makeArguments = makeArgs;
+		}
+	}
+
 	/* (non-Javadoc)
 	 * @see org.eclipse.cdt.managedbuilder.core.ITarget#setMakeCommand(java.lang.String)
 	 */

build/org.eclipse.cdt.managedbuilder.ui/ChangeLog

@@ -1,3 +1,16 @@
+2004-02-23 Sean Evoy
+	Fix for bug 52647.
+	In 1.2, the target stored the raw, overridden build command the user 
+	specified on the property page. This fix involves enhancing the logic 
+	to pry apart the command from the args, and moving it into the property 
+	itself, so the build system only pays the price to parse once. 
+	Obviously since the make command or the args can be overridden by a user, 
+	the logic as to when to enable the edit field and check box in the manage 
+	dialog had to be tweaked. I am still not 100% satisfied, but this gets the 
+	meat of the fix into the hands of users.
+	* src/org/eclipse/cdt/managedbuilder/ui/properties/BuildPropertyPage.java
+	* src/org/eclipse/cdt/managedbuilder/ui/properties/ManageConfigDialog.java
+
 2004-2-17 Sean Evoy
 	Fixes for 51640
 	Externalized strings for the target names. 

build/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/properties/BuildPropertyPage.java

@@ -20,6 +20,8 @@ import java.util.Map;
 import java.util.Random;
 import java.util.Set;
 import java.util.SortedMap;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.eclipse.cdt.managedbuilder.core.IConfiguration;
 import org.eclipse.cdt.managedbuilder.core.IManagedBuildInfo;
@@ -458,8 +460,12 @@ public class BuildPropertyPage extends PropertyPage implements IWorkbenchPropert
 				// This is a cheap assignment to null so do it to be doubly sure
 				selectedTarget.resetMakeCommand();
 			} else {
-				String makeCommand = manageDialog.getMakeCommand();
+				// Parse for command and arguments
+				String rawCommand = manageDialog.getMakeCommand();
+				String makeCommand = parseMakeCommand(rawCommand);
 				selectedTarget.setMakeCommand(makeCommand);
+				String makeArguments = parseMakeArgs(rawCommand);
+				selectedTarget.setMakeArguments(makeArguments);
 			}
 			
 			// Check to see if any configurations have to be deleted
@@ -547,6 +553,126 @@ public class BuildPropertyPage extends PropertyPage implements IWorkbenchPropert
 		sashForm.setWeights(DEFAULT_SASH_WEIGHTS);
 	}
 
+	/* (non-Javadoc)
+	 * @param rawCommand
+	 * @return
+	 */
+	private String parseMakeArgs(String rawCommand) {
+		StringBuffer result = new StringBuffer();		
+		
+		// Parse out the command
+		String actualCommand = parseMakeCommand(rawCommand);
+		
+		// The flags and targets are anything not in the command
+		String arguments = rawCommand.substring(actualCommand.length());
+
+		// If there aren't any, we can stop
+		if (arguments.length() == 0) {
+			return result.toString().trim();
+		}
+
+		String[] tokens = arguments.trim().split("\\s");
+		/*
+		 * Cases to consider
+		 * --<flag>					Sensible, modern single flag. Add to result and continue.
+		 * -<flags>					Flags in single token, add to result and stop
+		 * -<flag_with_arg> ARG		Flag with argument. Add next token if valid arg.
+		 * -<mixed_flags> ARG		Mix of flags, one takes arg. Add next token if valid arg.
+		 * -<flag_with_arg>ARG		Corrupt case where next token should be arg but isn't
+		 * -<flags> [target]..		Flags with no args, another token, add flags and stop.
+		 */
+		Pattern flagPattern = Pattern.compile("C|f|I|j|l|O|W");
+		// Look for a '-' followed by 1 or more flags with no args and exactly 1 that expects args
+		Pattern mixedFlagWithArg = Pattern.compile("-[^CfIjloW]*[CfIjloW]{1}.+");
+		for (int i = 0; i < tokens.length; ++i) {
+			String currentToken = tokens[i];
+			if (currentToken.startsWith("--")) {
+				result.append(currentToken);
+				result.append(" ");
+			} else if (currentToken.startsWith("-")) {
+				// Is there another token
+				if (i + 1 >= tokens.length) {
+					//We are done
+					result.append(currentToken);
+				} else {
+					String nextToken = tokens[i + 1];
+					// Are we expecting arguments
+					Matcher flagMatcher = flagPattern.matcher(currentToken);
+					if (!flagMatcher.find()) {
+						// Evalutate whether the next token should be added normally
+						result.append(currentToken);
+						result.append(" ");
+					} else {
+						// Look for the case where there is no space between flag and arg
+						if (mixedFlagWithArg.matcher(currentToken).matches()) {
+							// Add this single token and keep going
+							result.append(currentToken);
+							result.append(" ");							
+						} else {
+							// Add this token and the next one right now
+							result.append(currentToken);
+							result.append(" ");
+							result.append(nextToken);
+							result.append(" ");
+							// Skip the next token the next time through, though
+							++i;
+						}
+					}
+				}
+			}
+		}
+		
+		return result.toString().trim();
+	}
+
+	/* (non-Javadoc)
+	 * 
+	 * @param string
+	 * @return
+	 */
+	private String parseMakeCommand(String rawCommand) {
+		StringBuffer command = new StringBuffer();
+		boolean hasSpace = false;
+		
+		// Try to separate out the command from the arguments 
+		String[] result = rawCommand.split("\\s");
+		
+		/*
+		 * Here are the cases to consider:
+		 * 	cmd								First segment is last segment, assume is command
+		 * 	cmd [flags]						First segment is the command
+		 * 	path/cmd [flags]				Same as above
+		 * 	path with space/make [flags]	Must append each segment up-to flags as command
+		 */
+		for (int i = 0; i < result.length; ++i) {
+			// Get the segment
+			String cmdSegment = result[i];
+			// If there is not another segment, we found the end
+			if (i + 1 >= result.length) {
+				command.append(cmdSegment);
+			} else {
+				// See if the next segment is the start of the flags
+				String nextSegment = result[i + 1];
+				if (nextSegment.startsWith("-")) {
+					// we have found the end of the command
+					command.append(cmdSegment);
+					break;
+				} else {
+					command.append(cmdSegment);
+					// Add the whitespace back
+					command.append(" ");
+					hasSpace = true;
+				}
+			}
+		}
+		
+//		if (hasSpace == true) {
+//			return "\"" + command.toString().trim() + "\"";
+//		} else {
+			return command.toString().trim();
+//		}
+	}
+
 	/*
 	 *  (non-Javadoc)
 	 * @see org.eclipse.jface.preference.PreferencePage#performDefaults()

build/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/properties/ManageConfigDialog.java

@@ -95,8 +95,7 @@ public class ManageConfigDialog extends Dialog {
 		this.title = title;
 		this.managedTarget = target;
 		
-		// Figure out the default make command
+		setMakeCommand();
-		makeCommand = managedTarget.getMakeCommand();
 		
 		// Get the name of the build artifact
 		artifactExt = managedTarget.getArtifactExtension();
@@ -186,7 +185,7 @@ public class ManageConfigDialog extends Dialog {
 		buildArtifactExt.setFont(outputGroup.getFont());
 		buildArtifactExt.setText(artifactExt);
 		data = new GridData(GridData.FILL_HORIZONTAL);
-		data.widthHint = IDialogConstants.ENTRY_FIELD_WIDTH;
+		data.widthHint = (IDialogConstants.ENTRY_FIELD_WIDTH / 2);
 		buildArtifactExt.setLayoutData(data);
 		buildArtifactExt.addDisposeListener(new DisposeListener() {
 			public void widgetDisposed(DisposeEvent e) {
@@ -238,7 +237,7 @@ public class ManageConfigDialog extends Dialog {
 		currentConfigList = new List(currentComp, SWT.SINGLE|SWT.V_SCROLL|SWT.H_SCROLL|SWT.BORDER);
 		currentConfigList.setFont(currentComp.getFont());
 		data = new GridData(GridData.FILL_BOTH);
-		data.widthHint = IDialogConstants.ENTRY_FIELD_WIDTH;
+		data.widthHint = (IDialogConstants.ENTRY_FIELD_WIDTH / 2);
 		currentConfigList.setLayoutData(data);
 		currentConfigList.addDisposeListener(new DisposeListener() {
 			public void widgetDisposed(DisposeEvent event) {
@@ -306,7 +305,7 @@ public class ManageConfigDialog extends Dialog {
 		deletedConfigList = new List(deletedComp, SWT.SINGLE|SWT.V_SCROLL|SWT.H_SCROLL|SWT.BORDER);
 		deletedConfigList.setFont(deletedComp.getFont());
 		data = new GridData(GridData.FILL_BOTH);
-		data.widthHint = IDialogConstants.ENTRY_FIELD_WIDTH;
+		data.widthHint = (IDialogConstants.ENTRY_FIELD_WIDTH / 2);
 		deletedConfigList.setLayoutData(data);
 		deletedConfigList.addDisposeListener(new DisposeListener() {
 			public void widgetDisposed(DisposeEvent event) {
@@ -563,7 +562,27 @@ public class ManageConfigDialog extends Dialog {
 	 */
 	protected void handleUseDefaultPressed() {
 		// If the state of the button is unchecked, then we want to enable the edit widget
-		makeCommandEntry.setEditable(!makeCommandDefault.getSelection());
+		boolean checked = makeCommandDefault.getSelection();
+		if (checked == true) {
+			managedTarget.resetMakeCommand();
+			setMakeCommand();
+			makeCommandEntry.setText(makeCommand);
+			makeCommandEntry.setEditable(false);
+		} else {
+			makeCommandEntry.setEditable(true);
+		}
+	}
+
+	/*
+	 * 
+	 */
+	private void setMakeCommand() {
+		// Figure out the make command
+		makeCommand = managedTarget.getMakeCommand();
+		String makeArgs = managedTarget.getMakeArguments();
+		if (makeArgs.length() > 0) {
+			makeCommand += " " + makeArgs;
+		}
 	}
 
 	private void updateButtons() {