From 5e4a66b0afcc48c833569e72689b36b2adeb28ae Mon Sep 17 00:00:00 2001 From: Mat Booth Date: Thu, 2 Jun 2022 10:15:42 +0100 Subject: [PATCH] Bug 580178 - Unable to stop build process from launchbar Switch from the standard Java ProcessBuilder to the CDT CommandLauncher for new style core build projects. The CommandLauncher uses a more sophiscated mechanism for watching the spawned process allowing us to interrupt the process when the user hits the stop button on the launchbar by properly listening to a monitor. The change adds new API to CBuildCongifuration that takes a progress monitor, and changes all the affected build configuration types to use this new API. Change-Id: I0c4225616ad8331c2cea28bcb502028455a8ea71 --- .../core/AutotoolsBuildConfiguration.java | 4 +- .../meson/core/MesonBuildConfiguration.java | 8 +- .../internal/CMakeBuildConfiguration.java | 15 +- .../org.eclipse.cdt.core/META-INF/MANIFEST.MF | 2 +- .../cdt/core/build/CBuildConfiguration.java | 152 ++++++------------ .../build/StandardBuildConfiguration.java | 6 +- .../qt/core/build/QtBuildConfiguration.java | 24 ++- 7 files changed, 73 insertions(+), 138 deletions(-) diff --git a/build/org.eclipse.cdt.core.autotools.core/src/org/eclipse/cdt/core/autotools/core/AutotoolsBuildConfiguration.java b/build/org.eclipse.cdt.core.autotools.core/src/org/eclipse/cdt/core/autotools/core/AutotoolsBuildConfiguration.java index f6497540771..4dc47efb12f 100644 --- a/build/org.eclipse.cdt.core.autotools.core/src/org/eclipse/cdt/core/autotools/core/AutotoolsBuildConfiguration.java +++ b/build/org.eclipse.cdt.core.autotools.core/src/org/eclipse/cdt/core/autotools/core/AutotoolsBuildConfiguration.java @@ -106,7 +106,7 @@ public class AutotoolsBuildConfiguration extends CBuildConfiguration { try { // TODO Error parsers Process process = builder.start(); - watchProcess(process, console); + watchProcess(console, monitor); } catch (IOException e) { throw new CoreException(Activator.errorStatus("Error executing: " + String.join(" ", command), e)); //$NON-NLS-1$ //$NON-NLS-2$ } @@ -153,7 +153,7 @@ public class AutotoolsBuildConfiguration extends CBuildConfiguration { Activator.errorStatus("Error executing: " + String.join(" ", command), null)); //$NON-NLS-1$ //$NON-NLS-2$ } - watchProcess(p, new IConsoleParser[] { epm }); + watchProcess(new IConsoleParser[] { epm }, monitor); } project.refreshLocal(IResource.DEPTH_INFINITE, monitor); diff --git a/build/org.eclipse.cdt.meson.core/src/org/eclipse/cdt/internal/meson/core/MesonBuildConfiguration.java b/build/org.eclipse.cdt.meson.core/src/org/eclipse/cdt/internal/meson/core/MesonBuildConfiguration.java index dc33eb56228..0be025a4838 100644 --- a/build/org.eclipse.cdt.meson.core/src/org/eclipse/cdt/internal/meson/core/MesonBuildConfiguration.java +++ b/build/org.eclipse.cdt.meson.core/src/org/eclipse/cdt/internal/meson/core/MesonBuildConfiguration.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2015, 2018 QNX Software Systems and others. + * Copyright (c) 2015, 2022 QNX Software Systems and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -183,7 +183,7 @@ public class MesonBuildConfiguration extends CBuildConfiguration { return null; } - watchProcess(p, console); + watchProcess(console, monitor); } if (!Files.exists(buildDir.resolve("build.ninja"))) { //$NON-NLS-1$ @@ -238,7 +238,7 @@ public class MesonBuildConfiguration extends CBuildConfiguration { return null; } - watchProcess(p, new IConsoleParser[] { epm }); + watchProcess(new IConsoleParser[] { epm }, monitor); } project.refreshLocal(IResource.DEPTH_INFINITE, monitor); @@ -298,7 +298,7 @@ public class MesonBuildConfiguration extends CBuildConfiguration { return; } - watchProcess(p, console); + watchProcess(console, monitor); } outStream.write(String.format(Messages.MesonBuildConfiguration_BuildingComplete, buildDir.toString())); diff --git a/cmake/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/internal/CMakeBuildConfiguration.java b/cmake/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/internal/CMakeBuildConfiguration.java index 4b39dc5a911..0827caca307 100644 --- a/cmake/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/internal/CMakeBuildConfiguration.java +++ b/cmake/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/internal/CMakeBuildConfiguration.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2015, 2016 QNX Software Systems and others. + * Copyright (c) 2015, 2022 QNX Software Systems and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -175,9 +175,6 @@ public class CMakeBuildConfiguration extends CBuildConfiguration { ParsingConsoleOutputStream errStream = new ParsingConsoleOutputStream(console.getErrorStream(), errorParser); IConsole errConsole = new CMakeConsoleWrapper(console, errStream); - // TODO startBuildProcess() calls java.lang.ProcessBuilder. - // Use org.eclipse.cdt.core.ICommandLauncher - // in order to run builds in a container. Process p = startBuildProcess(command.getArguments(), new IEnvironmentVariable[0], workingDir, errConsole, monitor); String arg0 = command.getArguments().get(0); @@ -190,7 +187,7 @@ public class CMakeBuildConfiguration extends CBuildConfiguration { } // check cmake exit status - final int exitValue = watchProcess(p, errConsole); + final int exitValue = watchProcess(errConsole, monitor); if (exitValue != 0) { // cmake had errors... String msg = String.format(Messages.CMakeBuildConfiguration_ExitFailure, arg0, exitValue); @@ -230,8 +227,6 @@ public class CMakeBuildConfiguration extends CBuildConfiguration { org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path( getBuildDirectory().toString()); - // TODO startBuildProcess() calls java.lang.ProcessBuilder. Use org.eclipse.cdt.core.ICommandLauncher - // in order to run builds in a container. // TODO pass envvars from CommandDescriptor once we use ICommandLauncher Process p = startBuildProcess(command, envVars.toArray(new IEnvironmentVariable[0]), workingDir, console, monitor); @@ -241,7 +236,7 @@ public class CMakeBuildConfiguration extends CBuildConfiguration { } // check exit status - final int exitValue = watchProcess(p, new IConsoleParser[] { epm }); + final int exitValue = watchProcess(new IConsoleParser[] { epm }, monitor); if (exitValue != 0) { // had errors... String msg2 = String.format(Messages.CMakeBuildConfiguration_ExitFailure, command.get(0), @@ -286,8 +281,6 @@ public class CMakeBuildConfiguration extends CBuildConfiguration { org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path( getBuildDirectory().toString()); - // TODO startBuildProcess() calls java.lang.ProcessBuilder. Use org.eclipse.cdt.core.ICommandLauncher - // in order to run builds in a container. Process p = startBuildProcess(command.getArguments(), new IEnvironmentVariable[0], workingDir, console, monitor); if (p == null) { @@ -299,7 +292,7 @@ public class CMakeBuildConfiguration extends CBuildConfiguration { } // check exit status - final int exitValue = watchProcess(p, console); + final int exitValue = watchProcess(console, monitor); if (exitValue != 0) { // had errors... String msg = String.format(Messages.CMakeBuildConfiguration_ExitFailure, command.getArguments().get(0), diff --git a/core/org.eclipse.cdt.core/META-INF/MANIFEST.MF b/core/org.eclipse.cdt.core/META-INF/MANIFEST.MF index 6224785337c..bc52049ddfe 100644 --- a/core/org.eclipse.cdt.core/META-INF/MANIFEST.MF +++ b/core/org.eclipse.cdt.core/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %pluginName Bundle-SymbolicName: org.eclipse.cdt.core; singleton:=true -Bundle-Version: 7.4.200.qualifier +Bundle-Version: 7.5.0.qualifier Bundle-Activator: org.eclipse.cdt.core.CCorePlugin Bundle-Vendor: %providerName Bundle-Localization: plugin diff --git a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/CBuildConfiguration.java b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/CBuildConfiguration.java index 68e5c472eea..039e1a4518b 100644 --- a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/CBuildConfiguration.java +++ b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/CBuildConfiguration.java @@ -10,15 +10,10 @@ *******************************************************************************/ package org.eclipse.cdt.core.build; -import java.io.BufferedReader; import java.io.File; import java.io.FileReader; import java.io.FileWriter; import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.OutputStream; -import java.io.PrintStream; import java.net.URI; import java.nio.file.Files; import java.nio.file.InvalidPathException; @@ -33,9 +28,11 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Properties; import java.util.Set; +import java.util.stream.Collectors; import org.eclipse.cdt.core.CCorePlugin; import org.eclipse.cdt.core.CommandLauncherManager; +import org.eclipse.cdt.core.ICommandLauncher; import org.eclipse.cdt.core.IConsoleParser; import org.eclipse.cdt.core.IConsoleParser2; import org.eclipse.cdt.core.IMarkerGenerator; @@ -60,6 +57,8 @@ import org.eclipse.cdt.core.parser.IExtendedScannerInfo; import org.eclipse.cdt.core.parser.IScannerInfo; import org.eclipse.cdt.core.parser.IScannerInfoChangeListener; import org.eclipse.cdt.core.resources.IConsole; +import org.eclipse.cdt.internal.core.BuildRunnerHelper; +import org.eclipse.cdt.internal.core.ConsoleOutputSniffer; import org.eclipse.cdt.internal.core.build.Messages; import org.eclipse.cdt.internal.core.model.BinaryRunner; import org.eclipse.cdt.internal.core.model.CModelManager; @@ -116,6 +115,8 @@ public abstract class CBuildConfiguration extends PlatformObject implements ICBu private final Map> scannerInfoListeners = new HashMap<>(); private ScannerInfoCache scannerInfoCache; + private ICommandLauncher launcher; + protected CBuildConfiguration(IBuildConfiguration config, String name) throws CoreException { this.config = config; this.name = name; @@ -488,7 +489,8 @@ public abstract class CBuildConfiguration extends PlatformObject implements ICBu .write(String.format(Messages.CBuildConfiguration_CommandNotFound, commands.get(0))); return null; } - commands.set(0, commandPath.toString()); + IPath cmd = new org.eclipse.core.runtime.Path(commandPath.toString()); + List args = commands.subList(1, commands.size()); // check if includes have been removed/refreshed and scanner info refresh is needed boolean needRefresh = CommandLauncherManager.getInstance().checkIfIncludesChanged(this); @@ -497,18 +499,28 @@ public abstract class CBuildConfiguration extends PlatformObject implements ICBu t.setProperty(NEED_REFRESH, Boolean.valueOf(needRefresh).toString()); } - ProcessBuilder processBuilder = new ProcessBuilder(commands).directory(buildDirectory.toFile()); - // Override environment variables - Map environment = processBuilder.environment(); + // Generate environment block before launching process + launcher = CommandLauncherManager.getInstance().getCommandLauncher(this); + Properties envProps = launcher.getEnvironment(); + HashMap environment = envProps.entrySet().stream() + .collect(Collectors.toMap(e -> String.valueOf(e.getKey()), e -> String.valueOf(e.getValue()), + (prev, next) -> next, HashMap::new)); for (IEnvironmentVariable envVar : envVars) { environment.put(envVar.getName(), envVar.getValue()); } setBuildEnvironment(environment); - process = processBuilder.start(); + launcher.setProject(getProject()); + process = launcher.execute(cmd, args.toArray(new String[0]), BuildRunnerHelper.envMapToEnvp(environment), + buildDirectory, monitor); } return process; } + /** + * @return The exit code of the build process. + * + * @deprecated use {@link #watchProcess(IConsole, IProgressMonitor)} or {@link #watchProcess(IConsoleParser[], IProgressMonitor)} instead + */ @Deprecated protected int watchProcess(Process process, IConsoleParser[] consoleParsers, IConsole console) throws CoreException { @@ -520,110 +532,42 @@ public abstract class CBuildConfiguration extends PlatformObject implements ICBu } /** + * @return The exit code of the build process. * @since 6.4 + * + * @deprecated use {@link #watchProcess(IConsole, IProgressMonitor)} instead and pass in a monitor */ + @Deprecated protected int watchProcess(Process process, IConsole console) throws CoreException { - Thread t1 = new ReaderThread(process.getInputStream(), console.getOutputStream()); - t1.start(); - Thread t2 = new ReaderThread(process.getErrorStream(), console.getErrorStream()); - t2.start(); - try { - int rc = process.waitFor(); - // Allow reader threads the chance to process all output to console - while (t1.isAlive()) { - Thread.sleep(100); - } - while (t2.isAlive()) { - Thread.sleep(100); - } - return rc; - } catch (InterruptedException e) { - CCorePlugin.log(e); - return -1; - } + return watchProcess(console, new NullProgressMonitor()); } /** - * @since 6.4 + * @return The exit code of the build process. + * @since 7.5 */ - protected int watchProcess(Process process, IConsoleParser[] consoleParsers) throws CoreException { - Thread t1 = new ReaderThread(this, process.getInputStream(), consoleParsers); - t1.start(); - Thread t2 = new ReaderThread(this, process.getErrorStream(), consoleParsers); - t2.start(); - try { - int rc = process.waitFor(); - // Allow reader threads the chance to process all output to console - while (t1.isAlive()) { - Thread.sleep(100); - } - while (t2.isAlive()) { - Thread.sleep(100); - } - return rc; - } catch (InterruptedException e) { - CCorePlugin.log(e); - return -1; - } + protected int watchProcess(IConsole console, IProgressMonitor monitor) throws CoreException { + return launcher.waitAndRead(console.getInfoStream(), console.getErrorStream(), monitor); } - private static class ReaderThread extends Thread { - CBuildConfiguration config; - private final BufferedReader in; - private final IConsoleParser[] consoleParsers; - private final PrintStream out; + /** + * @return The exit code of the build process. + * @since 6.4 + * + * @deprecated use {@link #watchProcess(IConsoleParser[], IProgressMonitor)} instead and pass in a monitor + */ + @Deprecated + protected int watchProcess(Process process, IConsoleParser[] consoleParsers) throws CoreException { + return watchProcess(consoleParsers, new NullProgressMonitor()); + } - public ReaderThread(CBuildConfiguration config, InputStream in, IConsoleParser[] consoleParsers) { - this.config = config; - this.in = new BufferedReader(new InputStreamReader(in)); - this.out = null; - this.consoleParsers = consoleParsers; - } - - public ReaderThread(InputStream in, OutputStream out) { - this.in = new BufferedReader(new InputStreamReader(in)); - this.out = new PrintStream(out); - this.consoleParsers = null; - this.config = null; - } - - @Override - public void run() { - List jobList = new ArrayList<>(); - try { - for (String line = in.readLine(); line != null; line = in.readLine()) { - if (consoleParsers != null) { - for (IConsoleParser consoleParser : consoleParsers) { - // Synchronize to avoid interleaving of lines - synchronized (consoleParser) { - // if we have an IConsoleParser2, use the processLine method that - // takes a job list (Container Build support) - if (consoleParser instanceof IConsoleParser2) { - ((IConsoleParser2) consoleParser).processLine(line, jobList); - } else { - consoleParser.processLine(line); - } - } - } - } - if (out != null) { - out.println(line); - } - } - for (Job j : jobList) { - try { - j.join(); - } catch (InterruptedException e) { - // ignore - } - } - if (config != null) { - config.shutdown(); - } - } catch (IOException e) { - CCorePlugin.log(e); - } - } + /** + * @return The exit code of the build process. + * @since 7.5 + */ + protected int watchProcess(IConsoleParser[] consoleParsers, IProgressMonitor monitor) throws CoreException { + ConsoleOutputSniffer sniffer = new ConsoleOutputSniffer(consoleParsers); + return launcher.waitAndRead(sniffer.getOutputStream(), sniffer.getErrorStream(), monitor); } private File getScannerInfoCacheFile() { diff --git a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/StandardBuildConfiguration.java b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/StandardBuildConfiguration.java index d10351cb41a..9f0d72803c0 100644 --- a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/StandardBuildConfiguration.java +++ b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/StandardBuildConfiguration.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2016 QNX Software Systems and others. + * Copyright (c) 2016, 2022 QNX Software Systems and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -267,7 +267,7 @@ public class StandardBuildConfiguration extends CBuildConfiguration { } IConsoleParser[] consoleParsers = new IConsoleParser[] { epm, this }; - watchProcess(p, consoleParsers); + watchProcess(consoleParsers, monitor); project.refreshLocal(IResource.DEPTH_INFINITE, monitor); @@ -326,7 +326,7 @@ public class StandardBuildConfiguration extends CBuildConfiguration { return; } - watchProcess(p, console); + watchProcess(console, monitor); outStream.write(Messages.CBuildConfiguration_BuildComplete); diff --git a/qt/org.eclipse.cdt.qt.core/src/org/eclipse/cdt/internal/qt/core/build/QtBuildConfiguration.java b/qt/org.eclipse.cdt.qt.core/src/org/eclipse/cdt/internal/qt/core/build/QtBuildConfiguration.java index 8e05b7aef14..189ed99053d 100644 --- a/qt/org.eclipse.cdt.qt.core/src/org/eclipse/cdt/internal/qt/core/build/QtBuildConfiguration.java +++ b/qt/org.eclipse.cdt.qt.core/src/org/eclipse/cdt/internal/qt/core/build/QtBuildConfiguration.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2015, 2016 QNX Software Systems and others. + * Copyright (c) 2015, 2022 QNX Software Systems and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -379,9 +379,8 @@ public class QtBuildConfiguration extends CBuildConfiguration implements IQtBuil IFile projectFile = project.getFile(project.getName() + ".pro"); //$NON-NLS-1$ command.add(projectFile.getLocation().toOSString()); - ProcessBuilder processBuilder = new ProcessBuilder(command).directory(getBuildDirectory().toFile()); - setBuildEnvironment(processBuilder.environment()); - Process process = processBuilder.start(); + startBuildProcess(command, new IEnvironmentVariable[0], + new org.eclipse.core.runtime.Path(buildDir.toString()), console, monitor); StringBuffer msg = new StringBuffer(); for (String arg : command) { @@ -391,7 +390,7 @@ public class QtBuildConfiguration extends CBuildConfiguration implements IQtBuil outStream.write(msg.toString()); // TODO qmake error parser - watchProcess(process, console); + watchProcess(console, monitor); doFullBuild = false; } @@ -401,11 +400,11 @@ public class QtBuildConfiguration extends CBuildConfiguration implements IQtBuil // run make List command = new ArrayList<>(Arrays.asList(makeCommand)); command.add("all"); //$NON-NLS-1$ - ProcessBuilder processBuilder = new ProcessBuilder(command).directory(buildDir.toFile()); - setBuildEnvironment(processBuilder.environment()); - Process process = processBuilder.start(); + + startBuildProcess(command, new IEnvironmentVariable[0], + new org.eclipse.core.runtime.Path(buildDir.toString()), console, monitor); outStream.write(String.join(" ", command) + '\n'); //$NON-NLS-1$ - watchProcess(process, new IConsoleParser[] { epm }); + watchProcess(new IConsoleParser[] { epm }, monitor); } getProject().refreshLocal(IResource.DEPTH_INFINITE, monitor); @@ -438,11 +437,10 @@ public class QtBuildConfiguration extends CBuildConfiguration implements IQtBuil // run make List command = new ArrayList<>(Arrays.asList(makeCommand)); command.add("clean"); //$NON-NLS-1$ - ProcessBuilder processBuilder = new ProcessBuilder(command).directory(buildDir.toFile()); - setBuildEnvironment(processBuilder.environment()); - Process process = processBuilder.start(); + startBuildProcess(command, new IEnvironmentVariable[0], + new org.eclipse.core.runtime.Path(buildDir.toString()), console, monitor); outStream.write(String.join(" ", command) + '\n'); //$NON-NLS-1$ - watchProcess(process, new IConsoleParser[] { epm }); + watchProcess(new IConsoleParser[] { epm }, monitor); } project.refreshLocal(IResource.DEPTH_INFINITE, monitor);