Skip to content

IEP-1472 Custom build folder: Whitespace in build folder path lead to error #1193

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Apr 22, 2025
Merged

IEP-1472 Custom build folder: Whitespace in build folder path lead to error #1193

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b6ba7fa
feat: add separate field for build folder
sigmaaa Mar 27, 2025
92079a4
fix: update build folder with right default value
sigmaaa Apr 7, 2025
9d2dfdb
fix: improved build folder update based on active launch config
sigmaaa Apr 7, 2025
f57fad6
fix: improved build folder selection for debugger config
sigmaaa Apr 7, 2025
0912488
feat: added error msg when -B is used
sigmaaa Apr 9, 2025
bff04f1
feat: updated doc and screenshot
sigmaaa Apr 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 4 additions & 36 deletions bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/IDFBuildConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@
* To work around that, we run cmake in advance with its dedicated working error parser.
*/
private ICMakeToolChainFile toolChainFile;
private String customBuildDir;
private IProgressMonitor monitor;
public boolean isProgressSet;

Expand All @@ -131,7 +130,7 @@
ICMakeToolChainFile toolChainFile, String launchMode)
{
super(config, name, toolChain, launchMode);
this.toolChainFile = toolChainFile;

Check warning on line 133 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/IDFBuildConfiguration.java

View workflow job for this annotation

GitHub Actions / spotbugs

EI_EXPOSE_REP2 

new com.espressif.idf.core.build.IDFBuildConfiguration(IBuildConfiguration, String, IToolChain, ICMakeToolChainFile, String) may expose internal representation by storing an externally mutable object into IDFBuildConfiguration.toolChainFile
Raw output 
This code stores a reference to an externally mutable object into the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.
}

@Override
Expand All @@ -158,43 +157,12 @@

public IPath getBuildContainerPath() throws CoreException
{
if (hasCustomBuild())
org.eclipse.core.runtime.Path path = new org.eclipse.core.runtime.Path(IDFUtil.getBuildDir(getProject()));
if (!path.toFile().exists())
{
org.eclipse.core.runtime.Path path = new org.eclipse.core.runtime.Path(customBuildDir);
if (!path.toFile().exists())
{
path.toFile().mkdirs();
}
return path;
path.toFile().mkdirs();

Check warning on line 163 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/IDFBuildConfiguration.java

View workflow job for this annotation

GitHub Actions / spotbugs

RV_RETURN_VALUE_IGNORED_BAD_PRACTICE 

Exceptional return value of java.io.File.mkdirs() ignored in com.espressif.idf.core.build.IDFBuildConfiguration.getBuildContainerPath()
Raw output 
This method returns a value that is not checked. The return value should be checked since it can indicate an unusual or unexpected function execution. For example, the File.delete() method returns false if the file could not be successfully deleted (rather than throwing an Exception). If you don't check the result, you won't notice if the method invocation signals unexpected behavior by returning an atypical return value.
}

return getBuildContainer().getLocation();
}

private boolean hasCustomBuild()
{
String userArgs = getProperty(CMAKE_ARGUMENTS);
// Custom build directory
String[] cmakeArgumentsArr = userArgs.split(" "); //$NON-NLS-1$
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(getProject(), customBuildDir);
}
catch (CoreException e)
{
Logger.log(e);
}

return !customBuildDir.isBlank();
return path;
}

@Override
Expand Down Expand Up @@ -279,7 +247,7 @@
new Status(IStatus.ERROR, IDFCorePlugin.PLUGIN_ID, Messages.IDFToolChainsMissingErrorMsg));
}
this.toolChainFile = manager.getToolChainFileFor(toolChain);
return toolChainFile;

Check warning on line 250 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/IDFBuildConfiguration.java

View workflow job for this annotation

GitHub Actions / spotbugs

EI_EXPOSE_REP 

com.espressif.idf.core.build.IDFBuildConfiguration.getToolChainFile() may expose internal representation by returning IDFBuildConfiguration.toolChainFile
Raw output 
Returning a reference to a mutable object value stored in one of the object's fields exposes the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Returning a new copy of the object is better approach in many situations.
}

private boolean isLocal() throws CoreException
Expand All @@ -306,7 +274,7 @@
Logger.log(e);
}

this.monitor = monitor;

Check warning on line 277 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/IDFBuildConfiguration.java

View workflow job for this annotation

GitHub Actions / spotbugs

EI_EXPOSE_REP2 

com.espressif.idf.core.build.IDFBuildConfiguration.build(int, Map, IConsole, IProgressMonitor) may expose internal representation by storing an externally mutable object into IDFBuildConfiguration.monitor
Raw output 
This code stores a reference to an externally mutable object into the internal representation of the object.  If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.
isProgressSet = false;

IProject project = getProject();
Expand All @@ -327,7 +295,7 @@
Path buildDir = getBuildDirectory();
if (!buildDir.toFile().exists())
{
buildDir.toFile().mkdir();

Check warning on line 298 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/IDFBuildConfiguration.java

View workflow job for this annotation

GitHub Actions / spotbugs

RV_RETURN_VALUE_IGNORED_BAD_PRACTICE 

Exceptional return value of java.io.File.mkdir() ignored in com.espressif.idf.core.build.IDFBuildConfiguration.build(int, Map, IConsole, IProgressMonitor)
Raw output 
This method returns a value that is not checked. The return value should be checked since it can indicate an unusual or unexpected function execution. For example, the File.delete() method returns false if the file could not be successfully deleted (rather than throwing an Exception). If you don't check the result, you won't notice if the method invocation signals unexpected behavior by returning an atypical return value.
}

try
Expand All @@ -348,38 +316,38 @@

private boolean buildPrechecks(IConsole console) throws Exception
{
ProjectDescriptionReader projectDescriptionReader = new ProjectDescriptionReader(getProject());
String projectDescriptionIdfPath = projectDescriptionReader.getIdfPath();
Path pathPdIdfPath = Paths.get(projectDescriptionIdfPath);

if (StringUtil.isEmpty(projectDescriptionIdfPath))
{
return true;
}
IDFEnvironmentVariables idfEnvironmentVariables = new IDFEnvironmentVariables();
String envIdfPath = idfEnvironmentVariables.getEnvValue(IDFEnvironmentVariables.IDF_PATH);
Path pathEnvIdf = Paths.get(envIdfPath);

boolean samePaths = false;
if (Platform.getOS().equals(Platform.OS_WIN32))
{
samePaths = pathEnvIdf.toString().equalsIgnoreCase(pathPdIdfPath.toString());
}
else
{
samePaths = pathEnvIdf.toString().equals(pathPdIdfPath.toString());
}

if (!samePaths)
{
String outputMessage = MessageFormat.format(Messages.IDFBuildConfiguration_PreCheck_DifferentIdfPath,
projectDescriptionIdfPath, envIdfPath);
console.getInfoStream().write(outputMessage);

return false;
}

return true;

Check warning on line 350 in bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/IDFBuildConfiguration.java

View workflow job for this annotation

GitHub Actions / spotbugs

THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION 

Method lists Exception in its throws clause.
Raw output 
Method lists Exception in its throws clause.
When declaring a method, the types of exceptions in the throws clause should be the most specific. Therefore, using Exception in the throws clause would force the caller to either use it in its own throws clause, or use it in a try-catch block (when it does not necessarily contain any meaningful information about the thrown exception).

For more information, see the SEI CERT ERR07-J rule [https://wiki.sei.cmu.edu/confluence/display/java/ERR07-J.+Do+not+throw+RuntimeException%2C+Exception%2C+or+Throwable].
}

private void runCmakeBuildCommand(IConsole console, IProgressMonitor monitor, IProject project, Instant start,
Expand Down
1 change: 1 addition & 0 deletions bundles/com.espressif.idf.core/src/com/espressif/idf/core/build/IDFLaunchConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@ public final class IDFLaunchConstants
public static final String IDF_TARGET_TYPE = "com.espressif.idf.launch.serial.core.serialFlashTarget"; //$NON-NLS-1$
public static final String OPEN_SERIAL_MONITOR = "OPEN_SERIAL_MONITOR"; //$NON-NLS-1$
public static final String SERIAL_MONITOR_ENCODING = "SERIAL_MONITOR_ENCODING"; //$NON-NLS-1$
public static final String BUILD_FOLDER_PATH = "com.espressif.idf.launch.serial.core.idfBuildFolderPath"; //$NON-NLS-1$
}
33 changes: 33 additions & 0 deletions ...l.ui/src/com/espressif/idf/launch/serial/ui/internal/SerialFlashLaunchConfigTabGroup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,25 @@
*******************************************************************************/
package com.espressif.idf.launch.serial.ui.internal;

import java.nio.file.Path;

import org.eclipse.cdt.debug.core.launch.CoreBuildLaunchConfigDelegate;
import org.eclipse.cdt.launch.ui.corebuild.CoreBuildTab;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IResource;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.debug.core.ILaunchConfiguration;
import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy;
import org.eclipse.debug.ui.AbstractLaunchConfigurationTabGroup;
import org.eclipse.debug.ui.CommonTab;
import org.eclipse.debug.ui.EnvironmentTab;
import org.eclipse.debug.ui.ILaunchConfigurationDialog;
import org.eclipse.debug.ui.ILaunchConfigurationTab;
import org.eclipse.launchbar.ui.internal.LaunchBarLaunchConfigDialog;

import com.espressif.idf.core.build.IDFLaunchConstants;
import com.espressif.idf.core.logging.Logger;
import com.espressif.idf.core.util.IDFUtil;

@SuppressWarnings("restriction")
public class SerialFlashLaunchConfigTabGroup extends AbstractLaunchConfigurationTabGroup
Expand All @@ -47,6 +54,32 @@ public void createTabs(ILaunchConfigurationDialog dialog, String mode)

}

@Override
public void performApply(ILaunchConfigurationWorkingCopy configuration)
{
super.performApply(configuration);
try
{
IProject project = CoreBuildLaunchConfigDelegate.getProject(configuration);
if (project == null)
{
return;
}
String buildFolder = configuration.getAttribute(IDFLaunchConstants.BUILD_FOLDER_PATH,
IDFUtil.getBuildDir(project));
buildFolder = buildFolder.isBlank() ? IDFUtil.getBuildDir(project) : buildFolder;
if (!Path.of(buildFolder).isAbsolute())
{
buildFolder = project.getLocation().append(buildFolder).toOSString();
}
IDFUtil.setBuildDir(project, buildFolder);
}
catch (CoreException e)
{
Logger.log(e);
}
}

@Override
public void initializeFrom(ILaunchConfiguration configuration)
{
Expand Down
76 changes: 48 additions & 28 deletions bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/dialogs/CMakeBuildTab2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
*******************************************************************************/
package com.espressif.idf.ui.dialogs;

import java.nio.file.Path;
import java.util.Map;

import org.eclipse.cdt.cmake.core.internal.CMakeBuildConfiguration;
import org.eclipse.cdt.debug.core.launch.CoreBuildLaunchConfigDelegate;
import org.eclipse.cdt.launch.internal.ui.LaunchMessages;
import org.eclipse.cdt.launch.ui.corebuild.CommonBuildTab;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.runtime.CoreException;
Expand All @@ -31,12 +31,14 @@
import org.eclipse.swt.widgets.Button;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Control;
import org.eclipse.swt.widgets.DirectoryDialog;
import org.eclipse.swt.widgets.Group;
import org.eclipse.swt.widgets.Label;
import org.eclipse.swt.widgets.Text;

import com.espressif.idf.core.IDFCorePlugin;
import com.espressif.idf.core.build.IDFBuildConfigurationProvider;
import com.espressif.idf.core.build.IDFLaunchConstants;
import com.espressif.idf.core.logging.Logger;
import com.espressif.idf.core.util.LaunchTargetHelper;
import com.espressif.idf.core.util.RecheckConfigsHelper;
Expand All @@ -48,14 +50,15 @@ public class CMakeBuildTab2 extends CommonBuildTab
private static final String LOCAL_CMAKE_ARGUMENTS = "local_cmake_arguments"; //$NON-NLS-1$
private static final String UNIX_MAKEFILES = "Unix Makefiles"; //$NON-NLS-1$
private static final String NINJA = "Ninja"; //$NON-NLS-1$
private static final String DEFAULT_CMAKE_MSG = "-B customBuildFolder"; //$NON-NLS-1$
private static final String DEFAULT_CMAKE_MSG = ""; //$NON-NLS-1$
private static final String DEFAULT_BUILD_MSG = "cmake --build ."; //$NON-NLS-1$
private static final String DEFAULT_CLEAN_MSG = "ninja clean"; //$NON-NLS-1$
private Button unixGenButton;
private Button ninjaGenButton;
private Text cmakeArgsText;
private Text buildCommandText;
private Text cleanCommandText;
private Text buildFolderText;

@Override
protected String getBuildConfigProviderId()
Expand Down Expand Up @@ -106,6 +109,37 @@ public void widgetSelected(SelectionEvent e)
}
});

label = new Label(cmakeGroup, SWT.NONE);
label.setText(Messages.CMakeBuildTab2_BuildFolderTextLbl);

// Create a composite to hold the text field and button
Composite buildFolderComp = new Composite(cmakeGroup, SWT.NONE);
buildFolderComp.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false));
buildFolderComp.setLayout(new GridLayout(2, false)); // Two columns: Text field & Button

// Text field for displaying the build folder path
buildFolderText = new Text(buildFolderComp, SWT.BORDER);
buildFolderText.setLayoutData(new GridData(SWT.FILL, SWT.CENTER, true, false));
buildFolderText.setMessage(Messages.CMakeBuildTab2_BuildFolderTextMsg);
buildFolderText.setToolTipText(Messages.CMakeBuildTab2_BuildFolderTextToolTip);

// Browse button to select a folder
Button browseButton = createPushButton(buildFolderComp, LaunchMessages.Launch_common_Browse_1, null); // $NON-NLS-1$
browseButton.addSelectionListener(new SelectionAdapter()
{
@Override
public void widgetSelected(SelectionEvent e)
{
DirectoryDialog dialog = new DirectoryDialog(parent.getShell());
dialog.setMessage(Messages.CMakeBuildTab2_SelectBuildFolderMsg);
String selectedDir = dialog.open();
if (selectedDir != null)
{
buildFolderText.setText(selectedDir);
}
}
});

label = new Label(cmakeGroup, SWT.NONE);
label.setText(Messages.CMakeBuildTab2_AdditionalCMakeArgs);

Expand Down Expand Up @@ -156,14 +190,16 @@ public void initializeFrom(ILaunchConfiguration configuration)
{
IProject project = CoreBuildLaunchConfigDelegate.getProject(configuration);
RecheckConfigsHelper.revalidateToolchain(project);

}
catch (CoreException e)
{
Logger.log(e);
}
try
{
String buildFolderPath = configuration.getAttribute(IDFLaunchConstants.BUILD_FOLDER_PATH, StringUtil.EMPTY);
buildFolderText.setText(buildFolderPath);

String generator = configuration.getAttribute(CMakeBuildConfiguration.CMAKE_GENERATOR, StringUtil.EMPTY);
updateGeneratorButtons(generator);

Expand Down Expand Up @@ -212,6 +248,8 @@ public void performApply(ILaunchConfigurationWorkingCopy configuration)
Logger.log(e);
}

configuration.setAttribute(IDFLaunchConstants.BUILD_FOLDER_PATH, buildFolderText.getText());

configuration.setAttribute(CMakeBuildConfiguration.CMAKE_GENERATOR,
ninjaGenButton.getSelection() ? NINJA : UNIX_MAKEFILES);

Expand All @@ -220,8 +258,7 @@ public void performApply(ILaunchConfigurationWorkingCopy configuration)
{
configuration.setAttribute(LOCAL_CMAKE_ARGUMENTS, cmakeArgs);
if (project != null)
configuration.setAttribute(CMakeBuildConfiguration.CMAKE_ARGUMENTS,
getCmakeArgumentsWithAbsProjectPath(project, cmakeArgs));
configuration.setAttribute(CMakeBuildConfiguration.CMAKE_ARGUMENTS, cmakeArgs);
}
else
{
Expand Down Expand Up @@ -257,6 +294,7 @@ protected void saveProperties(Map<String, String> properties)
super.saveProperties(properties);
properties.put(CMakeBuildConfiguration.CMAKE_GENERATOR, ninjaGenButton.getSelection() ? NINJA : UNIX_MAKEFILES);

properties.put(IDFLaunchConstants.BUILD_FOLDER_PATH, buildFolderText.getText().trim());
properties.put(LOCAL_CMAKE_ARGUMENTS, cmakeArgsText.getText().trim());
properties.put(CMakeBuildConfiguration.BUILD_COMMAND, buildCommandText.getText().trim());
properties.put(CMakeBuildConfiguration.CLEAN_COMMAND, cleanCommandText.getText().trim());
Expand All @@ -283,6 +321,11 @@ protected void restoreProperties(Map<String, String> properties)
}
}

String buildFolderPath = properties.get(IDFLaunchConstants.BUILD_FOLDER_PATH);
if (buildFolderPath != null)
{
buildFolderText.setText(buildFolderPath);
}
String cmakeArgs = properties.get(LOCAL_CMAKE_ARGUMENTS);
if (cmakeArgs != null)
{
Expand Down Expand Up @@ -352,27 +395,4 @@ public String getName()
return "CMake"; //$NON-NLS-1$
}

private String getCmakeArgumentsWithAbsProjectPath(IProject project, String cmakeArgumets)
{
String buildFolder = StringUtil.EMPTY;
String[] cmakeArgsArr = cmakeArgsText.getText().trim().split("\\s+"); // Split on any whitespace //$NON-NLS-1$

for (int i = 0; i < cmakeArgsArr.length - 1; i++)
{
if (cmakeArgsArr[i].equals("-B")) //$NON-NLS-1$
{
buildFolder = cmakeArgsArr[i + 1];
break;
}
}

if (!Path.of(buildFolder).isAbsolute())
{
// Getting the first argument after -B option
cmakeArgumets = cmakeArgumets.replaceFirst("(?<=-B)\\s+(\\S+)", //$NON-NLS-1$
" " + project.getLocation().append(buildFolder)); //$NON-NLS-1$
}
return cmakeArgumets;
}

}
4 changes: 4 additions & 0 deletions bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/dialogs/Messages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,14 @@ public class Messages extends NLS
public static String DeleteResourcesWizard_project_deleteConfigurations;
public static String CMakeBuildTab2_AdditionalCMakeArgs;
public static String CMakeBuildTab2_BuildCmd;
public static String CMakeBuildTab2_BuildFolderTextLbl;
public static String CMakeBuildTab2_BuildFolderTextMsg;
public static String CMakeBuildTab2_BuildFolderTextToolTip;
public static String CMakeBuildTab2_CleanCmd;
public static String CMakeBuildTab2_CMakeSettings;
public static String CMakeBuildTab2_Generator;
public static String CMakeBuildTab2_Ninja;
public static String CMakeBuildTab2_SelectBuildFolderMsg;
public static String CMakeBuildTab2_UnixMakeFiles;
public static String EraseFlashDialog_Title;
public static String EraseFlashDialog_OkButton;
Expand Down
4 changes: 4 additions & 0 deletions bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/dialogs/messages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@ BuildView_HintMsgLbl=Hint
BuildView_NoAvailableHintsMsg=No available hints found
CMakeBuildTab2_AdditionalCMakeArgs=Additional CMake arguments:
CMakeBuildTab2_BuildCmd=Build command
CMakeBuildTab2_BuildFolderTextLbl=Build folder location:
CMakeBuildTab2_BuildFolderTextMsg=Enter a path or leave empty to use the default build folder.
CMakeBuildTab2_BuildFolderTextToolTip=Enter a path or leave empty to use the default build folder.
CMakeBuildTab2_CleanCmd=Clean command
CMakeBuildTab2_CMakeSettings=CMake Settings
CMakeBuildTab2_Generator=Generator
CMakeBuildTab2_Ninja=Ninja
CMakeBuildTab2_SelectBuildFolderMsg=Select a build folder:
CMakeBuildTab2_UnixMakeFiles=Unix Makefiles
EraseFlashDialog_Title=Erase Flash
EraseFlashDialog_OkButton=Erase Flash
Expand Down
Loading