Skip to content
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

Improve B00_www_browser error reporting #4

Open
leviroth opened this issue Dec 23, 2020 · 12 comments
Open

Improve B00_www_browser error reporting #4

leviroth opened this issue Dec 23, 2020 · 12 comments
Labels
enhancement New feature or request

Comments

@leviroth
Copy link

I tried using odig doc from WSL and got some confusing error messages:

$ odig doc base --browser="/mnt/c/Program\ Files\ \(x86\)/Google/Chrome/Application/chrome"
odig: [ERROR] show uri file:///home/leviroth/.opam/4.11.1/var/cache/odig/html/base/index.html: No browser found. Use the BROWSER env var to set one.
$ BROWSER="/mnt/c/Program\ Files\ \(x86\)/Google/Chrome/Application/chrome" odig doc base
odig: [ERROR] show uri file:///home/leviroth/.opam/4.11.1/var/cache/odig/html/base/index.html: No browser found. Use the BROWSER env var to set one.

WSL is a weird environment, so I'm not too surprised that odig wouldn't work. (Notably, odig is running on a Linux environment that lives inside a Windows environment, and has to reach back out into the Windows environment to find the browser.) However, I am surprised that the error message suggests that the BROWSER environment variable isn't set.

I poked around in the source a bit and, while I'm not sure I understood all of it, it seems like B00_www_browser.show ~background ~prefix browser uri always complains about a missing environment variable when browser is None, while in reality B00_www_browser.find might be giving us None for various other reasons.

@dbuenzli dbuenzli transferred this issue from b0-system/odig Dec 23, 2020
@dbuenzli
Copy link
Contributor

I poked around in the source a bit and, while I'm not sure I understood all of it, it seems like B00_www_browser.show ~background ~prefix browser uri always complains about a missing environment variable when browser is None, while in reality B00_www_browser.find might be giving us None for various other reasons.

Indeed. I suspect in this particular case the None comes from this line, that is the program lookup fails. The erroring paths (and the whole thing) should be reviewed.

If you have a toplevel at hand would you mind invoking:

# #require "b0.b00.kit";;
# open B00_std;;
# Os.Cmd.find ((Cmd.of_string "/mnt/c/Program\ Files\ \(x86\)/Google/Chrome/Application/chrome") |> Result.to_failure);;

To see what happens.

Also I love to have Windows users at hand so bear with me. Could you tell me if using:

start file:///home/leviroth/.opam/4.11.1/var/cache/odig/html/base/index.html

would do what you'd expect.

(Also if you want to make tests you can use the show-uri tool which uses the same API).

@dbuenzli
Copy link
Contributor

dbuenzli commented Dec 23, 2020

BROWSER="/mnt/c/Program\ Files\ \(x86\)/Google/Chrome/Application/chrome"

Also since you have quotes here aren't the escaped spaces too much here ?

In the toplevel instructions above also please simply try with:

"/mnt/c/Program Files (x86)/Google/Chrome/Application/chrome"

@leviroth
Copy link
Author

You're right, the escapes are useless. I tried both versions, and also one with .exe for good measure.

# #require "b0.b00.kit";;
/home/leviroth/.opam/4.11.1/lib/ocaml/unix.cma: loaded
/home/leviroth/.opam/4.11.1/lib/cmdliner: added to search path
/home/leviroth/.opam/4.11.1/lib/cmdliner/cmdliner.cma: loaded
/home/leviroth/.opam/4.11.1/lib/b0/b00/std: added to search path
/home/leviroth/.opam/4.11.1/lib/b0/b00/std/b00_std.cma: loaded
/home/leviroth/.opam/4.11.1/lib/b0/b00: added to search path
/home/leviroth/.opam/4.11.1/lib/b0/b00/b00.cma: loaded
/home/leviroth/.opam/4.11.1/lib/b0/b00/kit: added to search path
/home/leviroth/.opam/4.11.1/lib/b0/b00/kit/b00_kit.cma: loaded
# open B00_std;;
# Os.Cmd.find ((Cmd.of_string "/mnt/c/Program\ Files\ \(x86\)/Google/Chrome/Application/chrome") |> Result.to_failure);;
Warning 14: illegal backslash escape in string.
Warning 14: illegal backslash escape in string.
- : (B00_std.Cmd.t option, string) result = Ok None
# Os.Cmd.find ((Cmd.of_string "/mnt/c/Program Files (x86)/Google/Chrome/Application/chrome") |> Result.to_failure);;
- : (B00_std.Cmd.t option, string) result = Ok None
# Os.Cmd.find ((Cmd.of_string "/mnt/c/Program Files (x86)/Google/Chrome/Application/chrome.exe") |> Result.to_failure);;
- : (B00_std.Cmd.t option, string) result = Ok None

start file:///home/leviroth/.opam/4.11.1/var/cache/odig/html/base/index.html

just results in

$ start file:///home/leviroth/.opam/4.11.1/var/cache/odig/html/base/index.html

Command 'start' not found, did you mean:

  command 'rstart' from deb x11-session-utils (7.7+4)
  command 'startx' from deb xinit (1.4.1-0ubuntu2)
  command 'stat' from deb coreutils (8.30-3ubuntu2)
  command 'tart' from deb tart (3.10-1build1)

Try: sudo apt install <deb name>

So it seems like start isn't known to Ubuntu or its package manager.

@dbuenzli
Copy link
Contributor

dbuenzli commented Dec 24, 2020

So it seems like start isn't known to Ubuntu or its package manager.

Well no that's a Windows thing which B00_www_browser should likely support.

You're right, the escapes are useless.

Actually they are not useless, Cmd.of_string, which is used on the BROWSER value parses a command line, not a tool specification. So I think this is just a matter of bad quoting:

# #install_printer Cmd.dump;;
# let cmd = Cmd.of_string "/mnt/c/Program\\ Files\\ (x86)/Google/Chrome/Application/chrome";;
val cmd : (B00_std.Cmd.t, string) result =
  Ok '/mnt/c/Program\' 'Files\' '(x86)/Google/Chrome/Application/chrome'
# let cmd = Cmd.of_string "'/mnt/c/Program Files (x86)/Google/Chrome/Application/chrome'";;
val cmd : (B00_std.Cmd.t, string) result =
  Ok '/mnt/c/Program Files (x86)/Google/Chrome/Application/chrome'

Could you please try again with the correct quoting. I'm not exactly sure who invokes what and which platform is discovered but you will likely have to add the .exe.

@leviroth
Copy link
Author

Ah, so the following opened the browser:

$ odig doc base --browser="'/mnt/c/Program Files (x86)/Google/Chrome/Application/chrome.exe'"

But with an important caveat: Windows Chrome is trying to read the docs from a filesystem path, but it doesn't have access to the Linux filesystem. So it sees a path like file:///home/leviroth/.opam/4.11.1/var/cache/odig/html/base/index.html and comes back with a 404 error.

@dbuenzli dbuenzli added the enhancement New feature or request label Dec 24, 2020
@dbuenzli dbuenzli changed the title Confusing "No browser found" error on WSL Improve B00_www_browser error reporting Dec 24, 2020
@dbuenzli
Copy link
Contributor

Good.

But with an important caveat: Windows Chrome is trying to read the docs from a filesystem path, but it doesn't have access to the Linux filesystem.

That I'm afraid you will have to sort out yourself :-)

@MisterDA
Copy link

MisterDA commented Nov 18, 2021

On Windows, I cannot set the browser to use, through the --browser flag or the BROWSER env var. I've tried both an Unix shell in Cygwin and the cmd.

$ odig doc --browser='C:\Program Files\Mozilla Firefox\firefox.exe' b0
odig.exe: [ERROR] show uri file://C:\Users\antonin\.opam\ocaml-variants.4.13.1+mingw64c\var\cache\odig\html\b0\index.html: No browser found. Use the BROWSER env var to set one.
$ odig doc --browser='C:/Program Files/Mozilla Firefox/firefox.exe' b0
odig.exe: [ERROR] show uri file://C:\Users\antonin\.opam\ocaml-variants.4.13.1+mingw64c\var\cache\odig\html\b0\index.html: No browser found. Use the BROWSER env var to set one.
$ odig doc --browser='/cygdrive/c/Program Files/Mozilla Firefox/firefox.exe' b0
odig.exe: [ERROR] show uri file://C:\Users\antonin\.opam\ocaml-variants.4.13.1+mingw64c\var\cache\odig\html\b0\index.html: No browser found. Use the BROWSER env var to set one.

Note that start is a cmd builtin, so you won't wind a start.exe. It could be invoked through Sys.command.

cmd /S /C start file://C:\Users\antonin\.opam\ocaml-variants.4.13.1+mingw64c\var\cache\odig\html\b0\index.html

and so this works:

odig doc --browser='cmd.exe /S /C start' b0

@dbuenzli
Copy link
Contributor

@MisterDA

  1. Maybe there is a bug in Os.Cmd.find. Could you please try to lookup these strings in the toplevel as mentioned in this this message.

  2. Do you think the following logic is good: Default to cmd.exe /S /C start if Sys.win32 is true ? (What are the /S and /C for ?)

@MisterDA
Copy link

MisterDA commented Nov 18, 2021

  1. Maybe there is a bug in Os.Cmd.find. Could you please try to lookup these strings in the toplevel as mentioned in this this message.
utop # #require "b0.b00.kit";;
utop # open B00_std;;

utop # Os.Cmd.find ((Cmd.of_string {|C:\Program Files\Mozilla Firefox\firefox.exe|}) |> Result.to_failure);;
- : (Cmd.t option, string) result = Ok None

utop # Os.Cmd.find ((Cmd.of_string {|C:/Program Files/Mozilla Firefox/firefox.exe|}) |> Result.to_failure);;
- : (Cmd.t option, string) result = Ok None

utop # Os.Cmd.find ((Cmd.of_string {|/cygdrive/c/Program Files/Mozilla Firefox/firefox.exe|}) |> Result.to_failure);;
- : (Cmd.t option, string) result = Ok None
  1. Do you think the following logic is good: Default to cmd.exe /S /C start if Sys.win32 is true ? (What are the /S and /C for ?)

Yes, I think it is good. start will use the user-defined association between URL and application. You could also call Unix.system.

if Sys.win32 then
  Unix.system({|start file://C:\Users\antonin\.opam\ocaml-variants.4.13.1+mingw64c\var\cache\odig\html\tar\index.html|})
else ...

/C is like sh -c, it executes the command given as parameter. /S strips the command of quotes, the flag is not needed here. I used it out of habit. Unofficial manual.

@dbuenzli
Copy link
Contributor

@MisterDA thanks would mind having a look what in this logic makes Os.Cmd.find return None. Normally it should end up in this branch.

In particular what does Unix.access file [Unix.X_OK] say on these files ?

You could also call Unix.system.

There's too much unreliability and quoting complexity in using Unix.system, I prefer to know exactly which program I invoke with which data.

@MisterDA
Copy link

In particular what does Unix.access file [Unix.X_OK] say on these files ?

It's actually documented!

On Windows: execute permission X_OK cannot be tested, just tests for read permission instead.

I'm running these examples in Zsh in Cygwin, with opam correctly set up.

utop # Unix.access {|C:\Program Files\Mozilla Firefox\firefox.exe|} [Unix.X_OK];;
- : unit = ()

utop # Unix.access {|C:/Program Files/Mozilla Firefox/firefox.exe|} [Unix.X_OK];;
- : unit = ()

utop # Unix.access {|/cygdrive/c/Program Files/Mozilla Firefox/firefox.exe|}  [Unix.X_OK];;
Exception:
Unix.Unix_error(Unix.ENOENT, "access", "/cygdrive/c/Program Files/Mozilla Firefox/firefox.exe")

Accessing using a Cygwin path raises.
Then, I don't quite follow the logic, but I'll retry next week.

@dbuenzli
Copy link
Contributor

It's actually documented!

Indeed so it should work. @MisterDA did you have time to investigate this further ?

I'd be interested in providing a good Windows default on the line of 2. above but I'd also like to fix 1. I reread the code and couldn't get what goes wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants