Do not specify the toolchain on Windows with R >= 4.2

extendr / rextendr

An R package that helps scaffolding extendr-enabled packages or compiling Rust code dynamically

https://extendr.github.io/rextendr/

Other

181 stars 27 forks source link

Do not specify the toolchain on Windows with R >= 4.2 #219

Closed yutannihilation closed 1 year ago

yutannihilation commented 1 year ago

Add a runner to test MSVC toolchain on R >= 4.2
Remove the toolchain specification (+$(TOOLCHAIN)) from the template

Ilia-Kosenkov commented 1 year ago

~~Also, we need to check what toolchain is used for dynamic compilation with rextendr::rust_source() and other functions,.~~

NVM, it uses system's default https://github.com/extendr/rextendr/blob/f627e943bfb507192b5599d1e331448c80049906/R/zzz.R#L20

Ilia-Kosenkov commented 1 year ago

If this works, I guess we should drop TOOLCHAIN from Windows completely. We arrived to using default toolchains on all platforms, so no need to override it via an additional parameter. If there ever is a need to use different toolchain, we should address it by using rustup default what-we-need

yutannihilation commented 1 year ago

Thanks, sounds a good idea.

yutannihilation commented 1 year ago

Hmm, it seems something is wrong with r-hub today so the CI fail... I'll retry tomorrow.

yutannihilation commented 1 year ago

Yay, I retried and all the CI passed. I think the current CI setting is a bit overkill; we can drop some combinations that are less important (i.e. mainly test with the MSVC toolchain, as it will be our main target, and keep only one case with GNU toolchain). I'll tweak later.

yutannihilation commented 1 year ago

This should be ready for review now.

yutannihilation commented 1 year ago

Thanks! I already use this in https://github.com/extendr/extendr/pull/439