Skip to content

Commit

Permalink
Bug 580178 - Unable to stop build process from launchbar
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@mbooth101
mbooth101 committed Jun 30, 2022
1 parent 4a95606 commit 5e4a66b
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 137 deletions.
4 changes: 2 additions & 2 deletions ...e.autotools.core/src/org/eclipse/cdt/core/autotools/core/AutotoolsBuildConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ protected void execute(List<String> command, IPath dir, IConsole console, IProgr
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$
}
Expand Down Expand Up @@ -153,7 +153,7 @@ protected void executeRemote(List<String> command, IPath dir, IConsole console,
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);
Expand Down
8 changes: 4 additions & 4 deletions ...lipse.cdt.meson.core/src/org/eclipse/cdt/internal/meson/core/MesonBuildConfiguration.java
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -183,7 +183,7 @@ public IProject[] build(int kind, Map<String, String> args, String[] ninjaEnv, S
return null;
}

watchProcess(p, console);
watchProcess(console, monitor);
}

if (!Files.exists(buildDir.resolve("build.ninja"))) { //$NON-NLS-1$
Expand Down Expand Up @@ -238,7 +238,7 @@ public IProject[] build(int kind, Map<String, String> args, String[] ninjaEnv, S
return null;
}

watchProcess(p, new IConsoleParser[] { epm });
watchProcess(new IConsoleParser[] { epm }, monitor);
}

project.refreshLocal(IResource.DEPTH_INFINITE, monitor);
Expand Down Expand Up @@ -298,7 +298,7 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti
return;
}

watchProcess(p, console);
watchProcess(console, monitor);
}

outStream.write(String.format(Messages.MesonBuildConfiguration_BuildingComplete, buildDir.toString()));
Expand Down
15 changes: 4 additions & 11 deletions ...lipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/internal/CMakeBuildConfiguration.java
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -175,9 +175,6 @@ public IProject[] build(int kind, Map<String, String> args, IConsole console, IP
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);
Expand All @@ -190,7 +187,7 @@ public IProject[] build(int kind, Map<String, String> args, IConsole console, IP
}

// 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);
Expand Down Expand Up @@ -230,8 +227,6 @@ public IProject[] build(int kind, Map<String, String> args, IConsole console, IP

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);
Expand All @@ -241,7 +236,7 @@ public IProject[] build(int kind, Map<String, String> args, IConsole console, IP
}

// 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),
Expand Down Expand Up @@ -286,8 +281,6 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti

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) {
Expand All @@ -299,7 +292,7 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti
}

// 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),
Expand Down
2 changes: 1 addition & 1 deletion core/org.eclipse.cdt.core/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
150 changes: 47 additions & 103 deletions core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/CBuildConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,9 +28,11 @@
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;
Expand All @@ -60,6 +57,8 @@
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;
Expand Down Expand Up @@ -116,6 +115,8 @@ public abstract class CBuildConfiguration extends PlatformObject implements ICBu
private final Map<IResource, List<IScannerInfoChangeListener>> scannerInfoListeners = new HashMap<>();
private ScannerInfoCache scannerInfoCache;

private ICommandLauncher launcher;

protected CBuildConfiguration(IBuildConfiguration config, String name) throws CoreException {
this.config = config;
this.name = name;
Expand Down Expand Up @@ -488,7 +489,8 @@ public Process startBuildProcess(List<String> commands, IEnvironmentVariable[] e
.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<String> 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);
Expand All @@ -497,18 +499,28 @@ public Process startBuildProcess(List<String> commands, IEnvironmentVariable[] e
t.setProperty(NEED_REFRESH, Boolean.valueOf(needRefresh).toString());
}

ProcessBuilder processBuilder = new ProcessBuilder(commands).directory(buildDirectory.toFile());
// Override environment variables
Map<String, String> environment = processBuilder.environment();
// Generate environment block before launching process
launcher = CommandLauncherManager.getInstance().getCommandLauncher(this);
Properties envProps = launcher.getEnvironment();
HashMap<String, String> 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 {
Expand All @@ -520,110 +532,42 @@ protected int watchProcess(Process process, IConsoleParser[] consoleParsers, ICo
}

/**
* @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());
}

/**
* @return The exit code of the build process.
* @since 7.5
*/
protected int watchProcess(IConsole console, IProgressMonitor monitor) throws CoreException {
return launcher.waitAndRead(console.getInfoStream(), console.getErrorStream(), monitor);
}

/**
* @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 {
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;
}
return watchProcess(consoleParsers, new NullProgressMonitor());
}

private static class ReaderThread extends Thread {
CBuildConfiguration config;
private final BufferedReader in;
private final IConsoleParser[] consoleParsers;
private final PrintStream out;

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<Job> 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() {
Expand Down
6 changes: 3 additions & 3 deletions core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/build/StandardBuildConfiguration.java
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -267,7 +267,7 @@ public IProject[] build(int kind, Map<String, String> args, IConsole console, IP
}

IConsoleParser[] consoleParsers = new IConsoleParser[] { epm, this };
watchProcess(p, consoleParsers);
watchProcess(consoleParsers, monitor);

project.refreshLocal(IResource.DEPTH_INFINITE, monitor);

Expand Down Expand Up @@ -326,7 +326,7 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti
return;
}

watchProcess(p, console);
watchProcess(console, monitor);

outStream.write(Messages.CBuildConfiguration_BuildComplete);

Expand Down
Loading

0 comments on commit 5e4a66b

Please sign in to comment.