-
Notifications
You must be signed in to change notification settings - Fork 10.6k
fix: use correct display ID for WSL2 when setting up Xvfb #36697
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
base: master
Are you sure you want to change the base?
Conversation
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 a hang issue in the Xvfb setup script when running on WSL2 by detecting WSL2 environments and using display :0 (which WSLg uses) instead of display :99 (standard for Xvfb). This allows the script to leverage the existing WSLg display server rather than waiting indefinitely for a new virtual display that won't be created.
Key Changes:
- Added WSL2 detection logic using
uname -r - Set display ID to 0 for WSL2 environments, 99 for standard Linux systems
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Sets up a virtual display for running map renderer and simulator without an X11 display | ||
|
|
||
| DISP_ID=99 | ||
| if uname -r | grep -q "WSL2"; then |
Copilot
AI
Nov 26, 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 WSL2 detection logic may not work correctly. WSL2 kernel versions typically contain "microsoft" or "WSL" (lowercase) in the output of uname -r, not "WSL2" (uppercase). For example, a typical WSL2 kernel version looks like "5.10.16.3-microsoft-standard-WSL2" or "5.15.90.1-microsoft-standard-WSL2".
Consider using a more robust detection pattern:
if uname -r | grep -qi "microsoft.*wsl"; thenThis uses case-insensitive matching (-qi) and checks for "microsoft" followed by "wsl" anywhere in the string, which should reliably detect WSL2 environments.
| if uname -r | grep -q "WSL2"; then | |
| if uname -r | grep -qi "microsoft.*wsl"; then |
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.
Mine looks like 6.6.87.2-microsoft-standard-WSL2. And you contradicted yourself.
|
This PR has had no activity for 24 days. It will be automatically closed in 7 days if there is no activity. |
|
This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes. |
1 similar comment
|
This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes. |
WSL2 uses WSLg which only uses display :0
In order to get the UI screenshot tests running locally, I have to run the
selfdrive/test/setup_xvfb.shscript first, but it just hangs on "Waiting for Xvfb..." without this fix.Now it's still not a great experience because the screenshots are black, I think due to a fundamental WSL limitation, but at least the script runs and I can see the window.
Note: I specifically targeted WSL2 because it seems the display stuff is different on previous versions, but I have no way to verify so just leaving the behavior the same for those.