swcarpentry / windows-installer

Software Carpentry installer for Windows.
MIT License
21 stars 17 forks source link

R not being added to path on some systems #7

Closed ethanwhite closed 10 years ago

ethanwhite commented 10 years ago

Running the installer on a clean system with newly installed R (following the defaults) and Git Bash (following the defaults) fails to add R to the path.

R executables are available at C:\Program Files\R\R-3.1.0\bin and C:\Program Files\R\R-3.1.0\bin\i386 and C:\Program Files\R\R-3.1.0\bin\x64. I guess the installer is automatically installing 32 and 64 bit versions.

wking commented 10 years ago

On Wed, Jul 09, 2014 at 12:39:41AM -0700, Ethan White wrote something like:

R executables are available at: C:\Program Files\R\R-3.1.0\bin and C:\Program Files\R\R-3.1.0\bin\i386 and C:\Program Files\R\R-3.1.0\bin\x64

So what needs to be in the PATH? …\bin\x64 and …\bin? Or are the presumably architecture-independent executables in …\bin also in the architecture-specific …\bin\x64?

Since 58b7183 (swc-windows-installer.py: Look for an R bin directory, 2014-06-09) and the fixups (e.g. _os → os) in 5d91596 (swc-windows-installer.py: Add logging, 2014-06-09), I think we should be finding and adding …\bin to the PATH. Can you confirm that that is working? The …\bin path should should show up in ~/.bash_profile.

ethanwhite commented 10 years ago

None of these paths is showing up in ~/.bash_profile. I've confirmed the same result on both a Windows 7 and Windows 8 machine.

Running the version in ...\bin works fine so I think keeping that as the target is fine, we just need to figure out why it's not detecting the path. I'll post debug output from the Windows 7 machine in a second.

ethanwhite commented 10 years ago
$ python swc-windows-installer.py  --verbose debug
Preparing your Software Carpentry awesomeness!
create nosetests entrypoint c:/Users/croryx\.swc\bin\nosetests
existing installation at c:/Users/croryx\.swc\lib\nano
existing installation at c:/Users/croryx\.swc\share\nanorc
existing installation at c:/Users/croryx\.swc\lib\sqlite
no R installation found under C:\Program Files (x86)
update bash profile at c:/Users/croryx\.bash_profile
extra paths:
* c:/Users/croryx\.swc\lib\nano
* c:/Users/croryx\.swc\lib\sqlite
* c:/Users/croryx\.swc\bin
wking commented 10 years ago

On Wed, Jul 09, 2014 at 10:40:44AM -0700, Ethan White wrote:

no R installation found under C:\Program Files (x86)

Hmm, where is that x86 coming from? And why is R installing i386 binaries under the 64-bit ‘C:\Program Files’? I'd expected the ‘ProgramFiles’ environment variable would expand to ‘C:\Program Files’ 1, not ‘C:\Program Files (x86)’ (which should be referenced by the ‘ProgramFiles(x86)’ environment variable).

Incidentally, it looks like my default for when ‘ProgramFiles’ isn't set is missing a space. I'll fix that once we figure out what's wrong with the current logic.

ethanwhite commented 10 years ago

Line 200 is returning C:\\Program Files (x86) on my Windows 7 machine as you'd inferred. The same line but with ProgramFiles(x86) replacing both instances of ProgramFiles returns exactly the same thing.

wking commented 10 years ago

On Wed, Jul 09, 2014 at 11:31:32AM -0700, W. Trevor King wrote:

Hmm, where is that x86 coming from? And why is R installing i386 binaries under the 64-bit ‘C:\Program Files’? I'd expected the ‘ProgramFiles’ environment variable would expand to ‘C:\Program Files’ 1, not ‘C:\Program Files (x86)’ (which should be referenced by the ‘ProgramFiles(x86)’ environment variable).

I can't find this mentioned in the Microsoft docs 1, but Wikipedia suggests ‘ProgramW6432’ for ‘C:\Program Files’ and claims ‘ProgramFiles’ expansion depends on the architecture for the expanding program 2. This seems to be supported by this Microsoft page 3. Do we have to worry about folks with purely-32-bit Windows installs? I'd be surprised if they had ProgramW6432 set at all. Are there any purely-64-bit Windows installs? They might not set ProgramW6432 either.

wking commented 10 years ago

On Wed, Jul 09, 2014 at 11:53:25AM -0700, W. Trevor King wrote:

Do we have to worry about folks with purely-32-bit Windows installs? I'd be surprised if they had ProgramW6432 set at all. Are there any purely-64-bit Windows installs? They might not set ProgramW6432 either.

I'm not sure about which category this falls under, but a Alfred Neumann comments at the end of 1:

ProgramFilesW6432 & CommonProgramFilesW6432 not available under Windows XP/2003/Vista/2008 See MSKB article 976039!

I imagine he meant ProgramW6432 and CommonProgramW6432. The referenced article seems to be 2, but I'm not fluent enough with Windows to understand that bug report.

wking commented 10 years ago

On Wed, Jul 09, 2014 at 11:50:17AM -0700, Ethan White wrote:

Line 200 is returning C:\\Program Files (x86) on my Windows 7 machine as you'd inferred. The same line but with ProgramFiles(x86) replacing both instances of ProgramFiles returns exactly the same thing.

Try ProgramW6432? And then try it on a number of Windows systems at various patch levels? ;). Alternatively, use a 64-bit Python ;). Or explicitly override the variable when launching a 32-bit Python? I don't know what the OS uses these for, so I'm not sure if such an override has negative side effects or not.

ethanwhite commented 10 years ago

Yes, we probably need to worry about 32-bit Windows installs still (this is probably the difference that explains why one of the machines I tested on works).

Neither ProgramFilesW6432 or CommonProgramFilesW6432 exist on the failing system I'm testing on at the moment.

Could we:

  1. Just check both 'C:\Program Files' and 'C:\Program Files (x86)' or
  2. Try using [1], which was recommended when I asked about getting R in the path on twitter

[1]

ethanwhite commented 10 years ago

[1] http://cran.r-project.org/bin/windows/Rtools/installer.html

wking commented 10 years ago

On Wed, Jul 09, 2014 at 12:07:14PM -0700, Ethan White wrote:

Neither ProgramFilesW6432 or CommonProgramFilesW6432 exist on the failing system I'm testing on at the moment.

I think the variable name should be ‘ProgramW6432’.

Could we:

  1. Just check both 'C:\Program Files' and 'C:\Program Files (x86)' or

I don't know what the environment variables are for, and I'm jumpy hard-coding something that's supposed to be transmitted via environment variable. I agree that it's probably unlikely to have a system that has a different value, but sticking to the environment variables is likely to be more future-proof (for 128-bit systems ;).

  1. Try using [1], which was recommended when I asked about getting R in the path on twitter

Hmm, so install Rtools under ~/.swc/bin (or ~/.swc/lib/rtools/) and use that? That sounds better to me than the first approach, although if we can figure out the appropriate environment variable, I'd prefer that.

ethanwhite commented 10 years ago

I think the variable name should be ‘ProgramW6432’.

Yep, you're right [1], and it works. Sorry I missed that (too much email flying around today).

Go ahead and make the change, and the other clean up that you wanted to get in.

[1] http://stackoverflow.com/a/1283667/133513

wking commented 10 years ago

On Wed, Jul 09, 2014 at 02:47:48PM -0700, Ethan White wrote:

I think the variable name should be ‘ProgramW6432’.

Yep, you're right [1], and it works.

Does #11 work then? Having access to (or helpers with) an assortment of Windows systems to test this sort of thing would be nice. Do R installs come in separate 32- and 64-bit flavors? Or is it a single package that installs both flavors (regardless of the underlying OS architecture?)?

ethanwhite commented 10 years ago

Does #11 work then?

Yep, works perfectly.

Having access to (or helpers with) an assortment of Windows systems to test this sort of thing would be nice.

Yes it would, and we do, it's just that the testing happens in production :). I think the best way to do is probably to release a new version with this feature and ask for the next person teaching an R workshop and running R from the shell to report back.

Do R installs come in separate 32- and 64-bit flavors? Or is it a single package that installs both flavors (regardless of the underlying OS architecture?)?

It's a single package. I can't remember if it gives you the option to choose one or just installs both.

wking commented 10 years ago

On Thu, Jul 10, 2014 at 11:34:36AM -0700, Ethan White wrote:

I think the best way to do is probably to release a new version with this feature and ask for the next person teaching an R workshop and running R from the shell to report back.

How do we identify that person?

ethanwhite commented 10 years ago

How do we identify that person?

Probably email the Discuss list, ask for testing in general and specifically request that the next person who runs an appropriately configured workshop provide feedback.

wking commented 10 years ago

Fixed by #11.

wking commented 10 years ago

On Thu, Jul 10, 2014 at 11:38:17AM -0700, Ethan White wrote:

How do we identify that person?

Probably email the Discuss list, ask for testing in general and specifically request that the next person who runs an appropriately configured workshop provide feedback.

Sounds good. I expect we should send the email in the run-up to cutting a release (e.g. #12) for those interested in testing the Python script, and then again after we tag and build the Inno installer (after which we'd have to cut a new release, since we'd have already tagged the master commit and pushed associated installer to our releases). If you agree that now seems like a good time for a v0.1 release, I'll send the initial “test the Python script” message to discuss@.

ethanwhite commented 10 years ago

On Thu, Jul 10, 2014 at 1:22 PM, W. Trevor King notifications@github.com wrote:

Sounds good. I expect we should send the email in the run-up to cutting a release (e.g. #12) for those interested in testing the Python script, and then again after we tag and build the Inno installer (after which we'd have to cut a new release, since we'd have already tagged the master commit and pushed associated installer to our releases). If you agree that now seems like a good time for a v0.1 release, I'll send the initial “test the Python script” message to discuss@.

Sounds good.