Skip to content

Commit

Permalink
Add some documentation and consistency to use of info vs output stream (
Browse files Browse the repository at this point in the history 
eclipse-cdt#1060)

With some better Javadocs the cross referencing between IConsole
and the UI aspects of the console is a little easier to follow.

This resolves eclipse-cdt#1059 in two parts:

1. For Core Build System this update consistently uses info stream to
show information messages and output stream to be stdout of launched
build tool. This resolves the "Build Complete" appearing as the output
color when doing clean (See screenshots in eclipse-cdt#1059)

2. CBuildConfiguration.watchProcess(IConsole, IProgressMonitor) incorrectly
passed the info stream as the output stream. Mostly this was used for
the clean stage of builds. This resolves the CMake output like ("Cleaning
all built files...") appearing as the info color when doing clean (See
screenshots in eclipse-cdt#1059)

Fixes eclipse-cdt#1059
  • Loading branch information
@jonahgraham
jonahgraham authored Jan 28, 2025
1 parent 0564831 commit 84d99f2
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 25 deletions.
6 changes: 3 additions & 3 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 @@ -90,16 +90,16 @@ protected void executeRemote(List<String> command, IPath processCwd, IConsole co
try {
project.deleteMarkers(ICModelMarker.C_MODEL_PROBLEM_MARKER, false, IResource.DEPTH_INFINITE);

ConsoleOutputStream outStream = console.getOutputStream();
ConsoleOutputStream infoStream = console.getInfoStream();

try (ErrorParserManager epm = new ErrorParserManager(project, getBuildDirectoryURI(), this,
getToolChain().getErrorParserIds())) {
epm.setOutputStream(console.getOutputStream());

IEnvironmentVariable[] env = new IEnvironmentVariable[0];

outStream.write("Building in: " + processCwd.toString() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
outStream.write("Running: " + commandJoined + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
infoStream.write("Building in: " + processCwd.toString() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
infoStream.write("Running: " + commandJoined + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
Process p = startBuildProcess(command, env, processCwd, console, monitor);
if (p == null) {
console.getErrorStream().write("Error executing: " + commandJoined); //$NON-NLS-1$
Expand Down
16 changes: 8 additions & 8 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
Expand Up @@ -119,11 +119,11 @@ public IProject[] build(int kind, Map<String, String> args, String[] ninjaEnv, S
try {
project.deleteMarkers(ICModelMarker.C_MODEL_PROBLEM_MARKER, false, IResource.DEPTH_INFINITE);

ConsoleOutputStream outStream = console.getOutputStream();
ConsoleOutputStream infoStream = console.getInfoStream();

Path buildDir = getBuildDirectory();

outStream.write(String.format(Messages.MesonBuildConfiguration_BuildingIn, buildDir.toString()));
infoStream.write(String.format(Messages.MesonBuildConfiguration_BuildingIn, buildDir.toString()));

// Make sure we have a toolchain file if cross
if (toolChainFile == null && !isLocal()) {
Expand Down Expand Up @@ -169,7 +169,7 @@ public IProject[] build(int kind, Map<String, String> args, String[] ninjaEnv, S

monitor.subTask(Messages.MesonBuildConfiguration_RunningMeson);

outStream.write(String.join(" ", envStr != null ? ("env " + envStr) : "", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
infoStream.write(String.join(" ", envStr != null ? ("env " + envStr) : "", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
"sh -c \"meson", userArgs != null ? userArgs : "", projOptions != null ? projOptions : "", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
getBuildDirectory().getParent().getParent().toString() + "\"\n")); //$NON-NLS-1$

Expand Down Expand Up @@ -246,7 +246,7 @@ public IProject[] build(int kind, Map<String, String> args, String[] ninjaEnv, S
// Process compile_commands.json file and generate Scanner info
refreshScannerInfo();

outStream.write(String.format(Messages.MesonBuildConfiguration_BuildingComplete, buildDir.toString()));
infoStream.write(String.format(Messages.MesonBuildConfiguration_BuildingComplete, buildDir.toString()));

return new IProject[] { project };
} catch (IOException e) {
Expand All @@ -261,11 +261,11 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti
try {
project.deleteMarkers(ICModelMarker.C_MODEL_PROBLEM_MARKER, false, IResource.DEPTH_INFINITE);

ConsoleOutputStream outStream = console.getOutputStream();
ConsoleOutputStream infoStream = console.getInfoStream();

Path buildDir = getBuildDirectory();

outStream.write(String.format(Messages.MesonBuildConfiguration_BuildingIn, buildDir.toString()));
infoStream.write(String.format(Messages.MesonBuildConfiguration_BuildingIn, buildDir.toString()));

if (!Files.exists(buildDir.resolve("build.ninja"))) { //$NON-NLS-1$
console.getOutputStream().write(Messages.MesonBuildConfiguration_NoNinjaFileToClean);
Expand All @@ -290,7 +290,7 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti

IEnvironmentVariable[] env = new IEnvironmentVariable[0];

outStream.write(String.join(" ", commandList) + '\n'); //$NON-NLS-1$
infoStream.write(String.join(" ", commandList) + '\n'); //$NON-NLS-1$
Process p = startBuildProcess(commandList, env, workingDir, console, monitor);
if (p == null) {
console.getErrorStream()
Expand All @@ -301,7 +301,7 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti
watchProcess(console, monitor);
}

outStream.write(String.format(Messages.MesonBuildConfiguration_BuildingComplete, buildDir.toString()));
infoStream.write(String.format(Messages.MesonBuildConfiguration_BuildingComplete, buildDir.toString()));

project.refreshLocal(IResource.DEPTH_INFINITE, monitor);
} catch (IOException e) {
Expand Down
8 changes: 4 additions & 4 deletions cmake/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/CMakeBuildConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,16 +309,16 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti
ICMakeProperties cmakeProperties = getCMakeProperties();
CommandDescriptorBuilder cmdBuilder = new CommandDescriptorBuilder(cmakeProperties);
CommandDescriptor command = cmdBuilder.makeCMakeBuildCommandline(cmakeProperties.getCleanTarget());
ConsoleOutputStream outStream = console.getOutputStream();
ConsoleOutputStream infoStream = console.getInfoStream();

Path buildDir = getBuildDirectory();

if (!Files.exists(buildDir.resolve("CMakeFiles"))) { //$NON-NLS-1$
outStream.write(Messages.CMakeBuildConfiguration_NotFound);
infoStream.write(Messages.CMakeBuildConfiguration_NotFound);
return;
}

outStream.write(String.join(" ", command.getArguments()) + '\n'); //$NON-NLS-1$
infoStream.write(String.join(" ", command.getArguments()) + '\n'); //$NON-NLS-1$

org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path(
getBuildDirectory().toString());
Expand All @@ -341,7 +341,7 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti
addMarker(project, -1, msg, IMarkerGenerator.SEVERITY_ERROR_BUILD, null);
}

outStream.write(Messages.CMakeBuildConfiguration_BuildComplete);
infoStream.write(Messages.CMakeBuildConfiguration_BuildComplete);

project.refreshLocal(IResource.DEPTH_INFINITE, monitor);
} catch (IOException e) {
Expand Down
6 changes: 6 additions & 0 deletions core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/ICommandLauncher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.io.OutputStream;
import java.util.Properties;

import org.eclipse.cdt.core.resources.IConsole;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
Expand Down Expand Up @@ -113,6 +114,11 @@ public Process execute(IPath commandPath, String[] args, String[] env, IPath wor
* polled to test if the cancel button has been pressed. Destroys the
* process if the monitor becomes canceled override to implement a different
* way to read the process inputs
*
* @param output the output stream that the command's stdout should be directed to.
* Typically connected to {@link IConsole#getOutputStream()}
* @param err the output stream that the command's stderr should be directed to.
* Typically connected to {@link IConsole#getErrorStream()}
*/
public int waitAndRead(OutputStream output, OutputStream err, IProgressMonitor monitor);
}
2 changes: 1 addition & 1 deletion 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 @@ -596,7 +596,7 @@ public Process startBuildProcess(List<String> commands, IEnvironmentVariable[] e
*/
protected int watchProcess(IConsole console, IProgressMonitor monitor) throws CoreException {
assertLauncherNotNull(launcher);
return launcher.waitAndRead(console.getInfoStream(), console.getErrorStream(), monitor);
return launcher.waitAndRead(console.getOutputStream(), console.getErrorStream(), monitor);
}

/**
Expand Down
16 changes: 7 additions & 9 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
Expand Up @@ -31,15 +31,13 @@
import org.eclipse.cdt.internal.core.build.Messages;
import org.eclipse.core.resources.IBuildConfiguration;
import org.eclipse.core.resources.IContainer;
import org.eclipse.core.resources.IFolder;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IResource;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.core.runtime.Status;

/**
Expand Down Expand Up @@ -238,11 +236,11 @@ public IProject[] build(int kind, Map<String, String> args, IConsole console, IP
try {
project.deleteMarkers(ICModelMarker.C_MODEL_PROBLEM_MARKER, false, IResource.DEPTH_INFINITE);

ConsoleOutputStream outStream = console.getOutputStream();
ConsoleOutputStream infoStream = console.getInfoStream();

Path buildDir = getBuildDirectory();

outStream.write(String.format(Messages.StandardBuildConfiguration_0, buildDir.toString()));
infoStream.write(String.format(Messages.StandardBuildConfiguration_0, buildDir.toString()));

List<String> command = new ArrayList<>();
command.add(buildCommand[0]);
Expand Down Expand Up @@ -278,7 +276,7 @@ public IProject[] build(int kind, Map<String, String> args, IConsole console, IP

project.refreshLocal(IResource.DEPTH_INFINITE, monitor);

outStream.write(String.format(Messages.StandardBuildConfiguration_1, epm.getErrorCount(),
infoStream.write(String.format(Messages.StandardBuildConfiguration_1, epm.getErrorCount(),
epm.getWarningCount(), buildDir.toString()));
}
return new IProject[] { project };
Expand All @@ -294,11 +292,11 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti
try {
project.deleteMarkers(ICModelMarker.C_MODEL_PROBLEM_MARKER, false, IResource.DEPTH_INFINITE);

ConsoleOutputStream outStream = console.getOutputStream();
ConsoleOutputStream infoStream = console.getInfoStream();

Path buildDir = getBuildDirectory();

outStream.write(String.format(Messages.StandardBuildConfiguration_0, buildDir.toString()));
infoStream.write(String.format(Messages.StandardBuildConfiguration_0, buildDir.toString()));

List<String> command = new ArrayList<>();
List<String> buildCommand;
Expand All @@ -323,7 +321,7 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti
}

// run make
outStream.write(String.format("%s\n", String.join(" ", command))); //$NON-NLS-1$ //$NON-NLS-2$
infoStream.write(String.format("%s\n", String.join(" ", command))); //$NON-NLS-1$ //$NON-NLS-2$

org.eclipse.core.runtime.Path workingDir = new org.eclipse.core.runtime.Path(
getBuildDirectory().toString());
Expand All @@ -335,7 +333,7 @@ public void clean(IConsole console, IProgressMonitor monitor) throws CoreExcepti

watchProcess(console, monitor);

outStream.write(Messages.CBuildConfiguration_BuildComplete);
infoStream.write(Messages.CBuildConfiguration_BuildComplete);

project.refreshLocal(IResource.DEPTH_INFINITE, monitor);
} catch (IOException e) {
Expand Down
23 changes: 23 additions & 0 deletions core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/resources/IConsole.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,32 @@ public interface IConsole {
*/
void start(IProject project);

/**
* Get the stream that shows up as output in the console. This
* is typically connected to the output of the build process.
*/
ConsoleOutputStream getOutputStream() throws CoreException;

/**
* Get the stream that shows up as information messages in
* the console. This is typically not connected to the output
* of the build process. Typically information messages, such
* as build started and build completed messages are written
* to the info stream.
*
* @apiNote Whether the command line used to launch the process
* is written to the info stream or to the output stream is
* very inconsistent in CDT's code base. Core Build mostly
* uses the info stream for this purpose, but MBS typically
* uses output stream.
*/
ConsoleOutputStream getInfoStream() throws CoreException;

/**
* Get the stream that shows up as output in the console. This
* is typically connected to the error output of the build process
* and errors detected when launching the process can be output
* to here as well.
*/
ConsoleOutputStream getErrorStream() throws CoreException;
}
10 changes: 10 additions & 0 deletions ...clipse.cdt.ui/src/org/eclipse/cdt/internal/ui/preferences/BuildConsolePreferencePage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*******************************************************************************/
package org.eclipse.cdt.internal.ui.preferences;

import org.eclipse.cdt.core.resources.IConsole;
import org.eclipse.cdt.internal.ui.ICHelpContextIds;
import org.eclipse.cdt.ui.CUIPlugin;
import org.eclipse.jface.preference.BooleanFieldEditor;
Expand Down Expand Up @@ -49,8 +50,17 @@ public class BuildConsolePreferencePage extends FieldEditorPreferencePage implem
public static final String PREF_BUILDCONSOLE_TAB_WIDTH = "buildConsoleTabWith"; //$NON-NLS-1$
public static final String PREF_BUILDCONSOLE_LINES = "buildConsoleLines"; //$NON-NLS-1$
public static final String PREF_BUILDCONSOLE_UPDATE_DELAY_MS = "buildConsoleUpdateDelayMs"; //$NON-NLS-1$
/**
* The color of the {@link IConsole#getInfoStream()}
*/
public static final String PREF_BUILDCONSOLE_INFO_COLOR = "buildConsoleInfoStreamColor"; //$NON-NLS-1$
/**
* The color of the {@link IConsole#getOutputStream()}
*/
public static final String PREF_BUILDCONSOLE_OUTPUT_COLOR = "buildConsoleOutputStreamColor"; //$NON-NLS-1$
/**
* The color of the {@link IConsole#getErrorStream()}
*/
public static final String PREF_BUILDCONSOLE_ERROR_COLOR = "buildConsoleErrorStreamColor"; //$NON-NLS-1$
public static final String PREF_BUILDCONSOLE_BACKGROUND_COLOR = "buildConsoleBackgroundColor"; //$NON-NLS-1$
public static final String PREF_BUILDCONSOLE_PROBLEM_BACKGROUND_COLOR = "buildConsoleProblemBackgroundColor"; //$NON-NLS-1$
Expand Down

0 comments on commit 84d99f2

Please sign in to comment.