Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.espressif.idf.core.IDFCorePlugin;
import com.espressif.idf.core.IDFEnvironmentVariables;
import com.espressif.idf.core.ProcessBuilderFactory;
import com.espressif.idf.core.SystemExecutableFinder;
import com.espressif.idf.core.logging.Logger;
import com.espressif.idf.core.tools.exceptions.EimVersionMismatchException;
import com.espressif.idf.core.tools.vo.EimJson;
Expand All @@ -48,16 +49,59 @@ public ToolInitializer(Preferences preferences)

public boolean isEimInstalled()
{
return !StringUtil.isEmpty(resolveEimExecutablePath(null));
}
Comment on lines 50 to +53
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all call sites of isEimInstalled() across the plugin bundles
rg -n --type=java -B2 -A2 '\bisEimInstalled\s*\('

Repository: espressif/idf-eclipse-plugin

Length of output: 1898


🏁 Script executed:

# Read the resolveEimExecutablePath method to understand the logic branches
cd bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools
cat -n ToolInitializer.java | sed -n '75,110p'

Repository: espressif/idf-eclipse-plugin

Length of output: 1248


🏁 Script executed:

# Read the isEimIdfJsonPresent method
cd bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools
cat -n ToolInitializer.java | sed -n '107,115p'

Repository: espressif/idf-eclipse-plugin

Length of output: 404


🏁 Script executed:

# Read the EimButtonLaunchListener.java context around line 72 to understand the impact
cat -n bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/EimButtonLaunchListener.java | sed -n '65,90p'

Repository: espressif/idf-eclipse-plugin

Length of output: 836


🏁 Script executed:

# Read loadEimJson() to understand the exception it declares
cd bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools
cat -n ToolInitializer.java | sed -n '114,130p'

Repository: espressif/idf-eclipse-plugin

Length of output: 465


🏁 Script executed:

# Also check if there are any other methods in ToolInitializer that catch EimVersionMismatchException
rg -n 'EimVersionMismatchException' bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/ToolInitializer.java

Repository: espressif/idf-eclipse-plugin

Length of output: 220


isEimInstalled() returns false when EIM exists only in eim_idf.json, triggering unnecessary downloads.

Passing null to resolveEimExecutablePath() skips the JSON-path branch (lines 83–90). If EIM is located only at a custom path recorded in eim_idf.json — not on the system PATH, not in EIM_PATH, and not at the default location — isEimInstalled() returns false. This causes EimButtonLaunchListener.java:72 to unconditionally trigger a download job even though EIM is already installed.

The simplest fix is to load the JSON locally and pass it through:

🔧 Proposed fix
 public boolean isEimInstalled()
 {
-  return !StringUtil.isEmpty(resolveEimExecutablePath(null));
+  EimJson eimJson = null;
+  try
+  {
+    eimJson = loadEimJson();
+  }
+  catch (EimVersionMismatchException e)
+  {
+    Logger.log(e);
+  }
+  return !StringUtil.isEmpty(resolveEimExecutablePath(eimJson));
 }
📝 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 boolean isEimInstalled()
{
return !StringUtil.isEmpty(resolveEimExecutablePath(null));
}
public boolean isEimInstalled()
{
EimJson eimJson = null;
try
{
eimJson = loadEimJson();
}
catch (EimVersionMismatchException e)
{
Logger.log(e);
}
return !StringUtil.isEmpty(resolveEimExecutablePath(eimJson));
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@bundles/com.espressif.idf.core/src/com/espressif/idf/core/tools/ToolInitializer.java`
around lines 50 - 53, isEimInstalled() currently passes null to
resolveEimExecutablePath(), skipping the eim_idf.json branch and returning false
when EIM is only recorded in eim_idf.json; change isEimInstalled() to load the
local eim_idf.json configuration (using the same loader used elsewhere in this
class) and pass that config into resolveEimExecutablePath(config) so
resolveEimExecutablePath can detect a custom path in eim_idf.json and return
true when appropriate, preventing EimButtonLaunchListener from triggering
unnecessary downloads.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@alirana01 please check this

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


/**
* Looks for an {@code eim} executable on the process {@code PATH}, using the same rules as other tools in this
* plugin ({@link SystemExecutableFinder}: PATHEXT on Windows, plain name on Linux/macOS).
*
* @return absolute path to the executable, or empty if not found
*/
private String findEimOnSystemPath()
{
IPath eimPath = new SystemExecutableFinder().find("eim"); //$NON-NLS-1$
return eimPath != null ? eimPath.toOSString() : StringUtil.EMPTY;
}

/**
* Resolves the EIM executable path: <strong>system {@code PATH} first</strong>, then {@code eimPath} from
* {@code eim_idf.json} when the path exists on disk, then {@code EIM_PATH} env variable, then
* {@link #getDefaultEimPath()} (existence-checked).
*
* @param eimJson parsed JSON or {@code null}
* @return resolved absolute path string, or empty if nothing could be resolved
*/
public String resolveEimExecutablePath(EimJson eimJson)
{
String fromPath = findEimOnSystemPath();
if (!StringUtil.isEmpty(fromPath))
{
return fromPath;
}

if (eimJson != null && !StringUtil.isEmpty(eimJson.getEimPath()))
{
String jsonPath = eimJson.getEimPath();
if (Files.exists(Paths.get(jsonPath)))
{
return jsonPath;
}
}

String eimExePathEnv = idfEnvironmentVariables.getEnvValue(IDFEnvironmentVariables.EIM_PATH);
boolean exists = !StringUtil.isEmpty(eimExePathEnv) && Files.exists(Paths.get(eimExePathEnv));
if (!exists)
if (!StringUtil.isEmpty(eimExePathEnv) && Files.exists(Paths.get(eimExePathEnv)))
{
return eimExePathEnv;
}

Path defaultEimPath = getDefaultEimPath();
if (defaultEimPath != null && Files.exists(defaultEimPath))
{
// Fallback: check in user home .espressif/eim_gui folder
Path defaultEimPath = getDefaultEimPath();
if (defaultEimPath != null)
exists = Files.exists(defaultEimPath);
return defaultEimPath.toString();
}
return exists;

return StringUtil.EMPTY;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

public boolean isEimIdfJsonPresent()
Expand Down Expand Up @@ -138,38 +182,50 @@ public boolean isEspIdfSet()
public Path getDefaultEimPath()
{
String userHome = System.getProperty("user.home"); //$NON-NLS-1$
Path defaultEimPath;
String os = Platform.getOS();
if (os.equals(Platform.OS_WIN32))
{
defaultEimPath = Paths.get(userHome, ".espressif", "eim_gui", //$NON-NLS-1$//$NON-NLS-2$
"eim.exe"); //$NON-NLS-1$
}
else if (os.equals(Platform.OS_MACOSX))
{
defaultEimPath = Paths.get("/Applications", //$NON-NLS-1$
"eim.app", "Contents", //$NON-NLS-1$//$NON-NLS-2$
"MacOS", "eim"); //$NON-NLS-1$ //$NON-NLS-2$
}
else
{
defaultEimPath = Paths.get(userHome, ".espressif", //$NON-NLS-1$
"eim_gui", "eim"); //$NON-NLS-1$//$NON-NLS-2$
}

return defaultEimPath;
}

public void findAndSetEimPath()
{
Path defaultEimPath = getDefaultEimPath();

if (defaultEimPath != null)
setEimPathInEnvVar(defaultEimPath.toString());
Path defaultEimPath;
String os = Platform.getOS();
if (os.equals(Platform.OS_WIN32))
{
defaultEimPath = Paths.get(userHome, ".espressif", "eim_gui", //$NON-NLS-1$//$NON-NLS-2$
"eim.exe"); //$NON-NLS-1$
if (!Files.exists(defaultEimPath))
{
Path eimGuiDir = Paths.get(userHome, ".espressif", "eim_gui"); //$NON-NLS-1$ //$NON-NLS-2$
if (Files.isDirectory(eimGuiDir))
{
try (var entries = Files.list(eimGuiDir))
{
Path found = entries
.filter(Files::isRegularFile)
.filter(p -> p.getFileName().toString().toLowerCase().startsWith("eim") //$NON-NLS-1$
&& p.getFileName().toString().toLowerCase().endsWith(".exe")) //$NON-NLS-1$
.findFirst()
.orElse(null);
if (found != null)
{
return found;
}
}
catch (IOException e)
{
Logger.log(e);
}
}
}
}
else if (os.equals(Platform.OS_MACOSX))
{
defaultEimPath = Paths.get("/Applications", //$NON-NLS-1$
"eim.app", "Contents", //$NON-NLS-1$//$NON-NLS-2$
"MacOS", "eim"); //$NON-NLS-1$ //$NON-NLS-2$
}
else
{
defaultEimPath = Paths.get(userHome, ".espressif", //$NON-NLS-1$
"eim_gui", "eim"); //$NON-NLS-1$//$NON-NLS-2$
}

return defaultEimPath;
}

private void setEimPathInEnvVar(String eimPath)
{
idfEnvironmentVariables.addEnvVariable(IDFEnvironmentVariables.EIM_PATH, eimPath);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,11 @@ else if (toolInitializer.isEimIdfJsonPresent() && !toolInitializer.isEspIdfSet()
promptUserToOpenToolManager(eimJson);
}

// Set EimPath on every startup to ensure proper path in configurations
if (eimJson != null)
// Set EimPath on every startup: system PATH first, then eim_idf.json, then default locations
String resolvedEimPath = toolInitializer.resolveEimExecutablePath(eimJson);
if (!StringUtil.isEmpty(resolvedEimPath))
{
idfEnvironmentVariables.addEnvVariable(IDFEnvironmentVariables.EIM_PATH, eimJson.getEimPath());
}
else
{
// Fail-safe call to ensure if the eim is in Applications or user.home it is setup in env vars
toolInitializer.findAndSetEimPath();
idfEnvironmentVariables.addEnvVariable(IDFEnvironmentVariables.EIM_PATH, resolvedEimPath);
}
Comment on lines +118 to 123
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

EIM_PATH may be overwritten with a non-existent default on every startup.

Because ToolInitializer.resolveEimExecutablePath() currently returns getDefaultEimPath().toString() without checking existence, this branch will almost always execute and set EIM_PATH to a path like ~/.espressif/eim_gui/eim[.exe] even when EIM isn't installed. Callers such as EimButtonLaunchListener (reads EIM_PATH and invokes eimLoader.launchEimWithResult(...)) will then fail at launch time with an IOException instead of taking the "EIM missing" branch.

The root-cause fix belongs in ToolInitializer.resolveEimExecutablePath (see the comment there). Once the resolver returns empty on a non-existent default, the guard here behaves as the comment implies.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/EspressifToolStartup.java`
around lines 118 - 123, ToolInitializer.resolveEimExecutablePath currently
returns getDefaultEimPath().toString() unconditionally which causes EIM_PATH to
be set to a non-existent default; update resolveEimExecutablePath to check
whether the resolved path (system PATH result or the default returned by
getDefaultEimPath()) actually exists and is executable, and return an empty
Optional/string (or null as the project style uses) when no valid executable is
found so callers (e.g., EspressifToolStartup handling EIM_PATH and
EimButtonLaunchListener calling eimLoader.launchEimWithResult) will fall back to
the “EIM missing” branch instead of storing a bogus default.


}
Expand Down Expand Up @@ -163,22 +159,15 @@ private void startExportOldConfig()
{
try
{
// if eim json is present it means that it contains the updated path and we use that else we fallback to
// finding eim in default paths
Path eimPath;
String eimPathEnvVar = idfEnvironmentVariables.getEnvValue(IDFEnvironmentVariables.EIM_PATH);
if (eimJson != null)
// Same resolution order as workspace EIM_PATH: PATH, then eim_idf.json, then defaults
String resolved = toolInitializer.resolveEimExecutablePath(eimJson);
if (StringUtil.isEmpty(resolved))
{
eimPath = Paths.get(eimJson.getEimPath());
}
else if (!StringUtil.isEmpty(eimPathEnvVar))
{
eimPath = Paths.get(eimPathEnvVar);
}
else
{
eimPath = toolInitializer.getDefaultEimPath();
Logger.log("Cannot export old config: EIM executable path could not be resolved."); //$NON-NLS-1$
writeToErrorConsoleStream(Messages.OldConfigExportCompleteFailMsg);
return;
}
Path eimPath = Paths.get(resolved);

IStatus status = toolInitializer.exportOldConfig(eimPath);
Logger.log("Tools Conversion Process Message: ");
Expand Down
Loading