krlmlr / r-appveyor

Tools for using R with AppVeyor (https://appveyor.com)
132 stars 60 forks source link

Explicitly setting USE_RTOOLS to false causes Rtools to be installed #128

Closed jdblischak closed 5 years ago

jdblischak commented 6 years ago

Observed behavior

I had set USE_RTOOLS: true so that I could build the latest versions of CRAN packages even when a binary version is not available. However, I recently decided that the added build time wasn't worth getting the latest package versions (compiling stringi was approximately doubling the build time). To make it easy to remember how to turn this back on in the future, I explicitly set the default of USE_RTOOLS: false. Unexpectedly, this caused Rtools to be installed.

Expected behavior

I expected setting USE_RTOOLS: false to behave the same as the default when no src/ directory is present.

Examples

krlmlr commented 5 years ago

Thanks. This is the offender:

https://github.com/krlmlr/r-appveyor/blob/545199b4e630febe6af795621cb4de32a33851d4/scripts/appveyor-tool.ps1#L161

The workaround is to just omit USE_RTOOLS, but we should really check for true . Would you like to submit a pull request?

LiNk-NY commented 5 years ago

Would it be something like ($env:USE_RTOOLS -is $true) ? I don't know PowerShell :sweat_smile:

jdblischak commented 5 years ago

Thanks. This is the offender:

I see. So it is checking to see if the environment variable USE_RTOOLS is set, but it could be anything.

The workaround is to just omit USE_RTOOLS,

Agreed. That is what I did to quickly fix the problem.

but we should really check for true . Would you like to submit a pull request?

@krlmlr @LiNk-NY I don't know much PowerShell. Here's my guess for a solution. Happy to submit a PR if this looks ok:

  $rtools= $env:USE_RTOOLS
  if ((Test-Path "src") -or ($rtools -eq "true")) {
    InstallRtools
  }
  Else {
    Progress "Skipping download of Rtools because src/ directory is missing."
  }