swc-windows-installer.py: Allow R-3.2.4revised\bin

swcarpentry / windows-installer

Software Carpentry installer for Windows.

MIT License

21 stars 17 forks source link

swc-windows-installer.py: Allow R-3.2.4revised\bin #46

Closed wking closed 8 years ago

wking commented 8 years ago

On Thu, Apr 14, 2016 at 12:41:01PM -0700, Ethan White wrote:

At a workshop I just ran we ran into issues with R not being in the path on a number of Windows computers. In the past this has been caused because R likes to change the path where it installs itself and we've needed to change the function that searches for R. The new path that is causing issues is:

C:\Program Files\R\R-3.2.4revised\bin

Fixes #44.

embray commented 8 years ago

Is it truly necessary to be even this specific / strict in regexp? Why not just R-.*/bin? Note: I know next to nothing about R and don't know if there are other paths we don't want that could conceivably match that regexp.

wking commented 8 years ago

On Fri, Apr 15, 2016 at 05:23:07AM -0700, Erik Bray wrote:

Is it truly necessary to be even this specific / strict in regexp? Why not just R-.*/bin? Note: I know next to nothing about R and don't know if there are other paths we don't want that could conceivably match that regexp.

I'm in the same boat on R. A more relaxed regexp would probably be fine, but staying stricter means:

We're less likely to pull in the wrong thing, although that's unlikely since we require one path component that's just ‘R’.
We can add version comparisons (since all currently-reported paths contain version information) if we want to pick the newest R, or some such.

The downside is that we sometimes hit situations like #44. This is the first bump to that regexp since it landed in 58b7183ea (swc-windows-installer.py: Look for an R bin directory, 2014-06-09), so I don't think this is a high risk.

embray commented 8 years ago

Yeah, looking again at your new regexp, it seems pretty unlikely to fail again, and pulling out the version number could be useful :)

embray commented 8 years ago

@wking Do you have merge access on this repo?

wking commented 8 years ago

On Mon, Jun 13, 2016 at 11:31:16AM -0700, Erik Bray wrote:

@wking Do you have merge access on this repo?

I do, but I don't like to merge my own PRs ;). If you don't, we should get it for you too.

embray commented 8 years ago

I do, but I don't like to merge my own PRs ;).

Yep. same.

If you don't, we should get it for you too.

Please do!

wking commented 8 years ago

On Mon, Jun 13, 2016 at 12:58:59PM -0700, Erik Bray wrote:

If you don't, we should get it for you too.

Please do!

@gvwilson, or other SWC admin: can we get write access to this repo for @embray?

gvwilson commented 8 years ago

Done!