From a7e6cc8f4f91a006a72fcb4052bd0310c8b604c4 Mon Sep 17 00:00:00 2001 From: Daniel Gomez-Sanchez Date: Wed, 3 Nov 2021 15:02:48 +0100 Subject: [PATCH 1/2] Add Eclipse launcher command line to info logging Also cleanup TODOs on org.eclipse.gem.oomph plug-in adding issue numbers. --- .../org/aposin/gem/oomph/EclipseLauncher.java | 30 +++++++------------ .../gem/oomph/EclipseLauncherProvider.java | 3 +- .../internal/config/bean/GemEclipseBean.java | 2 -- 3 files changed, 12 insertions(+), 23 deletions(-) diff --git a/bundles/org.aposin.gem.oomph/src/org/aposin/gem/oomph/EclipseLauncher.java b/bundles/org.aposin.gem.oomph/src/org/aposin/gem/oomph/EclipseLauncher.java index 30a89e0..03cc147 100644 --- a/bundles/org.aposin.gem.oomph/src/org/aposin/gem/oomph/EclipseLauncher.java +++ b/bundles/org.aposin.gem.oomph/src/org/aposin/gem/oomph/EclipseLauncher.java @@ -28,6 +28,7 @@ import java.util.Collections; import java.util.List; import java.util.Properties; +import java.util.stream.Collectors; import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilderFactory; @@ -62,16 +63,9 @@ import org.xml.sax.helpers.DefaultHandler; /** - * This starter uses Oomph to setup Eclipse and it's workspace. - * - * TODO: The XML files are manually created and saved for Oomph, so the workspace startup works as - * expected. Instead, the Oomph library should be used if possible. Research has to be done how this - * can be implemented. - * - * TODO: The Eclipse installation is fixed and not configured via Oomph. Each setup should provides - * it's own Eclipse installation and workspace. This is useful if bundle pooling is used. Research - * has to be done how this can be implemented. + * Launcher to start and setup an Eclipse workspace with an already installed product and oomph configuration. */ +// TODO #41 - refactor to use the oomph libraries public class EclipseLauncher extends AbstractNoParamsLauncher implements IConfigurable { public static final String LAUNCHER_NAME = "eclipse"; @@ -172,7 +166,6 @@ public List launch() throws GemException { } final Path projectEnvironment = getConfiguration().getResourcesDirectory() // - // TODO: configurable or to static (I think that it is fine to be non-configurable) .resolve("eclipseworkspaces") // .resolve(getLaunchScope().getId()); @@ -182,7 +175,11 @@ public List launch() throws GemException { try { LOGGER.trace("About to run eclipse command"); - ExecUtils.exec(createCmd(eclipseFolder, projectEnvironment)); + final List cmd = createCmd(eclipseFolder, projectEnvironment); + if (LOGGER.isInfoEnabled()) { + LOGGER.info("Eclipse command: '{}'", cmd.stream().collect(Collectors.joining(" "))); + } + ExecUtils.exec(cmd); } catch (final IOException e) { LOGGER.error("Could not start Eclipse.", e); throw new GemException("Could not start eclipse.", e); @@ -208,7 +205,7 @@ private List createCmd(final Path eclipseFolder, final Path projectEnvir list.add("-Dosgi.configuration.area=file:/" + pathToString(projectEnvironment.resolve("eclipse").resolve("configuration"))); // should be quoted as it contains the special characters that might not work - list.add(quoteArg( + list.add(quoteArg( "-Doomph.redirection.index.redirection=index:/->" + "file:/" + pathToString(getConfiguration().getRelativeToConfigFile(getEclipseBean().oomphpath)) + '/')); @@ -367,7 +364,6 @@ private void createEclipseOomphSetup(final Path projectEnvironment) .resolve("configuration") // .resolve("org.eclipse.oomph.setup") // .resolve("installation.setup"); - // TODO - extend this if to also creation if (Files.notExists(eclipseSetup)) { LOGGER.trace("Creating Eclipse Oomph Setup '{}'", eclipseSetup); final Path eclipseWorkspace = eclipseSetup.getParent(); @@ -385,7 +381,6 @@ private void createEclipseOomphSetup(final Path projectEnvironment) final Transformer transformer = factory.newTransformer(); transformer.setOutputProperty(OutputKeys.INDENT, "yes"); transformer.setOutputProperty(OutputKeys.ENCODING, StandardCharsets.UTF_8.toString()); - // TODO - only write if it doesn't exists? transformer.transform(new DOMSource(document), new StreamResult(eclipseSetup.toFile())); LOGGER.trace("Eclipse Oomph Setup file '{}' created", eclipseSetup); } catch (ParserConfigurationException | TransformerException e) { @@ -432,7 +427,6 @@ private void createWorkspaceOomphSetup(final Path projectEnvironment) { .resolve(".plugins") // .resolve("org.eclipse.oomph.setup") // .resolve("workspace.setup"); - // TODO - extend this if to also creation if (Files.notExists(workspaceSetup)) { LOGGER.trace("Creating Workspace Oomph Setup '{}'", workspaceSetup); final Path eclipseWorkspace = workspaceSetup.getParent(); @@ -450,7 +444,6 @@ private void createWorkspaceOomphSetup(final Path projectEnvironment) { final Transformer transformer = factory.newTransformer(); transformer.setOutputProperty(OutputKeys.INDENT, "yes"); transformer.setOutputProperty(OutputKeys.ENCODING, StandardCharsets.UTF_8.toString()); - // TODO - only write if it doesn't exists? transformer.transform(new DOMSource(document), new StreamResult(workspaceSetup.toFile())); LOGGER.trace("Workspace Oomph Setup file '{}' created", workspaceSetup); @@ -553,10 +546,7 @@ public void startElement(String uri, String localName, String qName, }); if (foundProjects.isEmpty()) { - // TODO - if not present, the launcher should not appear instead of fail - // TODO - that will be much better in terms of checking if something - // TODO - is properly configured - // TODO - that should be done at the EclipseLauncherProvider level + // TODO: #43 - diable/misconfigure instead of failing on launch throw new GemException("Oomph project for setup not found."); } else { foundProjects.sort((p1, p2) -> Integer.compare(p2.length(), p1.length())); diff --git a/bundles/org.aposin.gem.oomph/src/org/aposin/gem/oomph/EclipseLauncherProvider.java b/bundles/org.aposin.gem.oomph/src/org/aposin/gem/oomph/EclipseLauncherProvider.java index 91c5b32..1cd8f07 100644 --- a/bundles/org.aposin.gem.oomph/src/org/aposin/gem/oomph/EclipseLauncherProvider.java +++ b/bundles/org.aposin.gem.oomph/src/org/aposin/gem/oomph/EclipseLauncherProvider.java @@ -19,6 +19,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; + import org.aposin.gem.core.api.config.IConfiguration; import org.aposin.gem.core.api.launcher.ILauncher; import org.aposin.gem.core.api.model.IEnvironment; @@ -52,7 +53,7 @@ public List getLaunchers(IEnvironment environment) { final int nReleases = eclipseBean.releases.size(); final List starters = new ArrayList<>(nReleases); for (int idx = 0; idx < nReleases; idx++) { - // TODO - implement a filter on the configuration for environments/projects + // TODO #42 - implement a filter on the configuration for environments/projects starters.add(new EclipseLauncher(this, environment, idx)); } return starters; diff --git a/bundles/org.aposin.gem.oomph/src/org/aposin/gem/oomph/impl/internal/config/bean/GemEclipseBean.java b/bundles/org.aposin.gem.oomph/src/org/aposin/gem/oomph/impl/internal/config/bean/GemEclipseBean.java index 8482e5c..d1b3e89 100644 --- a/bundles/org.aposin.gem.oomph/src/org/aposin/gem/oomph/impl/internal/config/bean/GemEclipseBean.java +++ b/bundles/org.aposin.gem.oomph/src/org/aposin/gem/oomph/impl/internal/config/bean/GemEclipseBean.java @@ -84,8 +84,6 @@ public static class EclipseReleaseBean { public String name; public String displayname; - // TODO - do we really need a list of paths for each Eclipse? - // TODO - I will suggest to use just one public List paths; public List args; public List vmargs; From 8846851617d5dc46682fe5dfc9d2cb371abc8aad Mon Sep 17 00:00:00 2001 From: Daniel Gomez-Sanchez Date: Wed, 3 Nov 2021 15:04:51 +0100 Subject: [PATCH 2/2] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 337f4af..e20a23a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning (SemVer)](http://semver.org/). * Plug-in interface to create several services based on configuration * New menu and dialog to detect and clean obsolete environments * Extension to provide sorting of projects/environments +* Log Eclipse launcher command-line to INFO ### Changed