landam / grass-gis-git-migration-test

0 stars 0 forks source link

choose python interpreter during the GRASS installation on windows #123

Open landam opened 5 years ago

landam commented 5 years ago

Reported by zarch on 12 Jun 2014 09:11 UTC Using the Windows installer would be great to be able to choice which python interpreter must be used by GRASS.

Suppose that we have already installed a python interpreter like https://code.google.com/p/pythonxy/ pythonxy], at the moment the GRASS installer use is own python interpreter therefore if I want to use any python packages from GRASS, like http://www.scipy.org/ scipy or [http://pandas.pydata.org/ pandas I have, some how, to install these packages on the GRASS interpreter even if they are already present in pythonxy.

Do you think that would be possible to add this point to the GRASS installer? or It is not possible to add this option because GRASS must be compiled using the new interpreter? where should I look to try to add this option?

Any hints?

Operating system

MS Windows

GRASS GIS version and provenance

svn-trunk

Migrated-From: https://trac.osgeo.org/grass/ticket/2333

landam commented 5 years ago

Comment by mlennert on 12 Jun 2014 10:13 UTC Replying to [ticket:2333 zarch]:

Using the Windows installer would be great to be able to choice which python interpreter must be used by GRASS.

Suppose that we have already installed a python interpreter like https://code.google.com/p/pythonxy/ pythonxy], at the moment the GRASS installer use is own python interpreter therefore if I want to use any python packages from GRASS, like http://www.scipy.org/ scipy or [http://pandas.pydata.org/ pandas I have, some how, to install these packages on the GRASS interpreter even if they are already present in pythonxy.

Do you think that would be possible to add this point to the GRASS installer? or It is not possible to add this option because GRASS must be compiled using the new interpreter? where should I look to try to add this option?

Any hints?

http://osgeo-org.1560.x6.nabble.com/Handling-of-Python-scripts-on-MS-Windows-tt5081335.html

I would guess that this thread is probably one of the longest in grass-dev history. Have fun ! ;-)

Moritz

landam commented 5 years ago

Comment by hellik on 12 Jun 2014 11:20 UTC Replying to [ticket:2333 zarch]:

Using the Windows installer would be great to be able to choice which python interpreter must be used by GRASS.

Suppose that we have already installed a python interpreter like https://code.google.com/p/pythonxy/ pythonxy], at the moment the GRASS installer use is own python interpreter therefore if I want to use any python packages from GRASS, like http://www.scipy.org/ scipy or [http://pandas.pydata.org/ pandas I have, some how, to install these packages on the GRASS interpreter even if they are already present in pythonxy.

Do you think that would be possible to add this point to the GRASS installer? or It is not possible to add this option because GRASS must be compiled using the new interpreter? where should I look to try to add this option?

Any hints?

a first step would be to change

https://trac.osgeo.org/grass/browser/grass/trunk/mswindows/env.bat#L9

9   set GRASS_PYTHON=%GISBASE%\extrabin\python.exe 

to point to your already installed other python and test if this works (no idea if any other changes e.g. in wxgui are needed).

''env.bat'' is AFAIR in a subfolder of ''C:\Program Files (x86)\GRASS GIS X.x''

keep in mind, these other python should have compatible GRASS dependencies (e.g. numpy, wxpython, matplotlib,...).

if the change in env.bat works, the next step would be to add an option which python to use to the nsis-script:

https://trac.osgeo.org/grass/browser/grass/trunk/mswindows/GRASS-Installer.nsi.tmpl

landam commented 5 years ago

Comment by hellik on 12 Jun 2014 11:55 UTC

a first step would be to change

https://trac.osgeo.org/grass/browser/grass/trunk/mswindows/env.bat#L9

9 set GRASS_PYTHON=%GISBASE%\extrabin\python.exe 

oh, also this line has to be adapted:

https://trac.osgeo.org/grass/browser/grass/trunk/mswindows/env.bat#L10

10  set PYTHONHOME=%GISBASE%\Python27 

and should point to the pythonhome of your other python.

landam commented 5 years ago

Comment by zarch on 12 Jun 2014 11:59 UTC Replying to [comment:1 mlennert]:

http://osgeo-org.1560.x6.nabble.com/Handling-of-Python-scripts-on-MS-Windows-tt5081335.html

I would guess that this thread is probably one of the longest in grass-dev history. Have fun ! ;-)

Thanks Moritz, I saw the thread but it seems to me that it is not going anywhere, so I'm trying to change prospective. I think that my point is close but it is not the same. I'm not talking on how to execute automatically the GRASS scripts on windows, I'm talking on choosing the default interpreter that must be used by GRASS.

If I understood the long thread we were talking on how we can link the windows default python interpreter to run our scripts, if we have to use or not the default python interpreter. Instead here I'm talking to give to the user the chance of choosing which python interpreter must be used by GRASS, if not specify just use its own interpreter.

The problem if choosing a different python interpreter could be the ctypes binding of python that must be re-compiled, isn't it?

I don't want to use the default GRASS interpreter because I need other libraries that are not present on the GRASS python interpreter (GPyI) and install these libraries is not easy, generally they require to compile, and compile on windows is a pain.

The solution that I found and it is working so far, it is just to copy all the python/site-packages from pythonxy to the site-packages of the GPyI.

But it is just a work around, IMHO the best option should be to be able to choose during the installation process which one should be used by GRASS.

Pietro

landam commented 5 years ago

Comment by hellik on 12 Jun 2014 12:49 UTC Replying to [comment:4 zarch]:

The solution that I found and it is working so far, it is just to copy all the python/site-packages from pythonxy to the site-packages of the GPyI.

But it is just a work around, IMHO the best option should be to be able to choose during the installation process which one should be used by GRASS.

Pietro

see my two comments above and change ''env.bat'' accordingly in L9 and L10.

if that works, the option logic has to be added in the nsis-installer script.

Helmut

landam commented 5 years ago

Comment by glynn on 12 Jun 2014 22:21 UTC Replying to [comment:4 zarch]:

The problem if choosing a different python interpreter could be the ctypes binding of python that must be re-compiled, isn't it?

No. The ctypes module allows you to define Python wrappers for C functions using only Python code. Nothing in a compiled GRASS package links against the Python DLL/DSO (there used to be a couple of binary components in wxGUI, but those have long since been removed).

landam commented 5 years ago

Comment by zarch on 13 Jun 2014 08:34 UTC Replying to [comment:5 hellik]:

see my two comments above and change ''env.bat'' accordingly in L9 and L10.

if that works, the option logic has to be added in the nsis-installer script.

Thanks Helmut. Sorry for the delay, but I had to install all on my windows environment to be free to test, before I was using the computer of one of my colleagues... :-)

ok, so I created a new bat file called: grass70svn_xy.bat that contains:

set GISBASE=C:\Users\PZambelli\Documents\GRASS GIS 7.0.0svn

rem => add a environmental variable for pythonxy
set PY_XY=C:\Python27

rem => reading the env file for pxthonxy
call "%GISBASE%\etc\env_xy.bat"

cd "%USERPROFILE%"
"%GRASS_PYTHON%" "%GISBASE%\etc\grass70.py" %*

if %ERRORLEVEL% GEQ 1 pause

and in etc I added a new env_xy.bat file with:

set GRASS_SH=%GISBASE%\msys\bin\sh.exe

set GRASS_HTML_BROWSER=chrome

rem GRASS_PYTHON=%GISBASE%\extrabin\python.exe
set GRASS_PYTHON=%PY_XY%\python.exe

rem PYTHONHOME=%GISBASE%\Python27
set PYTHONHOME=%PY_XY%

set GRASS_PROJSHARE=%GISBASE%\share\proj

set PROJ_LIB=%GISBASE%\share\proj
set GDAL_DATA=%GISBASE%\share\gdal
set GEOTIFF_CSV=%GISBASE%\share\epsg_csv

set PATH=%GISBASE%\msys\bin;%PATH%
set PATH=%GISBASE%\extrabin;%PATH%
set PATH=%GISBASE%\bin;%PATH%

Then I open a cmd prompt, I move to the GISBASE directory and run: grass70svn_xy.bat...

every is working without errors and the GUI is starting, from the GRASS cmd prompt I got:

GRASS 7.0.0svn> python -c "import sys; print(sys.executable)"
c:\Users\PZambelli\Documents\GRASS GIS 7.0.0svn\extrabin\python.exe

and the same result from the GUI command console,

But from the command prompt if I check the GRASS_PYTHON environment it seems to be set correctly:

GRASS 7.0.0svn> echo $GRASS_PYTHON$
C:\Python27\python.exe$

But I got this error If I try to use the GRASS_PYTHON interpreter:

GRASS 7.0.0svn> $GRASS_PYTHON$
sh.exe": C:\Python27\python.exe$: No such file or directory

Instead the GUI Python shell is using the pythonxy interpreter:

>>> import sys; sys.executable
C:\Python27\python.exe

So it seemes to work, I guess...

landam commented 5 years ago

Comment by zarch on 13 Jun 2014 08:37 UTC Replying to [comment:6 glynn]:

Replying to [comment:4 zarch]:

The problem if choosing a different python interpreter could be the ctypes binding of python that must be re-compiled, isn't it?

No. The ctypes module allows you to define Python wrappers for C functions using only Python code. Nothing in a compiled GRASS package links against the Python DLL/DSO (there used to be a couple of binary components in wxGUI, but those have long since been removed).

Perfect! thank you for the clarification!

landam commented 5 years ago

Comment by hellik on 13 Jun 2014 08:52 UTC Replying to [comment:7 zarch]:

But I got this error If I try to use the GRASS_PYTHON interpreter:

GRASS 7.0.0svn> $GRASS_PYTHON$
sh.exe": C:\Python27\python.exe$: No such file or directory

try

GRASS 7.1.svn> $GRASS_PYTHON

see no $ at the end (works here)

landam commented 5 years ago

Comment by zarch on 13 Jun 2014 09:04 UTC Replying to [comment:9 hellik]:

Replying to [comment:7 zarch]:

But I got this error If I try to use the GRASS_PYTHON interpreter:

GRASS 7.0.0svn> $GRASS_PYTHON$
sh.exe": C:\Python27\python.exe$: No such file or directory

try

GRASS 7.1.svn> $GRASS_PYTHON

see no $ at the end (works here)

Right, It's working... sorry for the typo!

ok, next week I will try to look in to the nsis script to see if is it possible to add this option during the installation process.

Helmut Thanks for the help.

Pietro

landam commented 5 years ago

Comment by hellik on 13 Jun 2014 11:36 UTC Replying to [comment:10 zarch]:

ok, next week I will try to look in to the nsis script to see if is it possible to add this option during the installation process.

some steps may be:

technical implementation - some pointers:

a few years ago I've added two string replace functions to the nsis-script:

https://trac.osgeo.org/grass/browser/grass/trunk/mswindows/GRASS-Installer.nsi.tmpl#L104

https://trac.osgeo.org/grass/browser/grass/trunk/mswindows/GRASS-Installer.nsi.tmpl#L338

use some of these functions to adjust %GRASS_PYTHON% and %PYTHONHOME% in ''env.bat'' to the user defined paths by a e.g. if/else statement

techn

landam commented 5 years ago

Comment by zarch on 16 Jun 2014 09:24 UTC Replying to [comment:11 hellik]:

Replying to [comment:10 zarch]:

ok, next week I will try to look in to the nsis script to see if it is possible to add this option during the installation process.

some steps may be:

  • add e new installer gui-section "Customize python" (or something like that)

  • add some explanations to the gui-section "Customize python": e.g. compatible wxpython, numpy, matplotlib, etc. are needed, ...

  • default option: %GRASS_PYTHON% is the packaged python

  • user option: ask the user for the %PATH% to the user wanted python

Thank you Helmut for your help.

I'm not sure If I understood how to add a new section, I wrote:

Section "Customize python" CustomizePython
    ;Declares variables for optional Python interpreter
    Var /GLOBAL PYINTERPRETER
    Var /GLOBAL PYHOME

    ;Set default values for the Python Interpreter
    ${If} $ASK_FOR_PYINTERP == "NO"
        StrCpy $PYINTERPRETER "%GISBASE%\extrabin\python.exe"
    ${Else}
        StrCpy $PYINTERPRETER "$INST_PYINTERP"
    ${EndIf}

    ;Set default values for the Python home folder
    ${If} $ASK_FOR_PYHOME == "NO"
        StrCpy $PYHOME "%GISBASE%\Python27"
    ${Else}
        StrCpy $PYHOME "$INST_PYHOME"
    ${EndIf}

    ;Create the etc\env.bat
    ClearErrors
    FileOpen $0 $INSTALL_DIR\etc\env.bat w
    IfErrors done_create_grass_environments.bat
    FileWrite $0 '@echo off$\r$\n'
    FileWrite $0 'rem #########################################################################$\r$\n'
    FileWrite $0 'rem #$\r$\n'
    FileWrite $0 'rem # File dynamically created by NSIS installer script;$\r$\n'
    FileWrite $0 'rem #$\r$\n'
    FileWrite $0 'rem #########################################################################$\r$\n'
    FileWrite $0 'rem #$\r$\n'
    FileWrite $0 'rem # Environmental variables for GRASS stand-alone installer$\r$\n'
    FileWrite $0 'rem #$\r$\n'
    FileWrite $0 'rem #########################################################################$\r$\n'
    FileWrite $0 '$\r$\n'
    FileWrite $0 'set GRASS_SH=%GISBASE%\msys\bin\sh.exe$\r$\n'
    FileWrite $0 '$\r$\n'
    FileWrite $0 'set GRASS_HTML_BROWSER=explorer"$\r$\n'
    FileWrite $0 '$\r$\n'
    FileWrite $0 'set GRASS_PYTHON=$PYINTERPRETER$\r$\n'
    FileWrite $0 'set PYTHONHOME=$PYHOME$\r$\n'
    FileWrite $0 '$\r$\n'
    FileWrite $0 'set GRASS_PROJSHARE=%GISBASE%\share\proj$\r$\n'
    FileWrite $0 '$\r$\n'
    FileWrite $0 'set PROJ_LIB=%GISBASE%\share\proj$\r$\n'
    FileWrite $0 'set GDAL_DATA=%GISBASE%\share\gdal$\r$\n'
    FileWrite $0 'set GEOTIFF_CSV=%GISBASE%\share\epsg_csv$\r$\n'
    FileWrite $0 '$\r$\n'
    FileWrite $0 'set PATH=%GISBASE%\msys\bin;%PATH%$\r$\n'
    FileWrite $0 'set PATH=%GISBASE%\extrabin;%PATH%$\r$\n'
    FileWrite $0 'set PATH=%GISBASE%\bin;%PATH%$\r$\n'
    FileClose $0
SectionEnd

Do you see any better way to implement this, avoiding to generate the env.bat file?

How can I define as optional to set the path to the python interpreter and to the python home in the GUI? Where should I add the description with the python requirements (numpy, wxpython, etc)?

There is a way to view/test the modified NSIS file?

landam commented 5 years ago

Comment by @landam on 16 Jun 2014 09:36 UTC Replying to [comment:12 zarch]:

There is a way to view/test the modified NSIS file?

Ideal place would be trunk (+ daily builds). Unfortunately daily builds of winGRASS 7.1 (trunk) are completely broken (1)* thanks to https://trac.osgeo.org/grass/changeset/60679 (so called "magic to break the whole system"). We could probably try to fix it first(?).

Traceback (most recent call last): 
 File "c:/osgeo4w/usr/src/grass_trunk/dist.i686-pc-mingw32/scripts/d.out.file.py", line 59, in <module> 
   main() 
 File "c:/osgeo4w/usr/src/grass_trunk/dist.i686-pc-mingw32/scripts/d.out.file.py", line 44, in main 
   options, flags = gcore.parser() 
 File "c:\osgeo4w\usr\src\grass_trunk\dist.i686-pc-mingw32\etc\python\grass\script\core.py", line 647, in parser 
   p = Popen(['g.parser', '-n'] + argv, stdout=PIPE) 
 File "c:\OSGeo4W\apps\Python27\lib\subprocess.py", line 711, in __init__ 
   errread, errwrite) 
 File "c:\OSGeo4W\apps\Python27\lib\subprocess.py", line 948, in _execute_child 
   startupinfo) 
WindowsError: [Error 2] Systzt uveden soubor
landam commented 5 years ago

Comment by zarch on 16 Jun 2014 10:14 UTC Hi Martin,

Replying to [comment:13 martinl]:

Replying to [comment:12 zarch]:

There is a way to view/test the modified NSIS file?

Ideal place would be trunk (+ daily builds). Unfortunately daily builds of winGRASS 7.1 (trunk) are completely broken (1)* thanks to https://trac.osgeo.org/grass/changeset/60679 (so called "magic to break the whole system"). We could probably try to fix it first(?).

If I interpreted correctly [0], the solution should be to generate the .bat files for each GRASS module/scripts written in python, right?.

Do you think that could be possible to generate the .bat files in the NSIS file during the installation process?

Do you think that the NSIS file is the correct place where these .bat files should be generated or we should put this action in some other place during the installation process?

Pietro

[0] https://www.mail-archive.com/grass-dev@lists.osgeo.org/msg35001.html

landam commented 5 years ago

Comment by @landam on 16 Jun 2014 10:25 UTC Replying to [comment:14 zarch]:

If I interpreted correctly [0], the solution should be to generate the .bat files for each GRASS module/scripts written in python, right?.

no, https://trac.osgeo.org/grass/changeset/60679 simply broke running all commands (ie. also running exe files from python), see the traceback from the previous comment.

landam commented 5 years ago

Comment by @landam on 16 Jun 2014 10:28 UTC Replying to [comment:14 zarch]:

Do you think that could be possible to generate the .bat files in the NSIS file during the installation process?

in GRASS 6 bat files are generated by building system source:grass/branches/releasebranch_6_4/include/Make/Script.make#L26. I have local patch for that on my machine, but I still hope that https://trac.osgeo.org/grass/changeset/60679 will be somehow fixed firstly.

landam commented 5 years ago

Comment by @landam on 16 Jun 2014 10:30 UTC Replying to [comment:15 martinl]:

Replying to [comment:14 zarch]:

If I interpreted correctly [0], the solution should be to generate the .bat files for each GRASS module/scripts written in python, right?.

no, https://trac.osgeo.org/grass/changeset/60679 simply broke running all commands (ie. also running exe files from python), see the traceback from the previous comment.

http://wingrass.fsv.cvut.cz/grass71/logs/log-https://trac.osgeo.org/grass/changeset/60827-992/

landam commented 5 years ago

Comment by hellik on 16 Jun 2014 17:34 UTC Replying to [comment:12 zarch]:

Replying to [comment:11 hellik]:

Replying to [comment:10 zarch]:

ok, next week I will try to look in to the nsis script to see if it is possible to add this option during the installation process.

some steps may be:

  • add e new installer gui-section "Customize python" (or something like that)

  • add some explanations to the gui-section "Customize python": e.g. compatible wxpython, numpy, matplotlib, etc. are needed, ...

  • default option: %GRASS_PYTHON% is the packaged python

  • user option: ask the user for the %PATH% to the user wanted python

Thank you Helmut for your help.

I'm not sure If I understood how to add a new section, I wrote:

Section "Customize python" CustomizePython
  ;Declares variables for optional Python interpreter
  Var /GLOBAL PYINTERPRETER
  Var /GLOBAL PYHOME

  ;Set default values for the Python Interpreter
  ${If} $ASK_FOR_PYINTERP == "NO"
      StrCpy $PYINTERPRETER "%GISBASE%\extrabin\python.exe"
  ${Else}
      StrCpy $PYINTERPRETER "$INST_PYINTERP"
  ${EndIf}

  ;Set default values for the Python home folder
  ${If} $ASK_FOR_PYHOME == "NO"
      StrCpy $PYHOME "%GISBASE%\Python27"
  ${Else}
      StrCpy $PYHOME "$INST_PYHOME"
  ${EndIf}

  ;Create the etc\env.bat
  ClearErrors
  FileOpen $0 $INSTALL_DIR\etc\env.bat w
  IfErrors done_create_grass_environments.bat
  FileWrite $0 '@echo off$\r$\n'
  FileWrite $0 'rem #########################################################################$\r$\n'
  FileWrite $0 'rem #$\r$\n'
  FileWrite $0 'rem # File dynamically created by NSIS installer script;$\r$\n'
  FileWrite $0 'rem #$\r$\n'
  FileWrite $0 'rem #########################################################################$\r$\n'
  FileWrite $0 'rem #$\r$\n'
  FileWrite $0 'rem # Environmental variables for GRASS stand-alone installer$\r$\n'
  FileWrite $0 'rem #$\r$\n'
  FileWrite $0 'rem #########################################################################$\r$\n'
  FileWrite $0 '$\r$\n'
  FileWrite $0 'set GRASS_SH=%GISBASE%\msys\bin\sh.exe$\r$\n'
  FileWrite $0 '$\r$\n'
  FileWrite $0 'set GRASS_HTML_BROWSER=explorer"$\r$\n'
  FileWrite $0 '$\r$\n'
  FileWrite $0 'set GRASS_PYTHON=$PYINTERPRETER$\r$\n'
  FileWrite $0 'set PYTHONHOME=$PYHOME$\r$\n'
  FileWrite $0 '$\r$\n'
  FileWrite $0 'set GRASS_PROJSHARE=%GISBASE%\share\proj$\r$\n'
  FileWrite $0 '$\r$\n'
  FileWrite $0 'set PROJ_LIB=%GISBASE%\share\proj$\r$\n'
  FileWrite $0 'set GDAL_DATA=%GISBASE%\share\gdal$\r$\n'
  FileWrite $0 'set GEOTIFF_CSV=%GISBASE%\share\epsg_csv$\r$\n'
  FileWrite $0 '$\r$\n'
  FileWrite $0 'set PATH=%GISBASE%\msys\bin;%PATH%$\r$\n'
  FileWrite $0 'set PATH=%GISBASE%\extrabin;%PATH%$\r$\n'
  FileWrite $0 'set PATH=%GISBASE%\bin;%PATH%$\r$\n'
  FileClose $0
SectionEnd

Do you see any better way to implement this, avoiding to generate the env.bat file?

How can I define as optional to set the path to the python interpreter and to the python home in the GUI? Where should I add the description with the python requirements (numpy, wxpython, etc)?

There is a way to view/test the modified NSIS file?

Hi Pietro,

I'm travelling, no chance to look at it at the moment.

there is no need to write a new env.bat file, just replace the path string in the existing env.bat.

Helmut

landam commented 5 years ago

Comment by glynn on 17 Jun 2014 11:27 UTC Replying to [comment:15 martinl]:

If I interpreted correctly [0], the solution should be to generate the .bat files for each GRASS module/scripts written in python, right?.

no, https://trac.osgeo.org/grass/changeset/60679 simply broke running all commands (ie. also running exe files from python), see the traceback from the previous comment.

Ugh. So without shell=True it requires an explicit .exe suffix if the program has a dot in its name. And with shell=True, it interprets characters such as "|", "<", ">" and I don't know what else.

So, do we mimic the shell (locate the program and determine its suffix), or use it (and determine the escaping rules so that "special" character get passed through to the underlying program correctly)?

I still don't consider the previous approach (use the shell, don't bother escaping anything) to be a viable solution.

landam commented 5 years ago

Comment by @landam on 17 Jun 2014 11:43 UTC Replying to [comment:19 glynn]:

So, do we mimic the shell (locate the program and determine its suffix), or use it (and

There was something similar installed for Python scripts (1). One option would be to load all commands to an internal directory (similarly to wxGUI) when loading python script library

{ 'bat': [], 'exe': ['g.parser', ...] }

rather then determining it's suffix for every command separately.

I still don't consider the previous approach (use the shell, don't bother escaping anything) to be a viable solution.

Using shell=True seems to be less problematic to me, is it problem to determine escaping rules for special characters (I know only about pipe problem till now).

(1) http://trac.osgeo.org/grass/changeset/60679/grass/trunk/lib/python/script/core.py (L56)

landam commented 5 years ago

Comment by hellik on 18 Jun 2014 19:14 UTC Replying to [comment:12 zarch]:

I'm not sure If I understood how to add a new section, I wrote:

sorry, it's not a section, it should be a python costumize-'''page''':

see http://nsis.sourceforge.net/Docs/Modern%20UI/Readme.html

see ''3. Pages'' and ''Custom pages'' in the manual.

still travelling ...

landam commented 5 years ago

Comment by glynn on 19 Jun 2014 12:40 UTC Replying to [comment:20 martinl]:

So, do we mimic the shell (locate the program and determine its suffix), or use it (and

There was something similar installed for Python scripts (1).

Right. But if we take that route, it should apply to everything. And if the extension isn't one which !CreateProcess() handles directly (exe, bat, cmd, what else?), we should set shell=True (and ideally perform any necessary escaping).

IOW, something like:

class Popen(subprocess.Popen):
    def __init__(self, args, ...):
        if sys.platform == 'win32' and isinstance(args, list):
            cmd = shutil_which(args[0])
            args = [cmd] + args[1:]
            name, ext = os.path.splitext(cmd)
            if ext.lower() not in ['.exe', '.bat', '.cmd']:
                shell = True
                args = [escape_for_shell(arg) for arg in args]
        subprocess.Popen.__init__(self, args, shell=shell, ...)

I still don't consider the previous approach (use the shell, don't bother escaping anything) to be a viable solution.

Using shell=True seems to be less problematic to me, is it problem to determine escaping rules for special characters (I know only about pipe problem till now).

I'm unsure what the rules are, whether they're sufficient (i.e. whether there are sequences which simply cannot be passed), or whether I'd trust any documentation to be accurate. I also don't know whether there are any other gotchas (ANSI-versus-OEM codepage issues?). But we don't really have a choice; execution of arbitrary scripts has to go through the shell one way or another. The main thing is that we don't get the shell involved unless we have to (i.e. we don't do it for EXEs).

The specific issue with the parser() function can be dealt with by using 'g.parser.exe' on Windows. And it should probably use subprocess.Popen() directly rather than grass.script.Popen(), to avoid any issues with whatever magic may be added to the latter. Executing g.parser with shell=True may be where the need for double-escaping was coming from.

landam commented 5 years ago

Comment by wenzeslaus on 14 Sep 2014 15:59 UTC The things related to invocation are mainly tracked by https://trac.osgeo.org/grass/ticket/2150 but the original requirement is still valid whatever is the outcome of https://trac.osgeo.org/grass/ticket/2150.

landam commented 5 years ago

Comment by annakrat on 22 Jan 2016 20:04 UTC It seems Pietro got pretty close to solving this, any possibility we could restart the attempts? The patch may need a review by Helmut and then we can apply it to trunk? I would be glad to test it.

landam commented 5 years ago

Comment by hellik on 23 Jan 2016 19:20 UTC Replying to [comment:24 annakrat]:

It seems Pietro got pretty close to solving this, any possibility we could restart the attempts? The patch may need a review by Helmut and then we can apply it to trunk? I would be glad to test it.

the patch needs some review as the nsis-script changed a lot since then (e.g. 32bit/64bit, changes in env.bat, ...).

there are some open questions:

landam commented 5 years ago

Comment by wenzeslaus on 23 Jan 2016 22:22 UTC Replying to [comment:25 hellik]:

should there in the standalone installer be only the option to choose the python interpreter or also some check of needed dependencies (wxwidgets, python win api etc.)? for example my system wide python installation (installed by another software) isn't compatible to run winGRASS out of the box

In [comment:52:ticket:580 ticket 580] Michael is saying that wxPython is bundled with GRASS GIS and the system Python is used. Is there a way to do something like this on Windows?

how/if should such an option implemented in OSGeo4W-winGRASS too?

It would have to be implemented for the whole OSGeo4W I suppose. The reason for doing it there is the same as for the standalone version. OSGeo4W doesn't contain all the packages available through pip and it doesn't contain pip.

if not, there would be disparity between different platforms on windows (standalone, OSGeo4W, QGIS bundled).

I think the idea is to make it more similar across operating system. On Linux, one does not have to install Python packages for GRASS separately or do some workaround to get the already installed there. Mac OS X works the same if you are (very) lucky.

OSGeo4W, QGIS-bundled or whatever-bundled might need to use the internal Python option. Are the install scripts able to handle two options or it is too difficult?

how should our users deal with these many different variants? e.g. for myself I never work with the standalone installer, prefer the OSGeo4W-environment as there are quite a lot tools, which can easily be updated.

There are already different variants. And unless all OSGeo projects agree on using OSGeo4W exclusively, we will always have them. With installation of additional packages, I'm not worried about them much. I'm more worried about them in regard to https://trac.osgeo.org/grass/ticket/2873, but still, Linux, Mac and standalone would/could be the same, OSGeo4W and QGIS-bundled would be different.

landam commented 5 years ago

Comment by hellik on 23 Jan 2016 22:42 UTC Replying to [comment:26 wenzeslaus]:

Replying to [comment:25 hellik]:

should there in the standalone installer be only the option to choose the python interpreter or also some check of needed dependencies (wxwidgets, python win api etc.)? for example my system wide python installation (installed by another software) isn't compatible to run winGRASS out of the box

In [comment:52:ticket:580 ticket 580] Michael is saying that wxPython is bundled with GRASS GIS and the system Python is used. Is there a way to do something like this on Windows?

as mentioned at other places, there is no system wide python in windows at all. users has to do the install for themself, also all the packages

how/if should such an option implemented in OSGeo4W-winGRASS too?

It would have to be implemented for the whole OSGeo4W I suppose. The reason for doing it there is the same as for the standalone version. OSGeo4W doesn't contain all the packages available through pip and it doesn't contain pip.

what about to improve the situation in the underlying Osgeo4w? There are already some starting efforts regarding pip etc.

if not, there would be disparity between different platforms on windows (standalone, OSGeo4W, QGIS bundled).

I think the idea is to make it more similar across operating system. On Linux, one does not have to install Python packages for GRASS separately or do some workaround to get the already installed there. Mac OS X works the same if you are (very) lucky.

OSGeo4W, QGIS-bundled or whatever-bundled might need to use the internal Python option. Are the install scripts able to handle two options or it is too difficult?

how should our users deal with these many different variants? e.g. for myself I never work with the standalone installer, prefer the OSGeo4W-environment as there are quite a lot tools, which can easily be updated.

There are already different variants. And unless all OSGeo projects agree on using OSGeo4W exclusively, we will always have them. With installation of additional packages, I'm not worried about them much. I'm more worried about them in regard to https://trac.osgeo.org/grass/ticket/2873, but still, Linux, Mac and standalone would/could be the same, OSGeo4W and QGIS-bundled would be different.

I'm not quite sure about the effort regarding overcome the differences of OS at software level. but that's a personal point of view. If you look at the QGIS ticket at using "system wide python". there is at least some concern at the windows site of life.

landam commented 5 years ago

Comment by wenzeslaus on 24 Jan 2016 00:03 UTC Replying to [comment:27 hellik]:

as mentioned at other places, there is no system wide python in windows at all. users has to do the install for themself, also all the packages

Right. Python installed ahead which is on path, registry or whatever is appropriate - that's what I mean by system Python.

It would have to be implemented for the whole OSGeo4W I suppose. The reason for doing it there is the same as for the standalone version. OSGeo4W doesn't contain all the packages available through pip and it doesn't contain pip.

what about to improve the situation in the underlying Osgeo4w?

Good approach. I guess this ticket would still apply standalone installer?

There are already some starting efforts regarding pip etc.

Do you have some more info about it? Only thing I know is not really encouraging: https://lists.osgeo.org/pipermail/osgeo4w-dev/2015-November/002965.html my question regarding pip from September 2015] and [https://lists.osgeo.org/pipermail/osgeo4w-dev/2015-September/002957.html answer to it from November.

Even if we ignore the issue that user have to remember to install every package twice (I guess common on MS Windows), this still doesn't solve the issue of using the same Python from a Python editor and GRASS or using GRASS and ArcGIS together (both being a common request).

landam commented 5 years ago

Comment by neteler on 5 May 2016 14:08 UTC Milestone renamed

landam commented 5 years ago

Comment by neteler on 28 Dec 2016 15:04 UTC Ticket retargeted after milestone closed

landam commented 5 years ago

Modified by @landam on 5 May 2017 20:40 UTC

landam commented 5 years ago

Comment by @landam on 1 Sep 2017 20:28 UTC All enhancement tickets should be assigned to 7.4 milestone.

landam commented 5 years ago

Comment by neteler on 26 Jan 2018 11:40 UTC Ticket retargeted after milestone closed

landam commented 5 years ago

Modified by neteler on 12 Jun 2018 20:48 UTC

landam commented 5 years ago

Comment by @landam on 25 Sep 2018 16:51 UTC All enhancement tickets should be assigned to 7.6 milestone.

landam commented 5 years ago

Comment by @landam on 25 Jan 2019 21:07 UTC Ticket retargeted after milestone closed