Closed eitsupi closed 1 year ago
Hi! An interesting idea, though {rextendr}
here only provides templates -- it is OK to modify it to suit your needs. However, this is the first time I see such an issue. @yutannihilation, what do you think? We can provide a config option in rextendr::use_extendr()
for this particular case.
My issue here is how does CRAN
treat this?
Thanks for exploring, but I'm afraid tweaking a user's toolchain in Makevars
is not a good manner that we can spread. I think this is fine for R-universe as it's a separated environment.
It's really surprising that CRAN accepted this... Probably because there are already a toolchain installed on the CRAN machines?
Thinking this again, I feel these lines are unnecessary. I'm asking here.
The motivation for this was mostly to make the package build on a default GitHub actions environment. I think it is important that there are no special workflows needed to build a rust package.
For example, suppose a 3rd party package uses remotes: r-rust/hellorust
inside a GHA workflow that doesn't have special rust setup, it still works because the package automatically uses the correct target.
On R-universe we could try to automatically install rustc targets, but that still won't solve the problem in general.
I think it is harmless to have this on CRAN, because the line is a no-op if the toolchain is already installed. But if you want to be really careful I suppose you can comment out that line for a CRAN submission.
The question is should it be part of the makefile? We do not install RTools as part of some package installation script, and RTools is basically the same thing -- a set of external compilers that are needed for the compilation of packages from source and are not necessarily required for using R on its own. So while we have a separate config in the R installation github actions, why not do the same and configure rust?
The motivation for this was mostly to make the package build on a default GitHub actions environment. I think it is important that there are no special workflows needed to build a rust package.
I'm now convinced this is a fair reason to invoke rustup
in Makevars
.
However, I still think rustup
is too powerful to use in the installation scripts. For example, rustup target add
is probably fine, but what about rustup default
? You are a super-expert who won't make mistakes, but those who copy and paste your code might not.
It's not that you or we are wrong and the other is not, but there's a trade-off. So, I have no idea whether we should choose this or not.
It seems hellorust will soon work without rustup
as the toolchain will be pre-installed!
c.f. https://github.com/r-universe-org/help/issues/219#issuecomment-1332139457
This made me wonder if extendr should once again come back to the msvc-toolchain. While the cross-compilation is of so much pain, in reality, it seems we cannot expect the gnu-toolchain is easily available. This won't be a very problem on GHA, but if we seriously target CRAN, we might need to consider it (I'm not sure which toolchain is available on CRAN machines, btw).
Is it possible even to use the msvc toolchain with R? I thought we really needed the gnu toolchains?
I don't think I will change hellorust. It is nice that the compilers we need are preinstalled on GHA and CRAN now, but I think leaving the rustup add
call in there is still harmless.
Is it possible even to use the msvc toolchain with R? I thought we really needed the gnu toolchains?
Honestly, I'm poor at the compiler things so I might be wrong, but we did use MSVC toolchain on R 4.1 without any visible problem except for that testing is a bit complicated. Do you have any example of possible problems of using MSVC toolchain in your mind?
I don't think I will change hellorust. It is nice that the compilers we need are preinstalled on GHA and CRAN now, but I think leaving the
rustup add
call in there is still harmless.
Yeah, I agree it's harmless.
Is it possible even to use the msvc toolchain with R? I thought we really needed the gnu toolchains?
Yes, but there is a trick. Basically, we just need a gnu-compatible output. This can be achieved with cross-compilation. It works fine for us on R < 4.2, also providing support for 32bit version https://github.com/extendr/rextendr/blob/600944597472f2bf34fb7b1e1acbe0ff2536db19/inst/templates/Makevars.win#L4
The trick is you need *-pc-windows-gnu
targets installed for default *-x86_64-pc-windows-msvc
toolchain and to select appropriate targets when compiling libraries.
In the end, there is no bypass -- you either compile from gnu
toolchain to its native gnu
target, or you cross-compile from msvc
toolchain to the same gnu
target.
As tested in r-universe-org/help#219 and eitsupi/prqlr#22, perhaps there is no longer a need to install toolchain? It seems that the special GitHub Actions definition as proposed in #60 is also unnecessary.
Hm, we spent a lot of time figuring out how to best compile things on Windows. Rolling back to msvc
will take some time, for sure. I guess I need to investigate.
we spent a lot of time figuring out how to best compile things on Windows
But, honestly, I think we had little discussion over the decision (c.f. https://github.com/extendr/extendr/issues/396). It's true that we had bitter experiences with the MSVC toolchain, especially the things related to cross-compilation. However, it's unclear how the GNU toolchain is better than that (again, except for cross-compilation).
Let's revisit the histories. I think I forgot many things...
I think we can close this issue as we don't need to install it at least on CRAN and on R-universe. On other places, it might be a good workaround in some cases, but I think we don't need to include it in rextendr. Thanks so much for exploring, @eitsupi and @jeroen!
I filed a new issue about using MSVC toolchain. If you have any comments, please join: https://github.com/extendr/extendr/issues/437
Ah, no. Sorry, I was a bit confused. We can close this issue only when extendr decided to migrate to MSVC toolchain. But, I keep this closed because the MSVC toolchain is confirmed to work at least this simple case. Let's revisit here when we find we can't live with the MSVC toolchain.
Hello. I am wondering if it would be better to set up toolchain on Makevars.win, like the
hellorust
package recently registered on CRAN. https://github.com/r-rust/hellorust/blob/8902d6677d70d91b7336a90ba3d8d41f4a9011cd/src/Makevars.winI recently created a package using rextendr, but needed to edit Makevars.win a bit to get a successful build on R-universe. https://github.com/eitsupi/prqlr/blob/d4f01908e639ea4e9f1d336e014b02125f8c66b0/src/Makevars.win#L11-L35