From e744550278131f5c6ee3cd98a75bd7aa4b9ad399 Mon Sep 17 00:00:00 2001 From: Doug Schaefer Date: Fri, 5 Jul 2019 10:25:15 -0400 Subject: [PATCH] Bug 549014 - Manage CMake toolchain files by toolchain Use the toolchain type and id as the key to the Map holding the toolchain files instead of the Path to the file. Also simplifies the build config since we can always figure out the toolchain file from the toolchain. Deprecate getting toolchain by path since that shouldn't be needed any more. Also took the opportunity to add javadoc to the interfaces. Change-Id: I11ae2ad8177a3f60399742c8c19576f85ea4b8c0 --- .../META-INF/MANIFEST.MF | 2 +- .../cdt/cmake/core/ICMakeToolChainFile.java | 23 ++++ .../cmake/core/ICMakeToolChainManager.java | 51 +++++++++ .../cmake/core/ICMakeToolChainProvider.java | 6 + .../internal/CMakeBuildConfiguration.java | 33 +----- .../core/internal/CMakeToolChainManager.java | 105 ++++++++++-------- .../META-INF/MANIFEST.MF | 2 +- .../ui/internal/CMakePreferencePage.java | 45 +++++--- 8 files changed, 172 insertions(+), 95 deletions(-) diff --git a/build/org.eclipse.cdt.cmake.core/META-INF/MANIFEST.MF b/build/org.eclipse.cdt.cmake.core/META-INF/MANIFEST.MF index e03b41f4a61..9f085a6fbc3 100644 --- a/build/org.eclipse.cdt.cmake.core/META-INF/MANIFEST.MF +++ b/build/org.eclipse.cdt.cmake.core/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: CDT CMake Core Bundle-SymbolicName: org.eclipse.cdt.cmake.core;singleton:=true -Bundle-Version: 1.2.100.qualifier +Bundle-Version: 1.2.200.qualifier Bundle-Activator: org.eclipse.cdt.cmake.core.internal.Activator Bundle-Vendor: Eclipse CDT Require-Bundle: org.eclipse.core.runtime, diff --git a/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/ICMakeToolChainFile.java b/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/ICMakeToolChainFile.java index e1fe0c701e1..89ef37746fd 100644 --- a/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/ICMakeToolChainFile.java +++ b/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/ICMakeToolChainFile.java @@ -23,12 +23,35 @@ import org.eclipse.core.runtime.CoreException; */ public interface ICMakeToolChainFile { + /** + * Return the path to the toolchain file. + * + * @return path to the toolchain file + */ Path getPath(); + /** + * Return the value for a property. + * + * @param key key of the property + * @return the value + */ String getProperty(String key); + /** + * Set a property. + * + * @param key key of the property + * @param value value for the property + */ void setProperty(String key, String value); + /** + * Return the toolchain that this toolchain file enables. + * + * @return the toolchain for the file + * @throws CoreException + */ IToolChain getToolChain() throws CoreException; } diff --git a/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/ICMakeToolChainManager.java b/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/ICMakeToolChainManager.java index e0236ac0b42..f78a496bce2 100644 --- a/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/ICMakeToolChainManager.java +++ b/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/ICMakeToolChainManager.java @@ -24,22 +24,73 @@ import org.eclipse.cdt.core.build.IToolChain; */ public interface ICMakeToolChainManager { + /** + * Create a new toolchain file object to be added later. + * + * @param path path where the toolchain file resides. + * @return new toolchain objecta + */ ICMakeToolChainFile newToolChainFile(Path path); + /** + * Make the given toolchain file available. Also persists the path + * and properties for the file so it's recreated on startup. + * + * @param file the toolchain file to be added + */ void addToolChainFile(ICMakeToolChainFile file); + /** + * Remove the given toolchain file. + * + * @param file the toolchain file to be removed + */ void removeToolChainFile(ICMakeToolChainFile file); + /** + * We no longer use Path as a key so trying to remove the use of this method. + * Get by toolchain instead. + * @deprecated + * @return just returns null + */ + @Deprecated ICMakeToolChainFile getToolChainFile(Path path); + /** + * Return toolchain files that are applicable to toolchains with the given properties. + * + * @param properties properties to match + * @return toolchain files that do + */ Collection getToolChainFilesMatching(Map properties); + /** + * Return the toolchain file for the given toolchain. + * + * @param toolchain the toolchain + * @return the toolchain file for the toolchain + */ ICMakeToolChainFile getToolChainFileFor(IToolChain toolchain); + /** + * Return all available toolchain files. + * + * @return all toolchain files + */ Collection getToolChainFiles(); + /** + * Add a listener + * + * @param listener the listener + */ void addListener(ICMakeToolChainListener listener); + /** + * Remove a listener + * + * @param listener the listener + */ void removeListener(ICMakeToolChainListener listener); } diff --git a/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/ICMakeToolChainProvider.java b/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/ICMakeToolChainProvider.java index 9e34d40cf0e..d8d18ce1050 100644 --- a/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/ICMakeToolChainProvider.java +++ b/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/ICMakeToolChainProvider.java @@ -12,6 +12,12 @@ package org.eclipse.cdt.cmake.core; public interface ICMakeToolChainProvider { + /** + * Allows the provider to add any automatic toolchain files so the user + * doesn't have to. + * + * @param manager the manager object used to add toolchain files + */ void init(ICMakeToolChainManager manager); } diff --git a/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/internal/CMakeBuildConfiguration.java b/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/internal/CMakeBuildConfiguration.java index 99908efd71d..b695684af21 100644 --- a/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/internal/CMakeBuildConfiguration.java +++ b/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/internal/CMakeBuildConfiguration.java @@ -15,7 +15,6 @@ import java.io.FileReader; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -40,8 +39,6 @@ import org.eclipse.core.resources.IResource; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.jobs.Job; -import org.osgi.service.prefs.BackingStoreException; -import org.osgi.service.prefs.Preferences; import com.google.gson.Gson; @@ -53,25 +50,13 @@ public class CMakeBuildConfiguration extends CBuildConfiguration { public static final String BUILD_COMMAND = "cmake.command.build"; //$NON-NLS-1$ public static final String CLEAN_COMMAND = "cmake.command.clean"; //$NON-NLS-1$ - private static final String TOOLCHAIN_FILE = "cdt.cmake.toolchainfile"; //$NON-NLS-1$ - private ICMakeToolChainFile toolChainFile; public CMakeBuildConfiguration(IBuildConfiguration config, String name) throws CoreException { super(config, name); ICMakeToolChainManager manager = Activator.getService(ICMakeToolChainManager.class); - Preferences settings = getSettings(); - String pathStr = settings.get(TOOLCHAIN_FILE, ""); //$NON-NLS-1$ - if (!pathStr.isEmpty()) { - Path path = Paths.get(pathStr); - toolChainFile = manager.getToolChainFile(path); - } else { - toolChainFile = manager.getToolChainFileFor(getToolChain()); - if (toolChainFile != null) { - saveToolChainFile(); - } - } + toolChainFile = manager.getToolChainFileFor(getToolChain()); } public CMakeBuildConfiguration(IBuildConfiguration config, String name, IToolChain toolChain) { @@ -81,27 +66,13 @@ public class CMakeBuildConfiguration extends CBuildConfiguration { public CMakeBuildConfiguration(IBuildConfiguration config, String name, IToolChain toolChain, ICMakeToolChainFile toolChainFile, String launchMode) { super(config, name, toolChain, launchMode); - this.toolChainFile = toolChainFile; - if (toolChainFile != null) { - saveToolChainFile(); - } } public ICMakeToolChainFile getToolChainFile() { return toolChainFile; } - private void saveToolChainFile() { - Preferences settings = getSettings(); - settings.put(TOOLCHAIN_FILE, toolChainFile.getPath().toString()); - try { - settings.flush(); - } catch (BackingStoreException e) { - Activator.log(e); - } - } - private boolean isLocal() throws CoreException { IToolChain toolchain = getToolChain(); return (Platform.getOS().equals(toolchain.getProperty(IToolChain.ATTR_OS)) @@ -225,7 +196,7 @@ public class CMakeBuildConfiguration extends CBuildConfiguration { command.add("cmake"); //$NON-NLS-1$ command.add("--build"); //$NON-NLS-1$ command.add("."); //$NON-NLS-1$ - if ("Ninja".equals(generator)) { + if ("Ninja".equals(generator)) { //$NON-NLS-1$ command.add("--"); //$NON-NLS-1$ command.add("-v"); //$NON-NLS-1$ } diff --git a/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/internal/CMakeToolChainManager.java b/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/internal/CMakeToolChainManager.java index 7a5546d10d4..f7f2088e983 100644 --- a/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/internal/CMakeToolChainManager.java +++ b/build/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/internal/CMakeToolChainManager.java @@ -28,6 +28,7 @@ import org.eclipse.cdt.cmake.core.ICMakeToolChainListener; import org.eclipse.cdt.cmake.core.ICMakeToolChainManager; import org.eclipse.cdt.cmake.core.ICMakeToolChainProvider; import org.eclipse.cdt.core.build.IToolChain; +import org.eclipse.cdt.core.build.IToolChainManager; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IConfigurationElement; import org.eclipse.core.runtime.IExtensionPoint; @@ -40,7 +41,7 @@ import org.osgi.service.prefs.Preferences; public class CMakeToolChainManager implements ICMakeToolChainManager { - private Map files; + private Map filesByToolChain; private static final String N = "n"; //$NON-NLS-1$ private static final String PATH = "__path"; //$NON-NLS-1$ @@ -51,17 +52,29 @@ public class CMakeToolChainManager implements ICMakeToolChainManager { return InstanceScope.INSTANCE.getNode(Activator.getId()).node("cmakeToolchains"); //$NON-NLS-1$ } - private void init() { - if (files == null) { - files = new HashMap<>(); + public static String makeToolChainId(String tcType, String tcId) { + return tcType + '/' + tcId; + } + + public static String makeToolChainId(IToolChain toolchain) { + return makeToolChainId(toolchain.getTypeId(), toolchain.getId()); + } + + private synchronized void init() { + if (filesByToolChain == null) { + filesByToolChain = new HashMap<>(); Preferences prefs = getPreferences(); try { for (String childName : prefs.childrenNames()) { Preferences tcNode = prefs.node(childName); String pathStr = tcNode.get(PATH, "/"); //$NON-NLS-1$ + String tcType = tcNode.get(CMakeBuildConfiguration.TOOLCHAIN_TYPE, "?"); //$NON-NLS-1$ + String tcId = tcNode.get(CMakeBuildConfiguration.TOOLCHAIN_ID, "?"); //$NON-NLS-1$ Path path = Paths.get(pathStr); - if (Files.exists(path) && !files.containsKey(path)) { + IToolChainManager tcManager = Activator.getService(IToolChainManager.class); + IToolChain toolchain = tcManager.getToolChain(tcType, tcId); + if (toolchain != null && Files.exists(path)) { ICMakeToolChainFile file = new CMakeToolChainFile(childName, path); for (String key : tcNode.keys()) { String value = tcNode.get(key, ""); //$NON-NLS-1$ @@ -69,13 +82,13 @@ public class CMakeToolChainManager implements ICMakeToolChainManager { file.setProperty(key, value); } } - files.put(path, file); + filesByToolChain.put(makeToolChainId(tcType, tcId), file); } else { tcNode.removeNode(); prefs.flush(); } } - } catch (BackingStoreException e) { + } catch (BackingStoreException | CoreException e) { Activator.log(e); } @@ -104,42 +117,48 @@ public class CMakeToolChainManager implements ICMakeToolChainManager { @Override public void addToolChainFile(ICMakeToolChainFile file) { init(); - if (files.containsKey(file.getPath())) { - removeToolChainFile(file); - } - files.put(file.getPath(), file); - - // save it - - CMakeToolChainFile realFile = (CMakeToolChainFile) file; - Preferences prefs = getPreferences(); - String n = realFile.n; - if (n == null) { - n = prefs.get(N, "0"); //$NON-NLS-1$ - realFile.n = n; - } - prefs.put(N, Integer.toString(Integer.parseInt(n) + 1)); - - Preferences tcNode = prefs.node(n); - tcNode.put(PATH, file.getPath().toString()); - for (Entry entry : realFile.properties.entrySet()) { - tcNode.put(entry.getKey(), entry.getValue()); - } - try { - prefs.flush(); - } catch (BackingStoreException e) { - Activator.log(e); - } + IToolChain toolchain = file.getToolChain(); + String tcId = makeToolChainId(toolchain); + if (filesByToolChain.containsKey(tcId)) { + removeToolChainFile(file); + } + filesByToolChain.put(tcId, file); - fireEvent(new CMakeToolChainEvent(CMakeToolChainEvent.ADDED, file)); + // save it + CMakeToolChainFile realFile = (CMakeToolChainFile) file; + Preferences prefs = getPreferences(); + String n = realFile.n; + if (n == null) { + n = prefs.get(N, "0"); //$NON-NLS-1$ + realFile.n = n; + } + prefs.put(N, Integer.toString(Integer.parseInt(n) + 1)); + + Preferences tcNode = prefs.node(n); + tcNode.put(PATH, file.getPath().toString()); + for (Entry entry : realFile.properties.entrySet()) { + tcNode.put(entry.getKey(), entry.getValue()); + } + tcNode.put(CMakeBuildConfiguration.TOOLCHAIN_TYPE, toolchain.getTypeId()); + tcNode.put(CMakeBuildConfiguration.TOOLCHAIN_ID, toolchain.getId()); + + prefs.flush(); + + fireEvent(new CMakeToolChainEvent(CMakeToolChainEvent.ADDED, file)); + } catch (CoreException | BackingStoreException e) { + Activator.log(e); + return; + } } @Override public void removeToolChainFile(ICMakeToolChainFile file) { init(); fireEvent(new CMakeToolChainEvent(CMakeToolChainEvent.REMOVED, file)); - files.remove(file.getPath()); + String tcId = makeToolChainId(file.getProperty(CMakeBuildConfiguration.TOOLCHAIN_TYPE), + file.getProperty(CMakeBuildConfiguration.TOOLCHAIN_ID)); + filesByToolChain.remove(tcId); String n = ((CMakeToolChainFile) file).n; if (n != null) { @@ -157,13 +176,13 @@ public class CMakeToolChainManager implements ICMakeToolChainManager { @Override public ICMakeToolChainFile getToolChainFile(Path path) { init(); - return files.get(path); + return null; } @Override public Collection getToolChainFiles() { init(); - return Collections.unmodifiableCollection(files.values()); + return Collections.unmodifiableCollection(filesByToolChain.values()); } @Override @@ -187,15 +206,9 @@ public class CMakeToolChainManager implements ICMakeToolChainManager { @Override public ICMakeToolChainFile getToolChainFileFor(IToolChain toolchain) { - String id = toolchain.getId(); - - for (ICMakeToolChainFile file : getToolChainFiles()) { - if (id.equals(file.getProperty(CMakeBuildConfiguration.TOOLCHAIN_ID))) { - return file; - } - } - - return null; + init(); + String id = makeToolChainId(toolchain); + return filesByToolChain.get(id); } @Override diff --git a/build/org.eclipse.cdt.cmake.ui/META-INF/MANIFEST.MF b/build/org.eclipse.cdt.cmake.ui/META-INF/MANIFEST.MF index b7c393b5118..95db8a13243 100644 --- a/build/org.eclipse.cdt.cmake.ui/META-INF/MANIFEST.MF +++ b/build/org.eclipse.cdt.cmake.ui/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: CDT CMake UI Bundle-SymbolicName: org.eclipse.cdt.cmake.ui;singleton:=true -Bundle-Version: 1.2.1.qualifier +Bundle-Version: 1.2.100.qualifier Bundle-Activator: org.eclipse.cdt.cmake.ui.internal.Activator Bundle-Vendor: Eclipse CDT Require-Bundle: org.eclipse.core.runtime, diff --git a/build/org.eclipse.cdt.cmake.ui/src/org/eclipse/cdt/cmake/ui/internal/CMakePreferencePage.java b/build/org.eclipse.cdt.cmake.ui/src/org/eclipse/cdt/cmake/ui/internal/CMakePreferencePage.java index a0e1fcf7a73..4c85aab6ae4 100644 --- a/build/org.eclipse.cdt.cmake.ui/src/org/eclipse/cdt/cmake/ui/internal/CMakePreferencePage.java +++ b/build/org.eclipse.cdt.cmake.ui/src/org/eclipse/cdt/cmake/ui/internal/CMakePreferencePage.java @@ -19,6 +19,7 @@ import java.util.Map; import org.eclipse.cdt.cmake.core.ICMakeToolChainFile; import org.eclipse.cdt.cmake.core.ICMakeToolChainManager; +import org.eclipse.cdt.cmake.core.internal.CMakeToolChainManager; import org.eclipse.cdt.core.build.IToolChain; import org.eclipse.core.runtime.CoreException; import org.eclipse.jface.dialogs.MessageDialog; @@ -102,13 +103,18 @@ public class CMakePreferencePage extends PreferencePage implements IWorkbenchPre NewCMakeToolChainFileWizard wizard = new NewCMakeToolChainFileWizard(); WizardDialog dialog = new WizardDialog(getShell(), wizard); if (dialog.open() == Window.OK) { - ICMakeToolChainFile file = wizard.getNewFile(); - ICMakeToolChainFile oldFile = manager.getToolChainFile(file.getPath()); - if (oldFile != null) { - filesToRemove.put(oldFile.getPath(), oldFile); + try { + ICMakeToolChainFile file = wizard.getNewFile(); + IToolChain oldtc = file.getToolChain(); + ICMakeToolChainFile oldFile = manager.getToolChainFileFor(oldtc); + if (oldFile != null) { + filesToRemove.put(oldFile.getPath(), oldFile); + } + filesToAdd.put(file.getPath(), file); + updateTable(); + } catch (CoreException ex) { + Activator.log(ex); } - filesToAdd.put(file.getPath(), file); - updateTable(); } } }); @@ -159,18 +165,25 @@ public class CMakePreferencePage extends PreferencePage implements IWorkbenchPre } } - private Map getFiles() { - Map files = new HashMap<>(); - for (ICMakeToolChainFile file : manager.getToolChainFiles()) { - files.put(file.getPath(), file); - } + private Map getFiles() { + Map files = new HashMap<>(); + try { + for (ICMakeToolChainFile file : manager.getToolChainFiles()) { + String id = CMakeToolChainManager.makeToolChainId(file.getToolChain()); + files.put(id, file); + } - for (ICMakeToolChainFile file : filesToRemove.values()) { - files.remove(file.getPath()); - } + for (ICMakeToolChainFile file : filesToRemove.values()) { + String id = CMakeToolChainManager.makeToolChainId(file.getToolChain()); + files.remove(id); + } - for (ICMakeToolChainFile file : filesToAdd.values()) { - files.put(file.getPath(), file); + for (ICMakeToolChainFile file : filesToAdd.values()) { + String id = CMakeToolChainManager.makeToolChainId(file.getToolChain()); + files.put(id, file); + } + } catch (CoreException e) { + Activator.log(e); } return files;