Skip to content

IEP-1628: Remove conflicting System env variables for export command #1305

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

IEP-1628: Remove conflicting System env variables for export command #1305

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ea86896
IEP-1628: Remove conflicting System env variables for export command
alirana01 Sep 8, 2025
060265c
updated keys to remove that can cause possible issues
alirana01 Sep 9, 2025
938727b
moved function to utility method
alirana01 Sep 15, 2025
bdd4169
fixing typo
alirana01 Sep 15, 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
28 changes: 28 additions & 0 deletions bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/update/ExportIDFTools.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -132,6 +133,8 @@ public IStatus getToolsExportOutputFromGivenIdfPath(final String pythonExePath,
addGitToEnvironment(environment, gitExePath);
}

cleanUpSystemEnvironment(environment);

Copy link

@coderabbitai coderabbitai bot Sep 9, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Apply env cleanup to all export paths, not just the “given IDF path” flow.

The conflicting vars will still leak into these two flows:

  • runToolsExportAndProcessOutput(...)
  • getToolsExportOutput(...)

Call cleanUpSystemEnvironment(environment) there as well to make the fix complete.

*** a/.../ExportIDFTools.java
--- b/.../ExportIDFTools.java
@@ public IStatus runToolsExportAndProcessOutput(final String pythonExePath, final String gitExePath, final MessageConsoleStream console, MessageConsoleStream errorConsoleStream)
-        if (gitExePath != null)
-        {
-            addGitToEnvironment(environment, gitExePath);
-        }
+        if (gitExePath != null)
+        {
+            addGitToEnvironment(environment, gitExePath);
+        }
+        cleanUpSystemEnvironment(environment);
@@ public IStatus getToolsExportOutput(final String pythonExePath, final String gitExePath, final MessageConsoleStream console, MessageConsoleStream errorConsoleStream)
-        if (gitExePath != null)
-        {
-            addGitToEnvironment(environment, gitExePath);
-        }
+        if (gitExePath != null)
+        {
+            addGitToEnvironment(environment, gitExePath);
+        }
+        cleanUpSystemEnvironment(environment);

Please also verify Windows tool installation via all three code paths.

🏁 Script executed:

#!/bin/bash
set -eux
# locate ExportIDFTools.java
FILE=$(rg -l "class ExportIDFTools" -g "*.java")
echo "Found file: $FILE"
# show all cleanUpSystemEnvironment calls with context
rg -nP -C3 "cleanUpSystemEnvironment" "$FILE"
# show definitions of target methods with context
echo "\nMethod runToolsExportAndProcessOutput definition with context:"
rg -nP -C3 "public IStatus runToolsExportAndProcessOutput" "$FILE"
echo "\nMethod getToolsExportOutput definition with context:"
rg -nP -C3 "public IStatus getToolsExportOutput" "$FILE"

Length of output: 2461

Add cleanup in all export flows and verify Windows tool installation

  • Insert cleanUpSystemEnvironment(environment); immediately after the addGitToEnvironment(...) block in both runToolsExportAndProcessOutput(...) and getToolsExportOutput(...).
  • Confirm Windows tool installation works correctly across all three export paths.
🤖 Prompt for AI Agents 
In
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/update/ExportIDFTools.java
around lines 136-137, add a call to cleanUpSystemEnvironment(environment)
immediately after each addGitToEnvironment(...) call in both
runToolsExportAndProcessOutput(...) and getToolsExportOutput(...); ensure the
cleanup runs on all export code paths (normal, fallback and any Windows-specific
branch) and then run or write tests to verify Windows tool installation succeeds
across all three export flows so the environment is restored and Windows tools
install correctly.

final ProcessBuilderFactory processRunner = new ProcessBuilderFactory();
try
{
Expand All @@ -157,6 +160,31 @@ public IStatus getToolsExportOutputFromGivenIdfPath(final String pythonExePath,
}
}

/**
* Remove the variables which can affect the idf_tools.py export command
* These variables can come from system environment
* @param environment
*/
private void cleanUpSystemEnvironment(Map<String, String> environment)
{
List<String> keysToRenmove = new LinkedList<>();
keysToRenmove.add(IDFEnvironmentVariables.IDF_PYTHON_ENV_PATH);
keysToRenmove.add(IDFEnvironmentVariables.IDF_PATH);
Copy link
Collaborator

@sigmaaa sigmaaa Sep 9, 2025

Choose a reason for hiding this comment

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

Don't we want to exclude other environment variables that have IDF in their name to avoid conflicts? For example, in IDFEnvironmentVariables we also have IDF_CCACHE_ENABLE, IDF_COMPONENT_MANAGER, IDF_MAINTAINER, and ESP_IDF_VERSION, which can potentially affect the plugin's functionality.

Copy link
Collaborator Author

@alirana01 alirana01 Sep 9, 2025

Choose a reason for hiding this comment

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

yeah that sounds about right I think I missed those
Thanks

keysToRenmove.add(IDFEnvironmentVariables.ESP_IDF_VERSION);
keysToRenmove.add(IDFEnvironmentVariables.IDF_CCACHE_ENABLE);
keysToRenmove.add(IDFEnvironmentVariables.IDF_COMPONENT_MANAGER);
keysToRenmove.add(IDFEnvironmentVariables.IDF_MAINTAINER);
keysToRenmove.add(IDFEnvironmentVariables.OPENOCD_SCRIPTS);

for (String key : keysToRenmove)
{
if (environment.containsKey(key))
{
environment.remove(key);
}
}
}

private List<String> getExportCommandUsingGivenIdfPath(String pythonExePath, String idfPath)
{
final List<String> arguments = new ArrayList<>();
Expand Down
Loading