find_hugo all does not find all version on Windows

rstudio / blogdown

Create Blogs and Websites with R Markdown

https://pkgs.rstudio.com/blogdown/

1.73k stars 334 forks source link

find_hugo all does not find all version on Windows #503

Closed cderv closed 3 years ago

cderv commented 3 years ago

FYI @yihui

Only a scoop version installed

blogdown::find_hugo()
#> [1] "hugo.exe"
blogdown::find_hugo('all')
#> NULL
Sys.which("hugo")
#>                                       hugo 
#> "C:\\Users\\chris\\scoop\\shims\\hugo.exe"

With APPDAPA version

blogdown::find_hugo()
#> Warning: Found hugo at "C:\Users\chris\AppData\Roaming/Hugo/0.78.0/hugo.exe"
#> and "C:\Users\chris\scoop\shims\hugo.exe". The former will be used. If you
#> don't need both copies, you may delete/uninstall the latterperhaps with
#> system2("scoop", c("uninstall", "hugo")).
#> [1] "C:\\Users\\chris\\AppData\\Roaming\\Hugo\\0.78.0\\hugo.exe"
blogdown::find_hugo("all")
#> [1] "C:\\Users\\chris\\AppData\\Roaming/Hugo/0.78.0/hugo.exe"
Sys.which("hugo")
#>                                       hugo 
#> "C:\\Users\\chris\\scoop\\shims\\hugo.exe"

It feels unexpected that find_hugo() found it and not find_hugo("all")

cderv commented 3 years ago

This works ok now

blogdown::find_hugo()
#> [1] "hugo.exe"
blogdown::find_hugo("all")
#> [1] "C:/Users/chris/scoop/shims/hugo.exe"

^{Created on 2020-11-09 by the reprex package (v0.3.0.9001)}

We just got one full path, and one binary name (found because on PATH). Should we normalize in both case ?

yihui commented 3 years ago

We could normalize. I was mainly concerned about using the full path on Windows (e.g. when it contains spaces), but that might be an imaginary concern. This is probably not a big deal. I'll mention this on the help page.

cderv commented 3 years ago

I was mainly concerned about using the full path on Windows (e.g. when it contains spaces), but that might be an imaginary concern.

Hum, you may be right... Spaces can sometimes cause a lot of issues. We have some with TexLive and TinyTeX: https://community.rstudio.com/search?q=space%20user%20tinytex

Using the binary filename directly from the PATH is safer indeed.

yihui commented 3 years ago

Actually you brought up exactly the problem that I meant to ask you a while ago. I've seen this CreateProcess failed message several times in the past because of the spaces in user names on Windows, but I've never been able to reproduce it. It seems you don't know the reason, either: https://community.rstudio.com/t/60003/6 This is really weird.

cderv commented 3 years ago

Yes I don't know either for sure. And hard to recreate as each time it seems to be related with space in username.

I know that CreateProcess and space can be difficult:

https://stackoverflow.com/questions/4053241/windows-api-createprocess-path-with-space

I wonder if it is just a quoting issue. I was also not sure which program is calling this "CreateProcess".