PR: Validate that Main Interpreter is really a Python interpreter #3842
Conversation
| def is_python_interpreter(self, pyexec): | ||
| if not osp.isfile(pyexec): | ||
| return False | ||
| args = ["-c", "print('valid_python_interpreter')"] |
There was a problem hiding this comment.
You can also run in a terminal:
ipython -c 'print("foo")'
so this is not enough for a test.
There was a problem hiding this comment.
I think a better test is
- Verify that the file is called
python,python2orpython3 - Verify that it's a binary (using
is_text_filefrom utils/encoding.py) - Run the command you defined here.
If those three criteria are met, then we can be more certain (although no completely sure :-) that the selected file is a Python interpreter.
There was a problem hiding this comment.
The problem with point 1. on Linux is that python{2,3} are symlinks, so you should follow them to the file they point to and check that it's a binary.
Also, about 1., users could introduce python2.7 or python3.5, so that should be covered too here.
There was a problem hiding this comment.
Isn't it possible to execute this command?
python --versionAnd then retrieve the stdout output and compare it against the regexp python \d([.]\d)*?
There was a problem hiding this comment.
Problem is this also works for ipython (although we could also filter for 2.7.x and 3.{4,5,6})
Besides, most Linux programs have a --version option and some could coincide with the Python versions I mentioned above :-)
There was a problem hiding this comment.
python -h
usage: python [option] ... [-c cmd | -m mod | file | -] [arg] ...
Options and arguments (and corresponding environment variables):
-b : issue warnings about str(bytes_instance), str(bytearray_instance)
and comparing bytes/bytearray with str. (-bb: issue errors)
-B : don't write .py[co] files on import; also PYTHONDONTWRITEBYTECODE=x
-c cmd : program passed in as string (terminates option list)
-d : debug output from parser; also PYTHONDEBUG=x
-E : ignore PYTHON* environment variables (such as PYTHONPATH)
-h : print this help message and exit (also --help)
-i : inspect interactively after running script; forces a prompt even
if stdin does not appear to be a terminal; also PYTHONINSPECT=x
-I : isolate Python from the user's environment (implies -E and -s)
-m mod : run library module as a script (terminates option list)
-O : optimize generated bytecode slightly; also PYTHONOPTIMIZE=x
-OO : remove doc-strings in addition to the -O optimizations
-q : don't print version and copyright messages on interactive startup
-s : don't add user site directory to sys.path; also PYTHONNOUSERSITE
-S : don't imply 'import site' on initialization
-u : unbuffered binary stdout and stderr, stdin always buffered;
also PYTHONUNBUFFERED=x
see man page for details on internal buffering relating to '-u'
-v : verbose (trace import statements); also PYTHONVERBOSE=x
can be supplied multiple times to increase verbosity
-V : print the Python version number and exit (also --version)
-W arg : warning control; arg is action:message:category:module:lineno
also PYTHONWARNINGS=arg
-x : skip first line of source, allowing use of non-Unix forms of #!cmd
-X opt : set implementation-specific option
file : program read from script file
- : program read from stdin (default; interactive mode if a tty)
arg ...: arguments passed to program in sys.argv[1:]
Other environment variables:
PYTHONSTARTUP: file executed on interactive startup (no default)
PYTHONPATH : ':'-separated list of directories prefixed to the
default module search path. The result is sys.path.
PYTHONHOME : alternate <prefix> directory (or <prefix>:<exec_prefix>).
The default module search path uses <prefix>/pythonX.X.
PYTHONCASEOK : ignore case in 'import' statements (Windows).
PYTHONIOENCODING: Encoding[:errors] used for stdin/stdout/stderr.
PYTHONFAULTHANDLER: dump the Python traceback on fatal errors.
PYTHONHASHSEED: if this variable is set to 'random', a random value is used
to seed the hashes of str, bytes and datetime objects. It can also be
set to an integer in the range [0,4294967295] to get hash values with a
predictable seed.
and
ipython -h ?
=========
IPython
=========
Tools for Interactive Computing in Python
=========================================
A Python shell with automatic history (input and output), dynamic object
introspection, easier configuration, command completion, access to the
system shell and more. IPython can also be embedded in running programs.
Usage
ipython [subcommand] [options] [-c cmd | -m mod | file] [--] [arg] ...
If invoked with no options, it executes the file and exits, passing the
remaining arguments to the script, just as if you had specified the same
command with python. You may need to specify `--` before args to be passed
to the script, to prevent IPython from attempting to parse them. If you
specify the option `-i` before the filename, it will enter an interactive
IPython session after running the script, rather than exiting. Files ending
in .py will be treated as normal Python, but files ending in .ipy can
contain special IPython syntax (magic commands, shell expansions, etc.).
Almost all configuration in IPython is available via the command-line. Do
`ipython --help-all` to see all available options. For persistent
configuration, look into your `ipython_config.py` configuration file for
details.
This file is typically installed in the `IPYTHONDIR` directory, and there
is a separate configuration directory for each profile. The default profile
directory will be located in $IPYTHONDIR/profile_default. IPYTHONDIR
defaults to to `$HOME/.ipython`. For Windows users, $HOME resolves to
C:\Users\YourUserName in most instances.
To initialize a profile with the default configuration file, do::
$> ipython profile create
and start editing `IPYTHONDIR/profile_default/ipython_config.py`
In IPython's documentation, we will refer to this directory as
`IPYTHONDIR`, you can change its default location by creating an
environment variable with this name and setting it to the desired path.
For more information, see the manual available in HTML and PDF in your
installation, or online at http://ipython.org/documentation.html.
Subcommands
-----------
Subcommands are launched as `ipython cmd [args]`. For information on using
subcommand 'cmd', do: `ipython cmd -h`.
history
Manage the IPython history database.
notebook
DEPRECATED, Will be removed in IPython 6.0 : Launch the Jupyter HTML Notebook Server.
trust
DEPRECATED, Will be removed in IPython 6.0 : Sign notebooks to trust their potentially unsafe contents at load.
locate
print the path to the IPython dir
install-nbextension
DEPRECATED, Will be removed in IPython 6.0 : Install Jupyter notebook extension files
kernel
Start a kernel without an attached frontend.
profile
Create and manage IPython profiles.
kernelspec
DEPRECATED, Will be removed in IPython 6.0 : Manage Jupyter kernel specifications.
console
DEPRECATED, Will be removed in IPython 6.0 : Launch the Jupyter terminal-based Console.
qtconsole
DEPRECATED, Will be removed in IPython 6.0 : Launch the Jupyter Qt Console.
nbconvert
DEPRECATED, Will be removed in IPython 6.0 : Convert notebooks to/from other formats.
Options
-------
Arguments that take values are actually convenience aliases to full
Configurables, whose aliases are listed on the help line. For more information
on full configurables, see '--help-all'.
--no-banner
Don't display a banner upon starting IPython.
--no-automagic
Turn off the auto calling of magic commands.
--quick
Enable quick startup with no config files.
--confirm-exit
Set to confirm when you try to exit IPython with an EOF (Control-D
in Unix, Control-Z/Enter in Windows). By typing 'exit' or 'quit',
you can force a direct exit without any confirmation.
-i
If running code from the command line, become interactive afterwards.
It is often useful to follow this with `--` to treat remaining flags as
script arguments.
--classic
Gives IPython a similar feel to the classic Python prompt.
--pdb
Enable auto calling the pdb debugger after every exception.
--nosep
Eliminate all spacing between prompts.
--simple-prompt
Force simple minimal prompt using `raw_input`
--no-term-title
Disable auto setting the terminal title.
--pylab
Pre-load matplotlib and numpy for interactive use with
the default matplotlib backend.
--no-autoedit-syntax
Turn off auto editing of files with syntax errors.
--no-autoindent
Turn off autoindenting.
--banner
Display a banner upon starting IPython.
--no-confirm-exit
Don't prompt the user when exiting.
--autoindent
Turn on autoindenting.
--no-pdb
Disable auto calling the pdb debugger after every exception.
--debug
set log level to logging.DEBUG (maximize logging output)
--term-title
Enable auto setting the terminal title.
--init
Initialize profile with default config files. This is equivalent
to running `ipython profile create <profile>` prior to startup.
--no-simple-prompt
Use a rich interactive prompt with prompt_toolkit
--autoedit-syntax
Turn on auto editing of files with syntax errors.
--color-info
IPython can display information about objects via a set of functions,
and optionally can use colors for this, syntax highlighting
source code and various other elements. This is on by default, but can cause
problems with some pagers. If you see such problems, you can disable the
colours.
--quiet
set log level to logging.CRITICAL (minimize logging output)
--pprint
Enable auto pretty printing of results.
--automagic
Turn on the auto calling of magic commands. Type %%magic at the
IPython prompt for more information.
--no-pprint
Disable auto pretty printing of results.
--no-color-info
Disable using colors for info related things.
--matplotlib
Configure matplotlib for interactive use with
the default matplotlib backend.
--log-level=<Enum> (Application.log_level)
Default: 30
Choices: (0, 10, 20, 30, 40, 50, 'DEBUG', 'INFO', 'WARN', 'ERROR', 'CRITICAL')
Set the log level by value or name.
--logfile=<Unicode> (InteractiveShell.logfile)
Default: ''
The name of the logfile to use.
--ext=<Unicode> (InteractiveShellApp.extra_extension)
Default: ''
dotted module name of an IPython extension to load.
--profile-dir=<Unicode> (ProfileDir.location)
Default: ''
Set the profile location directly. This overrides the logic used by the
`profile` option.
--ipython-dir=<Unicode> (BaseIPythonApplication.ipython_dir)
Default: ''
The name of the IPython directory. This directory is used for logging
configuration (through profiles), history storage, etc. The default is
usually $HOME/.ipython. This option can also be specified through the
environment variable IPYTHONDIR.
--pylab=<CaselessStrEnum> (InteractiveShellApp.pylab)
Default: None
Choices: ['auto', 'gtk', 'gtk3', 'inline', 'nbagg', 'notebook', 'osx', 'qt', 'qt4', 'qt5', 'tk', 'wx']
Pre-load matplotlib and numpy for interactive use, selecting a particular
matplotlib backend and loop integration.
--cache-size=<Int> (InteractiveShell.cache_size)
Default: 1000
Set the size of the output cache. The default is 1000, you can change it
permanently in your config file. Setting it to 0 completely disables the
caching system, and the minimum value accepted is 20 (if you provide a value
less than 20, it is reset to 0 and a warning is issued). This limit is
defined because otherwise you'll spend more time re-flushing a too small
cache than working
-c <Unicode> (InteractiveShellApp.code_to_run)
Default: ''
Execute the given command string.
--config=<Unicode> (BaseIPythonApplication.extra_config_file)
Default: ''
Path to an extra config file to load.
If specified, load this config file in addition to any other IPython config.
--gui=<CaselessStrEnum> (InteractiveShellApp.gui)
Default: None
Choices: ['glut', 'gtk', 'gtk2', 'gtk3', 'osx', 'pyglet', 'qt', 'qt4', 'qt5', 'tk', 'wx', 'gtk2', 'qt4']
Enable GUI event loop integration with any of ('glut', 'gtk', 'gtk2',
'gtk3', 'osx', 'pyglet', 'qt', 'qt4', 'qt5', 'tk', 'wx', 'gtk2', 'qt4').
--autocall=<Enum> (InteractiveShell.autocall)
Default: 0
Choices: (0, 1, 2)
Make IPython automatically call any callable object even if you didn't type
explicit parentheses. For example, 'str 43' becomes 'str(43)' automatically.
The value can be '0' to disable the feature, '1' for 'smart' autocall, where
it is not applied if there are no more arguments on the line, and '2' for
'full' autocall, where all callable objects are automatically called (even
if no arguments are present).
--profile=<Unicode> (BaseIPythonApplication.profile)
Default: 'default'
The IPython profile to use.
--colors=<CaselessStrEnum> (InteractiveShell.colors)
Default: 'Neutral'
Choices: ['Neutral', 'NoColor', 'LightBG', 'Linux']
Set the color scheme (NoColor, Neutral, Linux, or LightBG).
--logappend=<Unicode> (InteractiveShell.logappend)
Default: ''
Start logging to the given file in append mode. Use `logfile` to specify a
log file to **overwrite** logs to.
-m <Unicode> (InteractiveShellApp.module_to_run)
Default: ''
Run the module as a script.
--matplotlib=<CaselessStrEnum> (InteractiveShellApp.matplotlib)
Default: None
Choices: ['auto', 'gtk', 'gtk3', 'inline', 'nbagg', 'notebook', 'osx', 'qt', 'qt4', 'qt5', 'tk', 'wx']
Configure matplotlib for interactive use with the default matplotlib
backend.
To see all available configurables, use `--help-all`
Examples
--------
ipython --matplotlib # enable matplotlib integration
ipython --matplotlib=qt # enable matplotlib integration with qt4 backend
ipython --log-level=DEBUG # set logging to DEBUG
ipython --profile=foo # start with profile foo
ipython profile create foo # create profile foo w/ default config files
ipython help profile # show the help for the profile subcmd
ipython locate # print the path to the IPython directory
ipython locate profile foo # print the path to the directory for profile `foo`
We could regex for stuff expected to be found
There was a problem hiding this comment.
That's a better idea. We could regex for the first line contents, which are very different.
ccordoba12
changed the title
| valid = str(proc.communicate()[0]) | ||
| if not valid.startswith("b'valid_python_interpreter"): | ||
| if len(valid) > 2 and valid[2:].startswith("usage: {}".format(pyexec)): |
There was a problem hiding this comment.
I think this match is still too weak. Please create a more complete function with some of my previous suggestions, add it to utils/programs.py and also add a test for it in utils/tests/test_programs.py
There was a problem hiding this comment.
And make sure to test it on Py27, py24 py34 and py35
There was a problem hiding this comment.
Especially, I'd like to see a validation for the selected file being a binary
|
@ccordoba12 I already make all the changes you suggested except for the tests in utils/tests/test_programs.py. I did test it on my computer because I knew the paths of real and fake python interpreters, but in this one I'd have the issue of knowing which file to use. I thought about using the python interpreter that runs spyder, if you think this is correct, is there a function already in spyder that can tell me which python is running spyder? Thank you! |
| if not osp.isfile(real_filename) or encoding.is_text_file(real_filename): | ||
| return False | ||
| possible_names = ['python', 'python2', 'python2.7', 'python3', 'python3.4', | ||
| 'python3.5'] |
There was a problem hiding this comment.
Python 3.6 is out, should we have a more robust thing moving forward?, otherwise we will need to update this every now and then (which seems something we can easily forget, even if not frequent)
There was a problem hiding this comment.
Maybe using a regex is better
r'python(?P<version>\d\.?\d*)?$'
This will match any of the options plus any version.. then we can check that version part (if not empty) is a float.
Thoughts @ccordoba12 ?
There was a problem hiding this comment.
True, we need to be smarter about this list. What if create a simple regexp that checks for python3.*?
There was a problem hiding this comment.
Well, this is a generic one, so we don't have to have both a list with options and a regex
r'python(?P<version>\d\.?\d*)?$'
There is someone doing a python2.8 out there also ;-)
There was a problem hiding this comment.
mariacamilaremolinagutierrez in order to test this, we need to break this into a small helper function
def is_python_interpreter_valid_name(fname):
"""Check that the python interpreter file has a valid name."""
pattern = r'python(?P<version>\d\.?\d*)?$'
# etc......That way we can test this method by itself to make sure it is grabbing the right stuff.
| @@ -462,6 +462,24 @@ def is_module_installed(module_name, version=None, installed_version=None, | |||
|
|
|||
| return check_version(actver, version, symb) | |||
|
|
|||
| def is_python_interpreter(filename): | |||
| """Evaluate wether a file is a python interpreter or not.""" | |||
| real_filename = os.path.realpath(filename) # To follow symlink if existent | |||
There was a problem hiding this comment.
There should be 2 spaces after code finishes and a comment starts.
real_filename = os.path.realpath(filename) # To follow symlink if existent
| return False | ||
| try: | ||
| proc = run_program(filename, ["-h"]) | ||
| valid = str(proc.communicate()[0]) |
There was a problem hiding this comment.
Is this a safe Py27 and Py3 way of doing this?, to_text_string? @ccordoba12 ?
There was a problem hiding this comment.
Yeah, please use to_text_string instead of str here :-)
|
@mariacamilaremolinagutierrez, the result is really nice! Thanks a lot for the extra effort ;-) About the tests, please create several of them:
You must skip those four tests if the |
| filename = "$HOME/miniconda/bin/python" | ||
| valid = is_python_interpreter(filename) | ||
|
|
||
| assert valid == True |
There was a problem hiding this comment.
I'd write assert is_python_interpreter(filename) or assert not is_python_interpreter(filename), depending on whether it should be True or False.
There was a problem hiding this comment.
Yep, even more concise!
Happy new year @jitseniesen :-) 🎉
There was a problem hiding this comment.
Also the best wishes for 2017 to you, @goanpeca! and to whoever else reads this 🍺 🎈
There was a problem hiding this comment.
Happy new year to you too @jitseniesen! Best wishes to you and your family for 2017 ;-)
There was a problem hiding this comment.
Awesome, thanks a lot to the 3 of you and have a wonderful 2017 ❤️
| filename = "$HOME/miniconda/bin/ipython" | ||
| valid = is_python_interpreter(filename) | ||
|
|
||
| assert valid == False |
| filename = "%PYTHON%\bin\python.exe" | ||
| valid = is_python_interpreter(filename) | ||
|
|
||
| assert valid == True |
| filename = "%PYTHON%\Scripts\ipython.exe" | ||
| valid = is_python_interpreter(filename) | ||
|
|
||
| assert valid == False |
| @@ -36,7 +37,39 @@ def test_run_python_script_in_terminal_with_wdir_empty(tmpdir, qtbot): | |||
| res = outfilepath.read() | |||
| assert res == 'done' | |||
|
|
|||
|
|
|||
| @pytest.mark.skipif(os.environ.get('TEST_CI_WIDGETS', None) == True, | |||
There was a problem hiding this comment.
We don't set TEST_CI_WIDGETS in CircleCI because we don't test our widgets there. That's why I told you to use the CI variable instead, which is set automatically by CircleCI, Travis and AppVeyor regardless of what we do in those services.
| @@ -36,7 +37,39 @@ def test_run_python_script_in_terminal_with_wdir_empty(tmpdir, qtbot): | |||
| res = outfilepath.read() | |||
| assert res == 'done' | |||
|
|
|||
|
|
|||
| @pytest.mark.skipif(os.environ.get('TEST_CI_WIDGETS', None) == True, | |||
| reason='To only run the test in spyders CI services.') | |||
There was a problem hiding this comment.
Change this to
It only runs in CI services.
|
|
||
| @pytest.mark.skipif(os.environ.get('TEST_CI_WIDGETS', None) == True, | ||
| reason='To only run the test in spyders CI services.') | ||
| def test_is_python_interpreter_linux_true(): |
There was a problem hiding this comment.
Simplify this name like this
test_is_valid_interpreter
|
|
||
| @pytest.mark.skipif(os.environ.get('TEST_CI_WIDGETS', None) == True, | ||
| reason='To only run the test in spyders CI services.') | ||
| def test_is_python_interpreter_linux_false(): |
There was a problem hiding this comment.
Please change this name to
test_is_invalid_interpreter
|
@mariacamilaremolinagutierrez, there are some details to polish but this on the right track! I'd suggest that instead of having four tests (which, I know, I suggested at the beginning) we could only have two, like this: test_is_valid_interpreter():
if os.name == 'nt':
# Windows code here
else:
# Linux code hereand test_is_invalid_interpreter():
if os.name == 'nt':
# Windows code here
else:
# Linux code here |
|
Or tests could be even simpler. At the beginning of if os.name == 'nt':
VALID_INTERPRETER = '%PYTHON%\...'
INVALID_INTERPRETER = '%PYTHON%\Scripts\...'
else:
VALID_INTERPRETER = '$HOME/...'
INVALID_INTERPRETER = '$HOME/...'
def test_is_valid_interpreter()
assert is_python_interpreter(VALID_INTERPRETER) |
| is_python_interpreter) | ||
|
|
||
| if os.name == 'nt': | ||
| VALID_INTERPRETER = r'%PYTHON%\bin\python.exe' |
There was a problem hiding this comment.
Sorry, my code was a simple draft of what you should do, not the right solution (that's why tests are still failing :-). So this should be
python_dir = os.environ['PYTHON']
VALID_INTERPRETER = os.path.join(python_dir, 'bin', 'python.exe')There was a problem hiding this comment.
Sorry, small correction (also please fix all what's needed until tests pass ;-)
python_dir = os.environ['PYTHON']
VALID_INTERPRETER = os.path.join(python_dir, 'python.exe')
| VALID_INTERPRETER = r'%PYTHON%\bin\python.exe' | ||
| INVALID_INTERPRETER = r'%PYTHON%\Scripts\ipython.exe' | ||
| else: | ||
| VALID_INTERPRETER = r'$HOME/miniconda/bin/python' |
There was a problem hiding this comment.
And this needs to be (please check it until tests pass)
home = os.environ['HOME']
VALID_INTERPRETER = os.path.join(home, 'miniconda', 'bin', 'python')
|
@mariacamilaremolinagutierrez, please don't touch this branch. I'm working on it to finish this PR. |
ccordoba12
force-pushed
the
fixes_issue_3756
branch
from
f9c28a1 to
8492931
Compare
ccordoba12
changed the title
| def test_is_python_interpreter_windows_false(): | ||
| filename = "%PYTHON%\Scripts\ipython.exe" | ||
| valid = is_python_interpreter(filename) | ||
| @pytest.mark.skipif(os.environ.get('CI', None) == True, |
There was a problem hiding this comment.
@mariacamilaremolinagutierrez Is this the right condition? It appears that the test will be skipped if CI is set to True, but I think you want the test to run when CI is set to True.
There was a problem hiding this comment.
The problem is these tests are running locally (which is what I wanted to prevent)
@mariacamilaremolinagutierrez, please fix that.
Fixes #3756