-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Added continued WSL support #4346
Conversation
#4260 and jupyter/help#496 WSL support was broken with the update to using an intermediate tempfile. This is because WSL uses Linux-like paths that Windows does not handle. If the user has setup a Xserver and runs a graphical browser within their WSL, the previous update would still work. **However**, if the user sets BROWSER to their Windows internet browser of choice, the tempfile path cannot be opened. While the user can just copy & paste the URL into their browser, this isn't super helpful for beginning/intermediate level students. This commit addresses that and dynamically opens the browser using the former one-time token method. This is dependent on the users platform having both 'Linux' as its system, and 'Microsoft' being present within the release. These two items should be true iff the user is on a WSL system. All other cases should go through the tempfile method recently added.
Is there a reliable way to translate the Linux style path into a Windows one? |
There have been attempts within the WSL ecosystem: specifically Since there is no elegant/reliable way for WSL to create the file and translate the path such that it can be passed to a Windows browser, this PR checks to make sure that the user is within a WSL environment and checks to make sure they aren't using any x11 forwarding. If these conditions are met, it will spawn using a token instead of a file. |
I don't think special directories are a big problem, because we can control where we put the file, and it looks like the translation is fairly straightforward for normal files. However, I guess it might be difficult to detect whether the default will launch a browser inside Linux (with a Linux path) or invoke a browser in Windows (so the path needs translating)? |
There is only one way to launch a browser from within the Linux WSL environment: the user has to have an Xserver installed on the Windows side and the Linux side needs the environment variable Therefore, if you wanted a way to check if it is WSL and linux-based browser: import os
from platform import uname
# Check to see if in a WSL environment
if uname().system == 'Linux' and 'Microsoft' in uname().release:
if not os.environ['DISPLAY']:
# This is a Windows-side browser
else:
# This is a Linux-side browser |
And it would be unusual to have an X server set up but no browser installed in Linux (or installed but not configured as default?) |
I guess the only way I can think of to be sure (after checking for the WSL environment above) is instead of checking for This is in cases like mine, where I have both an Xserver and |
Thanks. So if DISPLAY is set but BROWSER isn't, it must be a Linux browser. If BROWSER is set but not DISPLAY, it must be a Windows browser (or a terminal browser, but that won't work anyway). If they're both set, it's not certain what is being used, but there's a good chance it's a Windows browser. Is it possible to see the command lines of processes from other users with WSL? The possibility of this in real Linux was what prompted the file-URL approach. If that's definitely not possible on WSL, I think detecting the platform and falling back to opening the http URL makes sense. |
By default, a WSL environment does not set This means that the user (of a WSL environment) would only set Therefore, if we check like so: import os
from platform import uname
# Check to see if in a WSL environment
if uname().system == 'Linux' and 'Microsoft' in uname().release:
if os.environ['BROWSER']:
# This is a Windows-side browser
elif os.environ['DISPLAY']:
# This is a X11 Linux-side browser
else:
# Will not natively spawn a browser, so maybe just default to --no-browser We can properly detect a WSL environment, and (within reason) select the appropriate URL/File to pass to the browser. |
Hello, How would this logic be affected by a user running WSL and doesn't set a
Does the above logic handle this case |
Didn't think about that @raulf2012 . I have attempted to fix that here. The code below would take into account user-overrides in the def launch_browser(self):
# If 'Linux' and 'Microsoft' are both in the uname, the user is within a
# WSL environment
unix_name = platform.uname()
wsl_environ = 'Linux' in unix_name.system and 'Microsoft' in unix_name.release
# Check to see if the user has overridden the browser to redirect
# to a Windows-based browser. If this is the case, then redirect should
# be a token-based URL instead of a file
wsl_win_browser = os.environ['BROWSER'] or '.exe' in self.browser
wsl_redirect = wsl_environ and wsl_win_browser
try:
browser = webbrowser.get(self.browser or None)
except webbrowser.Error as e:
self.log.warning(_('No web browser found: %s.') % e)
browser = None
if not browser:
return
# If both of these conditions are True, it is a reasonable assumption
# that the user is in a WSL environment and redirecting the notebook
# to a Windows-based browser. Therefore, we spawn using a token-based URL
if wsl_redirect:
uri = self.default_url[len(self.base_url):]
if self.token:
uri = url_concat(uri, {'token': self.token})
# Now handle all the cases that aren't WSL porting to Windows-based browser
elif self.file_to_run:
if not os.path.exists(self.file_to_run):
self.log.critical(_("%s does not exist") % self.file_to_run)
self.exit(1)
relpath = os.path.relpath(self.file_to_run, self.notebook_dir)
uri = url_escape(url_path_join('notebooks', *relpath.split(os.sep)))
# Write a temporary file to open in the browser
fd, open_file = tempfile.mkstemp(suffix='.html')
with open(fd, 'w', encoding='utf-8') as fh:
self._write_browser_open_file(uri, fh)
else:
open_file = self.browser_open_file
# Now control the file/URL redirect
if wsl_redirect:
if browser:
b = lambda : browser.open(url_path_join(self.connection_url, uri),
new=self.webbrowser_open_new)
threading.Thread(target=b).start()
else:
b = lambda: browser.open(
urljoin('file:', pathname2url(open_file)),
new=self.webbrowser_open_new)
threading.Thread(target=b).start() |
This will track both the `jupyter_notebook_config.py` overrides mentioned by #4346 (comment) and `BROWSER` environment variable overrides originally proposed.
This also an issue running on a chromebook. Jupyter must be installed on a linux vm, but the chrome os browser does not have easy access to the linux file system. |
This is still an issue which requires some fun conda create -n some_env "python>=3.7" jupyter jupyterlab notebook=5.7.2 "tornado<6" # etc Note: because of tornado updates, if you don't downgrade it along with notebook, |
As of today, this issue is still present and prevents a straightforward jupyterlab/jupyter notebook installation on WSL systems. #4346 (comment) is still a good work around |
This issue also exists for Termux on Android. I'm +1 for simple config option to open the http localhost URL instead of a file, since that would work cross-platform. It would be up to the user to override the default behaviour iff they know they only have a single user on their platform. |
I believe this issue is resolved by #4999 and the 6.0.2 release and can be closed, right? Thank you very much! |
I agree, thanks all! |
#4260 and jupyter/help#496 WSL support was broken with the update to using an intermediate tempfile. This is because WSL uses Linux-like paths that Windows does not handle.
If the user has setup a Xserver and runs a graphical browser within their WSL, the previous update would still work. However, if the user sets BROWSER to their Windows internet browser of choice, the tempfile path cannot be opened.
While the user can just copy & paste the URL into their browser, this isn't super helpful for beginning/intermediate level students.
This commit addresses that and dynamically opens the browser using the former one-time token method. This is dependent on the users platform having both 'Linux' as its system, and 'Microsoft' being present within the release. These two items should be true iff the user is on a WSL system.
All other cases should go through the tempfile method recently added.