cderv / r-bucket

Personal scoop bucket I use as an R user and software engineer
49 stars 3 forks source link

tinytex scoop installation is not found correctly by tinytex R package #3

Closed cderv closed 3 years ago

cderv commented 3 years ago

This is due to the way scoop is creating shims that are not symlinks. They are special files that will execute the correct one.

For example, scoop create this shims for tlmgr in the tlmgr.cmd

@"C:\Users\chris\scoop\apps\tinytex\current\bin\win32\tlmgr.bat"  %*

But it is not a symlink, so tintytex::tinytex_root mechanism does not work...

We could either not create the shims and do otherwise, or change how tinytex works for supporting scoop. 🤔

cderv commented 3 years ago

Ok issue is only with tinytex_root() functions but that is not useful for other thing that TinyTeX R installation

yihui commented 3 years ago

What does Sys.which('tlmgr') return?

cderv commented 3 years ago

The full path to the shims :

They are the shims that do the magic but they are not symlink. The path to the binary is inside those files I think. For scoop at least.

yihui commented 3 years ago

Since you have run tlmgr path add:

https://github.com/cderv/r-bucket/blob/ff31c2624d01ca187cc53d125aa609545233ee96/bucket/TinyTex.json#L32

there is actually no need to create the shim:

https://github.com/cderv/r-bucket/blob/ff31c2624d01ca187cc53d125aa609545233ee96/bucket/TinyTex.json#L10-L15

Without the shim, tinytex_root() should work.

naveen521kk commented 3 years ago

You can very well remove shims for the scoop. Se what I explained https://github.com/yihui/tinytex-releases/pull/5#pullrequestreview-492005194 but scoop does it differently and not required.

cderv commented 3 years ago

I have removed the shims in 0d857e657002bb0734f1fb2c9e709ae87e42ca5d and it works now. I tested before but I was misleaded because environment variable are not refreshed by default in POWERSHELL terminal, and shim was overriding the path added to PATH. It seems it is working now.

naveen521kk commented 3 years ago

@cderv Can you help me out? Why do you need to remove shims? What does tinytex_root() do? Where is the source if I could read it? Why can't it handle with shims? I think Sys.which('tlmgr') is equivalent to where command in cmd right? If yes, you can query for tlmgr.bat, Sys.which('tlmgr.bat') which will return your necessary location. Possibly, having shims looks necessary for me. Did that by any chance help?

cderv commented 3 years ago

Where is the source if I could read it?

tinytex_root is from tinytex 📦 : https://github.com/yihui/tinytex/blob/cd1f01ae5d7e621ced383f184ccd7e778549d1a9/R/install.R#L242

What does tinytex_root() do?

The aim is to find the tinytex installation root folder. Usually two folders before where tlmgr binary found from PATH is.

Why do you need to remove shims?

tlmgr path add is indeed adding tmlgr to the PATH already as @yihui suggested. So I don't think a shim is required here anymore. It would be adding twice the same thing

If yes, you can query for tlmgr.bat, Sys.which('tlmgr.bat') which will return your necessary location.

Yes it is correct, it will find the bat file in the correct folder, because tlmgr add path correctly added the bin/win32 folder to the PATH. Sys.which("tlmgr") will return the shim if we have one, or directly tlmgr.bat if no shim because the PATH is already correct.

Possibly, having shims looks necessary for me.

Tex Live already handles the PATH thing, so I don't think that choco or scoop should also handle it with there shim folders on PATH too.

cderv commented 3 years ago

And @yihui why did you need tweak_path() for in tinytex ? Was tlmgr add path not enough In some case ?

yihui commented 3 years ago

And @yihui why did you need tweak_path() for in tinytex ? Was tlmgr add path not enough In some case ?

Because tlmgr path add may fail in certain cases (e.g. /usr/local/bin is not writable). It should be unlikely to fail on Windows, though.

Tex Live already handles the PATH thing, so I don't think that choco or scoop should also handle it with there shim folders on PATH too.

That's right. If we were to create shims at all, we'd better create them for all executables under bin/win32. Either no shims, or all, but not a subset of them.

Anyway, what does Sys.which('pdflatex') return for the choco and scoop installations? I can probably tweak tinytex_root() to work regardless of the existence of shims.

naveen521kk commented 3 years ago

Anyway, what does Sys.which('pdflatex') return for the choco and scoop installations? I can probably tweak tinytex_root() to work regardless of the existence of shims.

You can try the same out with Sys.which('tlmgr.bat').

cderv commented 3 years ago

Anyway, what does Sys.which('pdflatex') return for the choco and scoop installations? I can probably tweak tinytex_root() to work regardless of the existence of shims.

If we keep the shim, we got correct pdflatex because I did not created a shim for it

> Sys.which('tlmgr')
                                      tlmgr 
"C:\\Users\\chris\\scoop\\shims\\tlmgr.cmd" 
> Sys.which('pdflatex')
                                                                   pdflatex 
"C:\\Users\\chris\\scoop\\apps\\tinytex\\current\\bin\\win32\\pdflatex.exe" 

Either no shims, or all, but not a subset of them.

I removed the shim creation, so nothing else to do IMO.

yihui commented 3 years ago

Okay. I think this issue can be closed now.

You can try the same out with Sys.which('tlmgr.bat').

That's a good idea. Thanks!

cderv commented 3 years ago

Okay. I think this issue can be closed now.

Yep it can !