-
-
Notifications
You must be signed in to change notification settings - Fork 10
Fix WSL/Remote OS detection for host tools installation #202
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
Conversation
Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com>
Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com>
…tions Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes WSL/Remote OS detection for host tools installation by addressing the issue where Zephyr IDE incorrectly suggested Windows-specific tools like winget when running in WSL2 or SSH remote environments. The extension now properly detects the remote OS environment and suggests appropriate tools.
Key Changes
- Added asynchronous remote platform detection using VS Code's remote development APIs
- Updated host tools module to use async platform detection for package manager selection
- Fixed Python environment setup to use correct commands and paths based on detected OS
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/utilities/utils.ts |
Added async platform detection with remote environment support using vscode.env.remoteName and uname commands |
src/setup_utilities/host_tools.ts |
Updated all package manager functions to use async platform detection instead of local OS detection |
src/panels/host_tool_install_view/HostToolInstallView.ts |
Updated UI panel to use async platform detection functions |
src/setup_utilities/west-operations.ts |
Fixed Python command selection and PATH environment variable construction for remote environments |
src/test/platform-detection.test.ts |
Added comprehensive unit tests for platform detection functionality |
| export function getPlatformName() { | ||
| // For remote environments, we need to detect asynchronously | ||
| // This synchronous function will return the cached value if available | ||
| if (remotePlatformCache !== undefined) { | ||
| switch (remotePlatformCache) { | ||
| case "darwin": | ||
| return "macos"; | ||
| case "linux": | ||
| return "linux"; | ||
| case "win32": | ||
| return "windows"; | ||
| } | ||
| } |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The synchronous getPlatformName() function now depends on a cache that may not be populated if the async version hasn't been called first. This creates an implicit dependency and could lead to inconsistent behavior. Consider documenting this behavior or ensuring the cache is always initialized.
| wsConfig.activeSetupState.env["PATH"] = path.join(pythonenv, 'Scripts') + ';'; | ||
| } else { | ||
| wsConfig.activeSetupState.env["PATH"] = path.join(pythonenv, `bin${pathdivider}`); | ||
| wsConfig.activeSetupState.env["PATH"] = path.join(pythonenv, 'bin') + ':'; |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PATH construction is hardcoded with string concatenation. Consider using path.delimiter from Node.js which automatically provides the correct path separator (';' for Windows, ':' for Unix-like systems) instead of hardcoding the separators.
| // If uname fails, try to detect Windows (though unlikely in remote) | ||
| const winResult = await executeShellCommand("ver", "", false); | ||
| if (winResult.stdout && winResult.stdout.toLowerCase().includes("windows")) { | ||
| remotePlatformCache = "win32"; | ||
| return "win32"; | ||
| } |
Copilot
AI
Oct 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Windows detection using ver command is unlikely to work in remote environments (WSL, SSH to Linux), as the comment on line 73 suggests. This code path may be unreachable and could be removed or commented as a fallback that's not expected to execute.
| // If uname fails, try to detect Windows (though unlikely in remote) | |
| const winResult = await executeShellCommand("ver", "", false); | |
| if (winResult.stdout && winResult.stdout.toLowerCase().includes("windows")) { | |
| remotePlatformCache = "win32"; | |
| return "win32"; | |
| } | |
| // If uname fails, Windows detection via 'ver' is unlikely to work in remote environments (WSL, SSH to Linux). | |
| // The following code path is unreachable and has been removed as per CodeQL recommendation. |
|
Pushing this into develop for testing on pre-release. May need more modifications. |
* Initial plan * Implement remote OS detection for WSL/SSH environments Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com> * Fix Python command and path separator for remote environments Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com> * Address code review feedback: fix path separator logic and test assertions Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com>
Problem
When running Zephyr IDE in WSL2 (Windows Subsystem for Linux) or SSH remote development environments, the extension incorrectly detected the local host OS instead of the remote OS. This caused the setup panel to suggest Windows-specific tools like
wingetwhen it should have suggested Linux tools likeapt.Example Issue:
wingetcommands ✗ (Windows tool)Root Cause
The extension used Node.js
os.platform()API, which returns the platform where the extension host process runs. In VS Code's remote development:os.platform()returns"win32"(local OS) not"linux"(remote OS)This caused platform-dependent operations to use the wrong tools and commands.
Solution
Implemented asynchronous remote environment detection using VS Code's remote development APIs:
1. Added Remote Platform Detection
getPlatformNameAsync()that checksvscode.env.remoteNameto detect remote environmentsuname -sto determine the actual remote OS2. Updated Host Tools Module
getPackageManagerForPlatformAsync()- Detects correct package manager for remote OSgetPlatformPackages()- Now async to support remote detectionHostToolInstallViewpanel to use async functions3. Fixed Python Environment Setup
getPythonCommand()now dynamically selects the correct Python command based on detected OSpython3for Linux/macOSpythonfor Windows:for Unix-like systems;for WindowsbinvsScripts)4. Added Comprehensive Tests
platform-detection.test.tswith unit testsChanges
Files Modified:
src/utilities/utils.ts- Added async platform detection with remote environment supportsrc/setup_utilities/host_tools.ts- Updated all functions to use async platform detectionsrc/panels/host_tool_install_view/HostToolInstallView.ts- Updated to use async functionssrc/setup_utilities/west-operations.ts- Fixed Python command selection and PATH handlingFiles Added:
src/test/platform-detection.test.ts- Unit tests for platform detectionImpact
Fixed Scenarios
apt) and commandsBackward Compatibility
getPlatformName()function retained for compatibilityTesting
Fixes issue where setup panel proposed Windows tools in WSL2/Ubuntu environments.
Original prompt
Fixes #182
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.