840819 - activate.bat "Warning: Failed to find Python installation directory"

Status: RESOLVED WONTFIX

(Add-on SDK Graveyard :: General, defect, P3)

Description

[Todd Keck]

Attachment: activate.bat

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0
Build ID: 20130201065344

Steps to reproduce:

Python has been up and running for awhile now and is located in C:\Python27 on my Win 8 box. I downloaded the addon-sdk zip, unzipped it and placed it in my documents folder. I then went to the command line and ran bin\activate.bat.


Actual results:

 I was then given the error code. 
"Warning: Failed to find Python installation directory"


Expected results:

It should have activated the addon sdk and added environmental variables. I hate running into brick walls so I reinstalled python(This time making sure to install for 'all users', as opposed to 'current user only') Tried running the bin\activate.bat file again with no success. I decided something was up with the bat file itself, python was recognized universally on the disk and had no other issues. I messed around with keys in reg edit. Eventually I moved a " over at the end of:
"%PYTHONINSTALL%" to make it appear like this:
if "%PYTHONINSTALL% " NEQ "" goto FoundPython

The movement of one quote allowed it to activate and so far the sdk seems to be working normal.

Comment 1

[Peter Warren]

Same exact issue for me, using 64-bit python 2.7.7 on Windows 7 Professional.

Todd Keck's fix above worked for me as well. Thanks, Todd!

Comment 2

[marcuse7]

Seems that with the command
set PYTHONINSTALL=%PYTHONINSTALL: =%
the spaces are removed from the path as well, although it was meant for leading spaces only. This command is used twice in the batch.

That is, the found registry entry such as
    REG_SZ    C:\Program Files\Python27\
is reduced to
C:\ProgramFiles\Python27\
(note the missing space in ProgramFiles)

A workaround could be using
for /f "tokens=* delims= " %%a in ("%PYTHONINSTALL%") do set PYTHONINSTALL=%%a
instead of line above; it removes only leading spaces. Also, the line
if exist %PYTHONINSTALL%\python.exe goto :EOF
should be corrected to
if exist "%PYTHONINSTALL%\python.exe" goto :EOF

Comment 3

[BrainCrumbz Team]

While looking for same error on Windows 7 64, with Python installed under directory

c:\Program Files (x86)\Python27

I've also found another "workaround" at this StackOverflow answer:

http://stackoverflow.com/a/23003887/540776

Please note that I also tried with adding Python directory to path in its short form:

C:\PROGRA~2\Python27

and had the same error.

Comment 4

[BrainCrumbz Team]

Actually, debugging the batch, it looks like the issue is within this:

  FOR /F "usebackq tokens=2 delims=)>" %%A IN (`%reg% QUERY HKLM\%key% /ve 2^>NUL`) DO SET "%~1=%%A"

explained by a preceding comment: 

  rem SO: we use ")>" as the tokens to split on, then nuke.

After the FOR runs, the output string is (double * added by me):

  **    REG_SZ    C:\Program Files (x86**

which chops away the closing parens in "C:\Program Files (x86)" and everything afterwards.

HTH

Comment 5

[BrainCrumbz Team]

Here's an alternative way to isolate the path (possibly) returned by reg query, independent from Windows version:

 1. Replace the only guaranteed word in query result, "REG_SZ", with a unique single character, e.g. "?". (See note later)
 2. Then use that unique single char, if found, to split query result in 2 tokens and get only the 2nd one, if any. 
 3. Finally, trim tabs and spaces from the left of such token, to get the path.

Note:
A question mark could actually be part of a path, although that seems to be uncommon. Ideally it should be a character unallowed in paths: |, <, >, and so on. But some of those gave us troubles escaping them.

There's also another issue with original code, when checking: 

   if exist %PYTHONINSTALL%\whatever goto :EOF

the path being checked should be enclosed in double quotes, to take into account for paths containing spaces.

So all in all, here's the alternative implementation, e.g. just for the HKML part:

  rem Try HKLM
  SET QueryResult=
  FOR /F "usebackq delims=" %%r IN (`%reg% QUERY HKLM\%key% /ve 2^>NUL`) DO @SET QueryResult=%%r

  SET ReplacedResult=%QueryResult:REG_SZ=?%
  FOR /F "tokens=2 delims=?" %%t IN ("%ReplacedResult%") DO SET "%~1=%%t"

  rem trim tabs and spaces from the left (note: there's a literal tab in next line)
  FOR /F "tokens=* delims=	 " %%v IN ("%PYTHONINSTALL%") DO SET PYTHONINSTALL=%%v

  if exist "%PYTHONINSTALL%\python.exe" goto :EOF
  rem It may be a 32bit Python directory built from source, in which case the
  rem executable is in the PCBuild directory.
  if exist "%PYTHONINSTALL%\PCBuild\python.exe" (set "PYTHONINSTALL=%PYTHONINSTALL%\PCBuild" & goto :EOF)
  rem Or maybe a 64bit build directory.
  if exist "%PYTHONINSTALL%\PCBuild\amd64\python.exe" (set "PYTHONINSTALL=%PYTHONINSTALL%\PCBuild\amd64" & goto :EOF)

If needed, please have a look at this Github commit to see the actual diff:

https://github.com/BrainCrumbz/addon-sdk/commit/93580c847cc4b11cad3fc9768612fd79b278cc21

Comment 6

[BrainCrumbz Team]

Created PR on Github repo, at https://github.com/mozilla/addon-sdk/pull/1622

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Irakli, can you find a reviewer for that pull request?

Comment 8

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 713206 [details]
activate.bat

Could you please take this review ?

Comment 9

[BrainCrumbz Team]

* typo: 'just for the HKLM part'

Comment 10

[Jordan Santell [:jsantell] [@jsantell]]

I don't have a windows machine or any knowledge of batch, so this'll take me awhile to get around to unless there is a more appropriate reviewer

Comment 11

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 713206 [details]
activate.bat

Gonna have to remove myself from this review -- I won't be able to get around to this for a long time, not counting the time i have to learn batch

Comment 12

[BrainCrumbz Team]

:-\ ... not many exciting news on this thread

Comment 13

[Erik Vold [:erikvold] (please needinfo? me)]

Sorry we won't be releasing any new versions of cfx, jpm is the replacement https://www.npmjs.com/package/jpm