rstudio / tinytex-releases

Windows/macOS/Linux binaries and installation methods of TinyTeX
https://yihui.org/tinytex/
GNU General Public License v2.0
263 stars 29 forks source link

Use valid version #15

Closed fkohrt closed 3 years ago

fkohrt commented 3 years ago

I could not find 2020.10 (without v) in the list of GitHub releases.

cderv commented 3 years ago

Are we sure about this ?

The code of tinytex takes care of inserting the v for you normally https://github.com/yihui/tinytex/blob/19526a859e388a07db23954ad14a2d7f666f5ed2/R/install.R#L341-L344

With current proposal merged above, the wrong URL is built. See the first line of the error output I get

> tinytex::install_tinytex(version = "v2020.10")
trying URL 'https://github.com/yihui/tinytex-releases/releases/download/vv2020.10/TinyTeX-1-vv2020.10.zip'

The url as a duplicated v in the path and in the bundle name bundle name, which is not correct.

However, as initially documented, this works

tinytex::install_tinytex(version = "2020.10")

What was exactly the initial issue for you @fkohrt ?

fkohrt commented 3 years ago

With current proposal merged above, the wrong URL is built.

Oh no!

What was exactly the initial issue for you @fkohrt ?

I was initially confused because running xfun::github_releases("yihui/tinytex-releases") (as suggested by ?tinytex::install_tinytex) did not include 2020.10, but only v2020.10. So I thought the entry in the README.md is just a typo.

Given that it does not work at all I believe the wording the the documentation should be changed, right?

cderv commented 3 years ago

Oh I see how this could be confusing !

Maybe we should just detect the v if it is pass to intall_tinytex(), and remove it. This would allow to directly pass the result of one of the xfun::github_releases("yihui/tinytex-releases") version to install_tinytex().

Would that be less confusing ?

For reference, ?install_tinytex mention:

version
The version of TinyTeX, e.g., "2020.09" (see all available versions at https://github.com/yihui/tinytex-releases, or the last few releases via xfun::github_releases('yihui/tinytex-releases')). By default, it installs the latest daily build of TinyTeX. If version = 'latest', it installs the latest Github release of TinyTeX.

So I see how, even is the example for version is ok, this could be confusing.

fkohrt commented 3 years ago

As of today, I get the following:

> xfun::github_releases('yihui/tinytex-releases')
 [1] "v2021.07" "v2021.06" "v2021.05" "v2021.04" "v2021.03"
 [6] "v2021.02" "v2021.01" "v2020.12" "v2020.11" "v2020.10

So all of these should be valid values for the version parameter. But as you wrote above, running tinytex::install_tinytex(version = "v2020.10") leads to an error. So either the documentation or the current version detector code are wrong, aren't they?

cderv commented 3 years ago

I have made https://github.com/yihui/tinytex/pull/318 to make tinytex::install_tinytex(version = "v2020.10") works.

This means that any results of xfun::github_releases('yihui/tinytex-releases') would work as an input for install_tinytex()

This would take into account your feedback, right ?

fkohrt commented 3 years ago

Yes, that's perfect!

cderv commented 3 years ago

@yihui I let you merge the other PR if it is good to you.

We can left the README here as is as it will work after this, or revert this change. Up to you.

yihui commented 3 years ago

@cderv Great. Thanks!

(I'll revert this change.)