Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use rtools40 for R >= 4.0 #85

Merged
merged 1 commit into from Apr 19, 2020
Merged

Use rtools40 for R >= 4.0 #85

merged 1 commit into from Apr 19, 2020

Conversation

jeroen
Copy link
Member

@jeroen jeroen commented Apr 19, 2020

Match the default CRAN toolchain, which now uses rtools40 for R 4.0 and R-devel.

The logic is that if r-version starts with 3 (for example 3.6 or 3.x) then the default is rtools35. Otherwise (for example 4.0 or devel) we need to install rtools40.

We can tweak this more later, but for now this will suffice to start testing with R-devel (and soon R-4.0).

@codecov-io
Copy link

@codecov-io codecov-io commented Apr 19, 2020

Codecov Report

Merging #85 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #85   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            1         1           
=========================================
  Hits             1         1           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3168e24...eea59e7. Read the comment docs.

@jimhester
Copy link
Member

@jimhester jimhester commented Apr 19, 2020

I tested this out, it seems to be working fine, though I do get a warning from R CMD check --as-cran. The test package doesn't have a package Makevars or this flag in the user Makevars, so it must be added by the toolchain.

Compilation used the following non-portable flag(s):
'-mstackrealign'

Hopefully we can get R CMD check to whitelist this flag before R 4.0 is released if it is necessary for Rtools40?

full logs

Anyway the changes seem fine, thanks for sending the PR!

@jimhester jimhester merged commit 4d14c61 into r-lib:master Apr 19, 2020
13 of 16 checks passed
lcolladotor added a commit to LieberInstitute/recount3 that referenced this issue Apr 19, 2020
…ed to guess & specify the rtools version
lcolladotor added a commit to LieberInstitute/recount3 that referenced this issue Apr 19, 2020
@lcolladotor
Copy link
Contributor

@lcolladotor lcolladotor commented Apr 19, 2020

Hi @jeroen,

Thanks for doing this! From looking at eea59e7#diff-78c52b4ebcf50fd32bb306df7f1a5b7cR10 it seems to me that if the R version is specified as a string (release or devel) and no rtools-version is specified, then the current logic would fail, right?

    let version = core.getInput("r-version");
    core.debug(`got version ${version}`);
    let rtoolsVersion = core.getInput("rtools-version") || (version.charAt(0) == '3' ? '35' : '40');

Best,
Leo

@jeroen
Copy link
Member Author

@jeroen jeroen commented Apr 19, 2020

Why would it fail? Version devel does not start with "3" so it uses rtools40. And we do not support other aliases right now afaict.

@lcolladotor
Copy link
Contributor

@lcolladotor lcolladotor commented Apr 19, 2020

You are right! But it would fail for release right now, no? (It won't soon once r-release points to R 4.0)

@jeroen
Copy link
Member Author

@jeroen jeroen commented Apr 19, 2020

I don't think such aliases are supported: #26 (comment)

lcolladotor added a commit to LieberInstitute/recount3 that referenced this issue Apr 19, 2020
…tools-version number r-lib/actions#85 (comment). Also the code I wrote never specifies 'release' as the r-version. See also r-lib/actions#26 (comment) that Jeroen linked to.
@lcolladotor
Copy link
Contributor

@lcolladotor lcolladotor commented Apr 19, 2020

Awesome, thanks for helping me understand this!

Best,
Leo

lcolladotor added a commit to lcolladotor/biocthis that referenced this issue May 9, 2020
Created both an introductory and a developer's notes vignette, updated README
and docs with examples, added a second biocViews term, fixed some small
bugs/typos.

Related links (as many as I could remember):

* https://rstd.io/tidytools19
* https://twitter.com/CVWickham
* https://twitter.com/hadleywickham
* https://www.rstudio.com/products/rstudio/download
* https://comunidadbioinfo.github.io/post/building-tidy-tools-cdsb-runconf-2019/#.XrbLMxNKiu4
* http://bioconductor.org/
* https://lcolladotor.github.io/pkgs/
* https://stat.ethz.ch/pipermail/bioc-devel/2020-March/016365.html
* https://www.bioconductor.org/help/docker/
* https://stat.ethz.ch/pipermail/bioc-devel/2020-April/016532.html
* https://github.com/features/actions
* https://stat.ethz.ch/pipermail/bioc-devel/2020-April/016650.html
* r-lib/actions#84
* r-lib/usethis#1108
* r-lib/styler#636
* Bioconductor/BiocCheck#57
* Bioconductor/bioconductor.org#54
* http://bioconductor.org/developers/how-to/coding-style/
* https://style.tidyverse.org/
* https://twitter.com/lorenzwalthert
* https://twitter.com/mt_morgan
* https://docs.travis-ci.com/user/languages/r/
* r-lib/pkgdown#1206
* r-lib/pkgdown#1230
* https://twitter.com/jimhester_
* https://www.jimhester.com/talk/2020-rsc-github-actions/
* https://github.com/Bioconductor/BBS
* https://github.com/Bioconductor/packagebuilder
* https://www.appveyor.com/
* r-hub/rhub#52
* r-hub/rhub#38
* https://www.tidyverse.org/blog/2020/04/usethis-1-6-0/
* https://github.com/r-lib/actions/tree/master/examples
* https://yihui.org/en/2018/03/second-pull-request/
* https://github.com/r-lib/actions/blob/master/examples/check-standard.yaml
* https://help.github.com/en/actions
* https://ropenscilabs.github.io/actions_sandbox/
* https://twitter.com/seandavis12
* https://github.com/seandavi/BiocActions/blob/master/.github/workflows/main.yml
* https://twitter.com/CSoneson
* https://github.com/csoneson/dreval/blob/master/.github/workflows/R-CMD-check.yaml
* https://bioc-community.herokuapp.com/
* https://github.com/leekgroup/derfinderPlot/blob/master/.github/workflows/check-bioc.yml
* https://github.com/LieberInstitute/recount3/blob/master/.github/workflows/check-bioc.yml
* https://github.com/hpages
* r-lib/actions#68
* r-lib/actions#85
* https://twitter.com/opencpu
* https://community.rstudio.com/u/const-ae
* https://community.rstudio.com/t/compiler-support-fo-c-14-features-on-windows/57284/4
* r-lib/xml2#296
* r-lib/xml2#302
* https://github.com/r-lib/usethis/blob/master/.github/workflows/R-CMD-check.yaml
* https://github.com/r-lib/usethis/commits/master/.github/workflows/R-CMD-check.yaml
* https://stat.ethz.ch/pipermail/bioc-devel/2020-April/016703.html
* https://stat.ethz.ch/pipermail/bioc-devel/2020-April/thread.html
* r-lib/remotes#296
* r-lib/actions#86
* r-lib/covr#427
* https://github.com/r-lib/actions/blob/master/examples/pr-commands.yaml
* https://www.digitalocean.com/community/tutorials/how-to-install-git-on-ubuntu-18-04
* r-lib/actions#50
* actions/checkout#238
* https://github.com/rocker-org/rocker-versioned2/blob/master/dockerfiles/Dockerfile_rstudio_4.0.0-ubuntu18.04
* https://twitter.com/niteshturaga
* https://twitter.com/cboettig
* rocker-org/rocker-versioned#208
* https://github.community/t5/GitHub-Actions/bd-p/actions
* https://www.r-consortium.org/blog/2020/03/18/cdsb-diversity-and-outreach-hotspot-in-mexico
* https://github.com/maxheld83
* r-lib/actions#87
* https://github.com/yutannihilation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants