Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IEP-1247 Run CMake initial build for newly created project(idf.py reconfigure) #1051

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
3 changes: 2 additions & 1 deletion bundles/com.espressif.idf.core/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,5 @@ Bundle-ClassPath: .,
lib/commons-compress-1.21.jar,
lib/xz-1.9.jar
Import-Package: org.eclipse.embedcdt.core,
org.eclipse.launchbar.ui.target
org.eclipse.launchbar.ui.target,
org.eclipse.ui.console
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,13 @@ public class IDFEnvironmentVariables

public static final String ESP_IDF_VERSION = "ESP_IDF_VERSION"; //$NON-NLS-1$

public static final String GIT_PATH ="GIT_PATH"; //$NON-NLS-1$
public static final String PYTHON_EXE_PATH ="PYTHON_EXE_PATH"; //$NON-NLS-1$
public static final String GIT_PATH = "GIT_PATH"; //$NON-NLS-1$

public static final String PYTHON_EXE_PATH = "PYTHON_EXE_PATH"; //$NON-NLS-1$

public static final String IDF_MAINTAINER = "IDF_MAINTAINER"; //$NON-NLS-1$

public static final String IDF_CCACHE_ENABLE = "IDF_CCACHE_ENABLE"; //$NON-NLS-1$

/**
* @param variableName Environment variable Name
Expand All @@ -68,11 +69,11 @@ public void removeEnvVariable(String variableName)
IContributedEnvironment contributedEnvironment = getEnvironment();
contributedEnvironment.removeVariable(variableName, null);
}

public void removeAllEnvVariables()
{
Map<String, String> currentVarMap = getEnvMap();
for(Entry<String, String> varEntry : currentVarMap.entrySet())
for (Entry<String, String> varEntry : currentVarMap.entrySet())
{
removeEnvVariable(varEntry.getKey());
}
Expand Down Expand Up @@ -100,7 +101,7 @@ public String getEnvValue(String variableName)

return envValue;
}

public void addEnvVariable(String name, String value)
{
Logger.log(MessageFormat.format("Updating environment variables with key:{0} value:{1}", name, value)); //$NON-NLS-1$
Expand Down Expand Up @@ -142,7 +143,7 @@ public Map<String, String> getSystemEnvMap()
envMap.put(key, value);
}
}

return envMap;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,13 @@ public Process run(List<String> commands, IPath workingDirectory, Map<String, St
if (environment != null && !environment.isEmpty())
{
processBuilder.environment().putAll(environment);
// Removing Path, because we are using PATH
if (processBuilder.environment().containsKey("PATH") && processBuilder.environment().containsKey("Path")) //$NON-NLS-1$
{
processBuilder.environment().remove("Path"); //$NON-NLS-1$
}
}
// let's merge the error stream with the standard output

processBuilder.redirectErrorStream(true);
return processBuilder.start();
}
Expand All @@ -46,8 +51,7 @@ public IStatus runInBackground(List<String> commands, IPath workingDirectory, Ma
throws IOException
{
Process process = run(commands, workingDirectory, environment);
return processData(process.getInputStream(), process.getErrorStream(), process.getOutputStream(),
process);
return processData(process.getInputStream(), process.getErrorStream(), process.getOutputStream(), process);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*******************************************************************************
* Copyright 2024 Espressif Systems (Shanghai) PTE LTD. All rights reserved.
* Use is subject to license terms.
*******************************************************************************/

package com.espressif.idf.core.util;

import org.eclipse.ui.console.ConsolePlugin;
import org.eclipse.ui.console.IConsole;
import org.eclipse.ui.console.MessageConsole;

public class ConsoleManager
{

private ConsoleManager()
{
}

public static MessageConsole getConsole(String consoleName)
{
MessageConsole console = findConsole(consoleName);
if (console == null)
{
console = new MessageConsole(consoleName, null);
ConsolePlugin.getDefault().getConsoleManager().addConsoles(new IConsole[] { console });
}
ConsolePlugin.getDefault().getConsoleManager().showConsoleView(console);
return console;
}
Comment on lines +19 to +29
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add documentation and parameter validation.

The method logic is correct, but could benefit from these improvements:

  1. Add Javadoc with parameter and return descriptions
  2. Add null check for consoleName parameter
+/**
+ * Gets or creates a MessageConsole with the specified name.
+ * If a console with the given name already exists, it will be returned.
+ * Otherwise, a new console will be created and added to the console manager.
+ *
+ * @param consoleName the name of the console to get or create
+ * @return the MessageConsole instance
+ * @throws IllegalArgumentException if consoleName is null or empty
+ */
 public static MessageConsole getConsole(String consoleName)
 {
+    if (consoleName == null || consoleName.trim().isEmpty()) {
+        throw new IllegalArgumentException("Console name cannot be null or empty");
+    }
     MessageConsole console = findConsole(consoleName);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public static MessageConsole getConsole(String consoleName)
{
MessageConsole console = findConsole(consoleName);
if (console == null)
{
console = new MessageConsole(consoleName, null);
ConsolePlugin.getDefault().getConsoleManager().addConsoles(new IConsole[] { console });
}
ConsolePlugin.getDefault().getConsoleManager().showConsoleView(console);
return console;
}
/**
* Gets or creates a MessageConsole with the specified name.
* If a console with the given name already exists, it will be returned.
* Otherwise, a new console will be created and added to the console manager.
*
* @param consoleName the name of the console to get or create
* @return the MessageConsole instance
* @throws IllegalArgumentException if consoleName is null or empty
*/
public static MessageConsole getConsole(String consoleName)
{
if (consoleName == null || consoleName.trim().isEmpty()) {
throw new IllegalArgumentException("Console name cannot be null or empty");
}
MessageConsole console = findConsole(consoleName);
if (console == null)
{
console = new MessageConsole(consoleName, null);
ConsolePlugin.getDefault().getConsoleManager().addConsoles(new IConsole[] { console });
}
ConsolePlugin.getDefault().getConsoleManager().showConsoleView(console);
return console;
}


private static MessageConsole findConsole(String consoleName)
{
for (IConsole existing : ConsolePlugin.getDefault().getConsoleManager().getConsoles())
{
if (consoleName.equals(existing.getName()))
{
return (MessageConsole) existing;
}
}
return null;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont understand the need for this class, maybe I am not clear on this. Just a question from my side that why do we need a separate class to just execute this command when we can do that in the Job we are creating for the console?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @alirana01,

thanks for the review. I've created this class not only for this command but also for other idf commands that we have to execute in the console. My idea was that in the future we could refactor existing code and move other IDF commands and their preparation into this class to not scatter similar logic over the code base and to avoid duplicate code

Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
/*******************************************************************************
* Copyright 2021 Espressif Systems (Shanghai) PTE LTD. All rights reserved.
* Use is subject to license terms.
*******************************************************************************/

package com.espressif.idf.core.util;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;

import org.eclipse.core.resources.IProject;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Status;
import org.eclipse.debug.core.DebugPlugin;
import org.eclipse.debug.core.ILaunchConfiguration;
import org.eclipse.debug.core.ILaunchManager;
import org.eclipse.ui.console.MessageConsole;
import org.eclipse.ui.console.MessageConsoleStream;

import com.espressif.idf.core.IDFCorePlugin;
import com.espressif.idf.core.IDFEnvironmentVariables;
import com.espressif.idf.core.ProcessBuilderFactory;
import com.espressif.idf.core.build.ActiveLaunchConfigurationProvider;
import com.espressif.idf.core.build.IDFLaunchConstants;
import com.espressif.idf.core.logging.Logger;

public class IdfCommandExecutor
{

private final String target;
private final MessageConsole console;
private static final String CMAKE_ARGUMENTS = "cmake.arguments"; //$NON-NLS-1$
private static final ActiveLaunchConfigurationProvider LAUNCH_CONFIG_PROVIDER = new ActiveLaunchConfigurationProvider();

public IdfCommandExecutor(String target, MessageConsole console)
{
this.target = target;
this.console = console;
}

public IStatus executeReconfigure(IProject project)
{
console.activate();
return runIdfReconfigureCommand(project);
}

private IStatus runIdfReconfigureCommand(IProject project)
{
ProcessBuilderFactory processRunner = new ProcessBuilderFactory();
setBuildFolder(project);
List<String> arguments = prepareCmakeArguments(project);
Map<String, String> environment = new HashMap<>(new IDFEnvironmentVariables().getSystemEnvMap());

try (MessageConsoleStream messageConsoleStream = console.newMessageStream())
{
messageConsoleStream.println(String.join(" ", arguments)); //$NON-NLS-1$
return runProcess(arguments, environment, processRunner, project, messageConsoleStream);
}
catch (IOException e1)
{
Logger.log(e1);
return IDFCorePlugin.errorStatus(e1.getMessage(), e1);
}
}

private List<String> prepareCmakeArguments(IProject project)
{
List<String> arguments = new ArrayList<>();
arguments.add("cmake"); //$NON-NLS-1$
arguments.add("-G"); //$NON-NLS-1$
arguments.add("Ninja"); //$NON-NLS-1$
arguments.add("-DPYTHON_DEPS_CHECKED=1"); //$NON-NLS-1$
arguments.add("-DPYTHON=" + IDFUtil.getIDFPythonEnvPath()); //$NON-NLS-1$
arguments.add("DESP_PLATFORM=1"); //$NON-NLS-1$

String ccache = new IDFEnvironmentVariables().getEnvValue(IDFEnvironmentVariables.IDF_CCACHE_ENABLE);
ccache = ccache.isBlank() ? "0" : ccache; //$NON-NLS-1$

arguments.add("DCCACHE_ENABLE=" + ccache); //$NON-NLS-1$
arguments.add(project.getLocation().toOSString());
arguments.add("-B"); //$NON-NLS-1$
try
{
arguments.add(IDFUtil.getBuildDir(project));
}
catch (CoreException e)
{
Logger.log(e);
}
return arguments;
}

private boolean setBuildFolder(IProject project)
{
String userArgs = getProperty(CMAKE_ARGUMENTS);
// Custom build directory
String[] cmakeArgumentsArr = userArgs.split(" "); //$NON-NLS-1$
String customBuildDir = StringUtil.EMPTY;
for (int i = 0; i < cmakeArgumentsArr.length; i++)
{
if (cmakeArgumentsArr[i].equals("-B")) //$NON-NLS-1$
{
customBuildDir = cmakeArgumentsArr[i + 1];
break;
}
}
try
{
IDFUtil.setBuildDir(project, customBuildDir);
}
catch (CoreException e)
{
Logger.log(e);
}

return !customBuildDir.isBlank();
}

public String getProperty(String name)
{
try
{
ILaunchConfiguration configuration = LAUNCH_CONFIG_PROVIDER.getActiveLaunchConfiguration();
if (configuration != null
&& configuration.getType().getIdentifier().equals(IDFLaunchConstants.DEBUG_LAUNCH_CONFIG_TYPE))
{
configuration = getBoundConfiguration(configuration);
}
return configuration == null ? StringUtil.EMPTY : configuration.getAttribute(name, StringUtil.EMPTY);
}
catch (CoreException e)
{
Logger.log(e);
}
return StringUtil.EMPTY;
}

private ILaunchConfiguration getBoundConfiguration(ILaunchConfiguration configuration) throws CoreException
{
String bindedLaunchConfigName = configuration.getAttribute(IDFLaunchConstants.ATTR_LAUNCH_CONFIGURATION_NAME,
StringUtil.EMPTY);
ILaunchManager launchManager = DebugPlugin.getDefault().getLaunchManager();
ILaunchConfiguration[] launchConfigurations = launchManager.getLaunchConfigurations(DebugPlugin.getDefault()
.getLaunchManager().getLaunchConfigurationType(IDFLaunchConstants.RUN_LAUNCH_CONFIG_TYPE));
ILaunchConfiguration defaultConfiguration = launchConfigurations[0];
return Stream.of(launchConfigurations).filter(config -> config.getName().contentEquals(bindedLaunchConfigName))
.findFirst().orElse(defaultConfiguration);

}

private IStatus runProcess(List<String> arguments, Map<String, String> environment,
ProcessBuilderFactory processRunner, IProject project, MessageConsoleStream messageConsoleStream)
{
StringBuilder output = new StringBuilder();
try (BufferedReader reader = new BufferedReader(new InputStreamReader(
processRunner.run(arguments, project.getLocation(), environment).getInputStream())))
{
String line;
while ((line = reader.readLine()) != null)
{
output.append(line).append(System.lineSeparator());
messageConsoleStream.println(line);
}
return new Status(IStatus.OK, IDFCorePlugin.PLUGIN_ID, output.toString());
}
catch (Exception e)
{
Logger.log(e);
return IDFCorePlugin.errorStatus(e.getMessage(), e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,10 @@ public ITerminalConnector createTerminalConnector(Map<String, Object> properties
}
envpList.add(envKey + "=" + envValue); //$NON-NLS-1$
}

//Removing path, since we are using PATH
if (envMap.containsKey("PATH") && envMap.containsKey("Path")) { //$NON-NLS-1$ //$NON-NLS-2$
envMap.remove("Path"); //$NON-NLS-1$
}
// Convert back into a string array
envp = envpList.toArray(new String[envpList.size()]);

Expand Down
10 changes: 10 additions & 0 deletions bundles/com.espressif.idf.ui/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,12 @@
</iterate>
</visibleWhen>
</command>
<command
commandId="com.espressif.idf.ui.reconfigure"
icon="icons/espressif.png"
label="Reconfigure"
style="push">
</command>
</menuContribution>
</extension>
<extension
Expand Down Expand Up @@ -659,6 +665,10 @@
class="com.espressif.idf.ui.handlers.WriteFlashCommandHandler"
commandId="com.espressif.idf.ui.writeFlashCommand">
</handler>
<handler
class="com.espressif.idf.ui.handlers.IdfReconfigureHandler"
commandId="com.espressif.idf.ui.reconfigure">
</handler>
</extension>
<extension
point="org.eclipse.ui.commands">
Expand Down
Loading
Loading