From c3c56079b84ac2aa34d9e57e844a94d65987e7e8 Mon Sep 17 00:00:00 2001 From: Jonah Graham Date: Tue, 28 Jan 2025 13:23:53 -0500 Subject: [PATCH] Add some documentation and consistency to use of info vs output stream With some better Javadocs the cross referencing between IConsole and the UI aspects of the console is a little easier to follow. This resolves #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 #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 #1059) Fixes #1059 --- .../core/AutotoolsBuildConfiguration.java | 6 ++--- .../meson/core/MesonBuildConfiguration.java | 16 ++++++------- .../cmake/core/CMakeBuildConfiguration.java | 8 +++---- .../eclipse/cdt/core/ICommandLauncher.java | 6 +++++ .../cdt/core/build/CBuildConfiguration.java | 2 +- .../build/StandardBuildConfiguration.java | 16 ++++++------- .../eclipse/cdt/core/resources/IConsole.java | 23 +++++++++++++++++++ .../BuildConsolePreferencePage.java | 10 ++++++++ 8 files changed, 62 insertions(+), 25 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 055f8972115..bdd6d920db3 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 @@ -90,7 +90,7 @@ public class AutotoolsBuildConfiguration extends CBuildConfiguration { 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())) { @@ -98,8 +98,8 @@ public class AutotoolsBuildConfiguration extends CBuildConfiguration { 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$ 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 0be025a4838..d133cbd0456 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 @@ -119,11 +119,11 @@ public class MesonBuildConfiguration extends CBuildConfiguration { 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()) { @@ -169,7 +169,7 @@ public class MesonBuildConfiguration extends CBuildConfiguration { 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$ @@ -246,7 +246,7 @@ public class MesonBuildConfiguration extends CBuildConfiguration { // 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) { @@ -261,11 +261,11 @@ public class MesonBuildConfiguration extends CBuildConfiguration { 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); @@ -290,7 +290,7 @@ public class MesonBuildConfiguration extends CBuildConfiguration { 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() @@ -301,7 +301,7 @@ public class MesonBuildConfiguration extends CBuildConfiguration { 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) { diff --git a/cmake/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/CMakeBuildConfiguration.java b/cmake/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/CMakeBuildConfiguration.java index 1760baff6f1..f25f5ad293b 100644 --- a/cmake/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/CMakeBuildConfiguration.java +++ b/cmake/org.eclipse.cdt.cmake.core/src/org/eclipse/cdt/cmake/core/CMakeBuildConfiguration.java @@ -309,16 +309,16 @@ public class CMakeBuildConfiguration extends CBuildConfiguration implements ICMa 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()); @@ -341,7 +341,7 @@ public class CMakeBuildConfiguration extends CBuildConfiguration implements ICMa 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) { diff --git a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/ICommandLauncher.java b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/ICommandLauncher.java index 9b67ceeb85f..13eee806776 100644 --- a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/ICommandLauncher.java +++ b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/ICommandLauncher.java @@ -16,6 +16,7 @@ package org.eclipse.cdt.core; 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; @@ -113,6 +114,11 @@ public interface ICommandLauncher { * 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); } \ No newline at end of file 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 837b2042e3a..448783dc020 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 @@ -596,7 +596,7 @@ public abstract class CBuildConfiguration extends PlatformObject implements ICBu */ 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); } /** 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 da5726bfef1..179cbdda0a4 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 @@ -31,7 +31,6 @@ import org.eclipse.cdt.core.resources.IConsole; 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; @@ -39,7 +38,6 @@ 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; /** @@ -238,11 +236,11 @@ public class StandardBuildConfiguration extends CBuildConfiguration { 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 command = new ArrayList<>(); command.add(buildCommand[0]); @@ -278,7 +276,7 @@ public class StandardBuildConfiguration extends CBuildConfiguration { 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 }; @@ -294,11 +292,11 @@ public class StandardBuildConfiguration extends CBuildConfiguration { 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 command = new ArrayList<>(); List buildCommand; @@ -323,7 +321,7 @@ public class StandardBuildConfiguration extends CBuildConfiguration { } // 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()); @@ -335,7 +333,7 @@ public class StandardBuildConfiguration extends CBuildConfiguration { watchProcess(console, monitor); - outStream.write(Messages.CBuildConfiguration_BuildComplete); + infoStream.write(Messages.CBuildConfiguration_BuildComplete); project.refreshLocal(IResource.DEPTH_INFINITE, monitor); } catch (IOException e) { diff --git a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/resources/IConsole.java b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/resources/IConsole.java index 2f63e789180..66028c6cab2 100644 --- a/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/resources/IConsole.java +++ b/core/org.eclipse.cdt.core/src/org/eclipse/cdt/core/resources/IConsole.java @@ -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; } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/preferences/BuildConsolePreferencePage.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/preferences/BuildConsolePreferencePage.java index 4b6caa74dc0..d378b5cb731 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/preferences/BuildConsolePreferencePage.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/preferences/BuildConsolePreferencePage.java @@ -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; @@ -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$