bug 396411: Combined patch from master (fixes workspace relative paths from MBS)

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

@@ -122,22 +122,22 @@ public class BuildSystemTestHelper {
 		return project;
 	}
 
-	static public void checkDiff(Object[] arr1, Object[] arr2){
+	static public void checkDiff(Object[] expected, Object[] actual){
-		LinkedHashSet<? extends Object> set1 = new LinkedHashSet<Object>(Arrays.asList(arr1));
+		LinkedHashSet<? extends Object> set1 = new LinkedHashSet<Object>(Arrays.asList(expected));
-		LinkedHashSet<? extends Object> set2 = new LinkedHashSet<Object>(Arrays.asList(arr2));
+		LinkedHashSet<? extends Object> set2 = new LinkedHashSet<Object>(Arrays.asList(actual));
 		LinkedHashSet<? extends Object> set1Copy = new LinkedHashSet<Object>(set1);
 		set1.removeAll(set2);
 		set2.removeAll(set1Copy);
 
 		String set1String = collectionToString(set1);
 		String set2String = collectionToString(set2);
-		String diffMsg = "array1 entries: " + set1String + ",\n array2 entries: " + set2String + "\n";
+		String diffMsg = "expected entries: " + set1String + ",\n actual entries: " + set2String + "\n";
-		Assert.assertEquals("arrays have different size\n" + diffMsg, arr1.length, arr2.length);
+		Assert.assertEquals("arrays have different size\n" + diffMsg, expected.length, actual.length);
-		Assert.assertEquals("arrays have different contents\n" + diffMsg, 0, set1.size());
+		Assert.assertTrue("arrays have different contents\n" + diffMsg, set1.size() == 0);
-		Assert.assertEquals("arrays have different contents\n" + diffMsg, 0, set2.size());
+		Assert.assertTrue("arrays have different contents\n" + diffMsg, set2.size() == 0);
 
-		if(!Arrays.equals(arr1, arr2)){
+		if(!Arrays.equals(expected, actual)){
-			Assert.fail("different element order, dumping..\n array1 entries: " + arrayToString(arr1) + "\n array2 entries: " + arrayToString(arr2) + "\n");
+			Assert.fail("different element order, dumping..\n expected entries: " + arrayToString(expected) + "\n actual entries: " + arrayToString(actual) + "\n");
 		}
 	}
 

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

@@ -10,6 +10,8 @@
  *******************************************************************************/
 package org.eclipse.cdt.managedbuilder.core.tests;
 
+import java.io.File;
+import java.io.IOException;
 import java.util.Arrays;
 import java.util.Map;
 import java.util.Properties;
@@ -22,6 +24,7 @@ import org.eclipse.cdt.core.CCorePlugin;
 import org.eclipse.cdt.core.parser.IScannerInfo;
 import org.eclipse.cdt.core.parser.IScannerInfoChangeListener;
 import org.eclipse.cdt.core.parser.IScannerInfoProvider;
+import org.eclipse.cdt.internal.core.language.settings.providers.LanguageSettingsScannerInfoProvider;
 import org.eclipse.cdt.managedbuilder.core.BuildException;
 import org.eclipse.cdt.managedbuilder.core.IConfiguration;
 import org.eclipse.cdt.managedbuilder.core.IManagedBuildInfo;
@@ -198,6 +201,26 @@ public class ManagedBuildCoreTests20 extends TestCase {
 	}
 
 
+	/**
+	 * Convert path to OS specific representation
+	 */
+	private String toOSLocation(String path) {
+		File file = new File(path);
+		try {
+			path = file.getCanonicalPath();
+		} catch (IOException e) {
+		}
+		
+		return path;
+	}
+
+	/**
+	 * Convert path to OS specific representation
+	 */
+	private String toOSString(String path) {
+		return new Path(path).toOSString();
+	}
+
 	/**
 	 * The purpose of this test is to exercise the build path info interface.
 	 * To get to that point, a new project/config has to be created in the test
@@ -218,24 +241,44 @@ public class ManagedBuildCoreTests20 extends TestCase {
 		}
 
 		//These are the expected path settings
-		 final String[] expectedPaths = new String[5];
+		IPath buildCWD = project.getLocation().append("Sub Config");
 
-		 // This first path is a built-in, so it will not be manipulated by build manager
+		final String[] expectedPaths;
-		 expectedPaths[0] = (new Path("/usr/include")).toOSString();
+		if (new Path("C:\\home\\tester/include").isAbsolute()) {
-		 expectedPaths[1] = (new Path("/opt/gnome/include")).toOSString();
+			// Windows
-		 IPath path = new Path("C:\\home\\tester/include");
+			expectedPaths = new String[] {
-		 if(path.isAbsolute()) // for win32 path is treated as absolute
+					toOSLocation("/usr/include"),
-			 expectedPaths[2] = path.toOSString();
+					toOSLocation("/opt/gnome/include"),
-		 else // for Linux path is relative
+					toOSLocation("C:\\home\\tester/include"),
-			 expectedPaths[2] = project.getLocation().append("Sub Config").append(path).toOSString();
+					// relative paths from MBS will make 3 entries
-		 expectedPaths[3] = project.getLocation().append( "includes" ).toOSString();
+					project.getLocation().append("includes").toOSString(),
-		 expectedPaths[4] = (new Path("/usr/gnu/include")).toOSString();
+					buildCWD.append("includes").toOSString(),
+					toOSString("includes"),
+					"/usr/gnu/include", // Not converted to OS string due to being flagged as ICSettingEntry.RESOLVED
+			};
+		} else {
+			// Unix
+			expectedPaths = new String[] {
+					toOSLocation("/usr/include"),
+					toOSLocation("/opt/gnome/include"),
+					// on unix "C:\\home\\tester/include" is relative path
+					// looks like nonsense but has to be this way as MBS converts entry to keep "Sub Config/C:\\home\\tester/include" in its storage
+					project.getLocation().append("Sub Config/C:\\home\\tester/include").toOSString(),
+					buildCWD.append("Sub Config/C:\\home\\tester/include").toOSString(),
+					toOSString("Sub Config/C:\\home\\tester/include"),
+					// relative paths from MBS will make 3 entries
+					project.getLocation().append("includes").toOSString(),
+					buildCWD.append("includes").toOSString(),
+					toOSString("includes"),
+					"/usr/gnu/include", // Not converted to OS string due to being flagged as ICSettingEntry.RESOLVED
+			};
+		}
 
 		// Create a new managed project based on the sub project type
 		IProjectType projType = ManagedBuildManager.getExtensionProjectType("test.sub");
 		assertNotNull(projType);
 
-		// Create the managed-project (.cdtbuild) for our project
+		// Create the managed-project for our project
 		IManagedProject newProject = null;
 		try {
 			newProject = ManagedBuildManager.createManagedProject(project, projType);
@@ -281,6 +324,7 @@ public class ManagedBuildCoreTests20 extends TestCase {
 		// Find the first IScannerInfoProvider that supplies build info for the project
 		IScannerInfoProvider provider = CCorePlugin.getDefault().getScannerInfoProvider(project);
 		assertNotNull(provider);
+		assertTrue(provider instanceof LanguageSettingsScannerInfoProvider);
 
 		// Now subscribe (note that the method will be called after a change
 		provider.subscribe(project, new IScannerInfoChangeListener () {
@@ -515,7 +559,7 @@ public class ManagedBuildCoreTests20 extends TestCase {
 		IProjectType projType = ManagedBuildManager.getExtensionProjectType("test.root");
 		assertNotNull(projType);
 
-		// Create the managed-project (.cdtbuild) for our project that builds a dummy executable
+		// Create the managed-project for our project that builds a dummy executable
 		IManagedProject newProject = ManagedBuildManager.createManagedProject(project, projType);
 		assertEquals(newProject.getName(), projType.getName());
 		assertFalse(newProject.equals(projType));

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

@@ -1841,11 +1841,10 @@ public class Builder extends HoldsOptions implements IBuilder, IMatchKeyProvider
 //				}
 				
 				if(!buildPath.isAbsolute()){
-					buildPath = project.getFullPath().append(buildPath);
 					IStringVariableManager mngr = VariablesPlugin.getDefault().getStringVariableManager();
-	
+					// build dir may not exist yet and non-existent paths will resolve to empty string by VariablesPlugin
-					result = buildPath.toString();
+					// so append relative part outside of expression, i.e. ${workspace_loc:/Project}/BuildDir
-					result = mngr.generateVariableExpression("workspace_loc", result); //$NON-NLS-1$
+					result = mngr.generateVariableExpression("workspace_loc", project.getFullPath().toString()) + Path.SEPARATOR + buildPath.toString(); //$NON-NLS-1$
 				} else {
 					result = buildPath.toString();
 				}

build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/internal/language/settings/providers/MBSLanguageSettingsProvider.java

@@ -22,11 +22,16 @@ import org.eclipse.cdt.core.settings.model.ICFileDescription;
 import org.eclipse.cdt.core.settings.model.ICFolderDescription;
 import org.eclipse.cdt.core.settings.model.ICLanguageSetting;
 import org.eclipse.cdt.core.settings.model.ICLanguageSettingEntry;
+import org.eclipse.cdt.core.settings.model.ICPathEntry;
 import org.eclipse.cdt.core.settings.model.ICResourceDescription;
 import org.eclipse.cdt.core.settings.model.ICSettingBase;
+import org.eclipse.cdt.core.settings.model.util.CDataUtil;
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IResource;
 import org.eclipse.core.runtime.IPath;
+import org.eclipse.core.runtime.Path;
+import org.eclipse.core.variables.IStringVariableManager;
+import org.eclipse.core.variables.VariablesPlugin;
 
 /**
  * Implementation of language settings provider for CDT Managed Build System.
@@ -56,11 +61,30 @@ public class MBSLanguageSettingsProvider extends AbstractExecutableExtensionBase
 			for (ICLanguageSetting langSetting : languageSettings) {
 				if (langSetting != null) {
 					String id = langSetting.getLanguageId();
-					if (id != null && id.equals(languageId)) {
+					if (id == languageId || (id != null && id.equals(languageId))) {
 						int kindsBits = langSetting.getSupportedEntryKinds();
 						for (int kind=1; kind <= kindsBits; kind <<= 1) {
 							if ((kindsBits & kind) != 0) {
-								list.addAll(langSetting.getSettingEntriesList(kind));
+								List<ICLanguageSettingEntry> additions = langSetting.getSettingEntriesList(kind);
+								for (ICLanguageSettingEntry entry : additions) {
+									if (entry instanceof ICPathEntry) {
+										// have to use getName() rather than getLocation() and not use IPath operations to avoid collapsing ".."
+										String pathStr = ((ICPathEntry) entry).getName();
+										if (!new Path(pathStr).isAbsolute()) {
+											// We need to add project-rooted entry for relative path as MBS counts it this way in some UI
+											// The relative entry below also should be added for indexer to resolve from source file locations
+											IStringVariableManager mngr = VariablesPlugin.getDefault().getStringVariableManager();
+											String projectRootedPath = mngr.generateVariableExpression("workspace_loc", rc.getProject().getName()) + Path.SEPARATOR + pathStr; //$NON-NLS-1$
+											ICLanguageSettingEntry projectRootedEntry = (ICLanguageSettingEntry) CDataUtil.createEntry(kind, projectRootedPath, projectRootedPath, null, entry.getFlags());
+											if (! list.contains(projectRootedEntry)) {
+												list.add(projectRootedEntry);
+											}
+										}
+									}
+									if (! list.contains(entry)) {
+										list.add(entry);
+									}
+								}
 							}
 						}
 					}

core/org.eclipse.cdt.core/model/org/eclipse/cdt/core/language/settings/providers/LanguageSettingsManager.java

@@ -184,7 +184,7 @@ public class LanguageSettingsManager {
 	 * commonly come from the input type(s).
 	 *
 	 * @param rcDescription - resource description
-	 * @return list of language IDs for the resource.
+	 * @return list of language IDs for the resource. The list can contain {@code null} ID.
 	 *    Never returns {@code null} but empty list if no languages can be found.
 	 *
 	 */
@@ -202,9 +202,9 @@ public class LanguageSettingsManager {
 		List<String> languageIds = new ArrayList<String>();
 		if (languageSettings != null) {
 			for (ICLanguageSetting languageSetting : languageSettings) {
-				if (languageSetting!=null) {
+				if (languageSetting != null) {
 					String languageId = languageSetting.getLanguageId();
-					if (languageId != null && !languageId.isEmpty()) {
+					if (! languageIds.contains(languageId)) {
 						languageIds.add(languageId);
 					}
 				}

core/org.eclipse.cdt.core/model/org/eclipse/cdt/core/settings/model/ICLanguageSetting.java

@@ -21,6 +21,9 @@ public interface ICLanguageSetting extends ICSettingObject {
 
 //	String[] getHeaderExtensions();
 
+	/**
+	 * @return language id. Note that that id can be {@code null}.
+	 */
 	String getLanguageId();
 	
 //	ICLanguageSettingEntry[] getSettingEntries();

core/org.eclipse.cdt.core/model/org/eclipse/cdt/core/settings/model/util/PathEntryTranslator.java

@@ -2051,9 +2051,11 @@ public class PathEntryTranslator {
 		IProject project = cfgDescription.getProjectDescription().getProject();
 		if (ScannerDiscoveryLegacySupport.isLanguageSettingsProvidersFunctionalityEnabled(project)) {
 			IResource rc = findResourceInWorkspace(project, rcData.getPath());
+			if (rc != null) {
 				for (CLanguageData lData : lDatas) {
 					list.addAll(LanguageSettingsProvidersSerializer.getSettingEntriesByKind(cfgDescription, rc, lData.getLanguageId(), kind));
 				}
+			}
 			return list.size()>0;
 
 		}

core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/language/settings/providers/LanguageSettingsScannerInfoProvider.java

@@ -30,10 +30,10 @@ import org.eclipse.cdt.core.parser.ExtendedScannerInfo;
 import org.eclipse.cdt.core.parser.IScannerInfo;
 import org.eclipse.cdt.core.parser.IScannerInfoChangeListener;
 import org.eclipse.cdt.core.parser.IScannerInfoProvider;
-import org.eclipse.cdt.core.settings.model.ACPathEntry;
 import org.eclipse.cdt.core.settings.model.ICConfigurationDescription;
 import org.eclipse.cdt.core.settings.model.ICLanguageSettingEntry;
 import org.eclipse.cdt.core.settings.model.ICMacroEntry;
+import org.eclipse.cdt.core.settings.model.ICPathEntry;
 import org.eclipse.cdt.core.settings.model.ICProjectDescription;
 import org.eclipse.cdt.core.settings.model.ICSettingEntry;
 import org.eclipse.cdt.core.settings.model.util.CDataUtil;
@@ -129,17 +129,15 @@ public class LanguageSettingsScannerInfoProvider implements IScannerInfoProvider
 		return new ExtendedScannerInfo(definedMacros, includePaths, macroFiles, includeFiles, includePathsLocal);
 	}
 
-	private IPath expandVariables(IPath path, ICConfigurationDescription cfgDescription) {
+	private String expandVariables(String pathStr, ICConfigurationDescription cfgDescription) {
-		ICdtVariableManager varManager = CCorePlugin.getDefault().getCdtVariableManager();
-		String pathStr = path.toString();
 		try {
+			ICdtVariableManager varManager = CCorePlugin.getDefault().getCdtVariableManager();
 			pathStr = varManager.resolveValue(pathStr, "", null, cfgDescription); //$NON-NLS-1$
-		} catch (CdtVariableException e) {
+		} catch (Exception e) {
 			// Swallow exceptions but also log them
 			CCorePlugin.log(e);
 		}
-		IPath resolvedLoc = new Path(pathStr);
+		return pathStr;
-		return resolvedLoc;
 	}
 
 	/**
@@ -148,7 +146,7 @@ public class LanguageSettingsScannerInfoProvider implements IScannerInfoProvider
 	 */
 	private static IPath getBuildCWD(ICConfigurationDescription cfgDescription) {
 		IPath buildCWD = cfgDescription.getBuildSetting().getBuilderCWD();
-		if (buildCWD==null) {
+		if (buildCWD == null) {
 			IProject project = cfgDescription.getProjectDescription().getProject();
 			buildCWD = project.getLocation();
 		} else {
@@ -159,7 +157,12 @@ public class LanguageSettingsScannerInfoProvider implements IScannerInfoProvider
 				// still, MBS does that and we need to handle that
 				String buildPathString = buildCWD.toString();
 				buildPathString = mngr.resolveValue(buildPathString, "", null, cfgDescription); //$NON-NLS-1$
+				if (!buildPathString.isEmpty()) {
 					buildCWD = new Path(buildPathString);
+				} else {
+					IProject project = cfgDescription.getProjectDescription().getProject();
+					buildCWD = project.getLocation();
+				}
 			} catch (CdtVariableException e) {
 				CCorePlugin.log(e);
 			}
@@ -187,28 +190,44 @@ public class LanguageSettingsScannerInfoProvider implements IScannerInfoProvider
 			CCorePlugin.log(e);
 		}
 		// use OS file separators (i.e. '\' on Windows)
-		if (java.io.File.separatorChar != '/') {
+		if (java.io.File.separatorChar != IPath.SEPARATOR) {
-			location = location.replace('/', java.io.File.separatorChar);
+			location = location.replace(IPath.SEPARATOR, java.io.File.separatorChar);
 		}
 
+		IPath locPath = new Path(location);
+		if (locPath.isAbsolute() && locPath.getDevice() == null) {
+			IPath buildCWD = getBuildCWD(cfgDescription);
+			// prepend device (C:) for Windows
+			String device = buildCWD.getDevice();
+			if (device != null) {
 				// note that we avoid using org.eclipse.core.runtime.Path for manipulations being careful
 				// to preserve "../" segments and not let collapsing them which is not correct for symbolic links.
-		Path locPath = new Path(location);
-		if (locPath.isAbsolute() && locPath.getDevice()==null) {
-			// prepend device (C:) for Windows
-			IPath buildCWD = getBuildCWD(cfgDescription);
-			String device = buildCWD.getDevice();
-			if (device!=null)
 				location = device + location;
 			}
+		}
+
 		if (!locPath.isAbsolute()) {
 			// consider relative path to be from build working directory
 			IPath buildCWD = getBuildCWD(cfgDescription);
-			location = buildCWD.toOSString() + locPath;
+			// again, we avoid using org.eclipse.core.runtime.Path for manipulations being careful
+			// to preserve "../" segments and not let collapsing them which is not correct for symbolic links.
+			location = buildCWD.addTrailingSeparator().toOSString() + location;
 		}
 		return location;
 	}
 
+	/**
+	 * Convert path delimiters to OS representation avoiding using org.eclipse.core.runtime.Path
+	 * being careful to preserve "../" segments and not let collapsing them which is not correct for symbolic links.
+	 */
+	private String toOSString(String loc) {
+		// use OS file separators (i.e. '\' on Windows)
+		if (java.io.File.separatorChar != IPath.SEPARATOR) {
+			loc = loc.replace(IPath.SEPARATOR, java.io.File.separatorChar);
+		}
+		return loc;
+	}
+
 	/**
 	 * Convert the path entries to absolute file system locations represented as String array.
 	 * Resolve the entries which are not resolved.
@@ -217,10 +236,10 @@ public class LanguageSettingsScannerInfoProvider implements IScannerInfoProvider
 	 * @param cfgDescription - configuration description for resolving entries.
 	 * @return array of the locations.
 	 */
-	private String[] convertToLocations(LinkedHashSet<ICLanguageSettingEntry> entriesPath, ICConfigurationDescription cfgDescription){
+	private String[] convertToLocations(LinkedHashSet<ICLanguageSettingEntry> entriesPath, ICConfigurationDescription cfgDescription) {
 		List<String> locations = new ArrayList<String>(entriesPath.size());
 		for (ICLanguageSettingEntry entry : entriesPath) {
-			ACPathEntry entryPath = (ACPathEntry)entry;
+			ICPathEntry entryPath = (ICPathEntry)entry;
 			if (entryPath.isValueWorkspacePath()) {
 				ICLanguageSettingEntry[] entries = new ICLanguageSettingEntry[] {entry};
 				if (!entry.isResolved()) {
@@ -228,7 +247,7 @@ public class LanguageSettingsScannerInfoProvider implements IScannerInfoProvider
 				}
 
 				for (ICLanguageSettingEntry resolved : entries) {
-					IPath loc = ((ACPathEntry) resolved).getLocation();
+					IPath loc = ((ICPathEntry) resolved).getLocation();
 					if (loc != null) {
 						if (checkBit(resolved.getFlags(), ICSettingEntry.FRAMEWORKS_MAC)) {
 							// handle frameworks, see IScannerInfo.getIncludePaths()
@@ -240,24 +259,25 @@ public class LanguageSettingsScannerInfoProvider implements IScannerInfoProvider
 					}
 				}
 			} else {
-				String locStr = entryPath.getName();
+				// have to use getName() rather than getLocation() to avoid collapsing ".."
+				String loc = entryPath.getName();
 				if (entryPath.isResolved()) {
-					locations.add(locStr);
+					locations.add(loc);
 				} else {
-					locStr = resolveEntry(locStr, cfgDescription);
+					loc = resolveEntry(loc, cfgDescription);
-					if (locStr!=null) {
+					if (loc != null) {
 						if (checkBit(entryPath.getFlags(), ICSettingEntry.FRAMEWORKS_MAC)) {
 							// handle frameworks, see IScannerInfo.getIncludePaths()
-							locations.add(locStr+FRAMEWORK_HEADERS_INCLUDE);
+							locations.add(toOSString(loc + FRAMEWORK_HEADERS_INCLUDE));
-							locations.add(locStr+FRAMEWORK_PRIVATE_HEADERS_INCLUDE);
+							locations.add(toOSString(loc + FRAMEWORK_PRIVATE_HEADERS_INCLUDE));
 						} else {
-							locations.add(locStr);
+							locations.add(toOSString(loc));
+							String unresolvedPath = entryPath.getName();
+							if (!new Path(unresolvedPath).isAbsolute()) {
 								// add relative paths again for indexer to resolve from source file location
-							IPath unresolvedPath = entryPath.getLocation();
+								String expandedPath = expandVariables(unresolvedPath, cfgDescription);
-							if (!unresolvedPath.isAbsolute()) {
+								if (!expandedPath.isEmpty() && !new Path(expandedPath).isAbsolute()) {
-								IPath expandedPath = expandVariables(unresolvedPath, cfgDescription);
+									locations.add(toOSString(expandedPath));
-								if (!expandedPath.isAbsolute()) {
-									locations.add(expandedPath.toOSString());
 								}
 							}
 						}

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/language/settings/providers/LanguageSettingsEntriesTab.java

@@ -968,6 +968,8 @@ public class LanguageSettingsEntriesTab extends AbstractCPropertyTab {
 		currentLanguageId = null;
 
 		List<String> languageIds = LanguageSettingsManager.getLanguages(rcDes);
+		// Not sure what to do with null language ID, ignoring for now
+		languageIds.remove(null);
 		Collections.sort(languageIds);
 		for (String langId : languageIds) {
 			ILanguage language = LanguageManager.getInstance().getLanguage(langId);
@@ -1124,10 +1126,13 @@ public class LanguageSettingsEntriesTab extends AbstractCPropertyTab {
 		if (page.isForFile()) {
 			List<String> languageIds = LanguageSettingsManager.getLanguages(getResDesc());
 			for (String langId : languageIds) {
+				if (langId != null) {
 					ILanguage language = LanguageManager.getInstance().getLanguage(langId);
-				if (language != null)
+					if (language != null) {
 						return true;
 					}
+				}
+			}
 			return false;
 		}