Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed mechanism that change $PATH #148

Merged
merged 5 commits into from
Jun 19, 2019
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 43 additions & 19 deletions jupyter_core/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,25 @@
import sys
from subprocess import Popen

try:
# py3
from shutil import which
except ImportError:
from .utils.shutil_which import which

from . import paths
from .version import __version__

class JupyterParser(argparse.ArgumentParser):

@property
def epilog(self):
"""Add subcommands to epilog on request

Avoids searching PATH for subcommands unless help output is requested.
"""
return 'Available subcommands: %s' % ' '.join(list_subcommands())

@epilog.setter
def epilog(self, x):
"""Ignore epilog set in Parser.__init__"""
Expand All @@ -42,7 +48,7 @@ def jupyter_parser():
group.add_argument('--version', action='store_true',
help="show the jupyter command's version and exit")
group.add_argument('subcommand', type=str, nargs='?', help='the subcommand to launch')

group.add_argument('--config-dir', action='store_true',
help="show Jupyter config dir")
group.add_argument('--data-dir', action='store_true',
Expand All @@ -53,15 +59,15 @@ def jupyter_parser():
help="show all Jupyter paths. Add --json for machine-readable format.")
parser.add_argument('--json', action='store_true',
help="output paths as machine-readable json")

return parser


def list_subcommands():
"""List all jupyter subcommands

searches PATH for `jupyter-name`

Returns a list of jupyter's subcommand names, without the `jupyter-` prefix.
Nested children (e.g. jupyter-sub-subsub) are not included.
"""
Expand Down Expand Up @@ -89,7 +95,7 @@ def list_subcommands():

def _execvp(cmd, argv):
"""execvp, except on Windows where it uses Popen

Python provides execvp on Windows, but its behavior is problematic (Python bug#9148).
"""
if sys.platform.startswith('win'):
Expand All @@ -113,9 +119,29 @@ def _execvp(cmd, argv):
os.execvp(cmd, argv)


def _jupyter_abspath(path, subcommand):
"""This method get the abspath of jupyter with no changes on ENV."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: "...the abspath of a specified jupyter-subcommand..."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok! done! thanks for the feedback!

# get env PATH with self
search_path = os.pathsep.join(_path_with_self())
# get the abs path for the jupyter-<subcommand>
jupyter_subcommand = '{}-{}'.format(path, subcommand)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Now you don't need to pass an argument, you can just hardcode it in this line. That's one less thing to follow around when reading the code. :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok :) done!

abs_path = which(jupyter_subcommand, path=search_path)
if abs_path is None:
raise Exception(
'Jupyter command `{}` not found.'.format(jupyter_subcommand)
)

if not os.access(abs_path, os.X_OK):
raise Exception(
'Jupyter command `{}` is not executable.'.format(jupyter_subcommand)
)

return abs_path


def _path_with_self():
"""Put `jupyter`'s dir at the front of PATH

Ensures that /path/to/jupyter subcommand
will do /path/to/jupyter-subcommand
even if /other/jupyter-subcommand is ahead of it on PATH
Expand All @@ -128,18 +154,17 @@ def _path_with_self():
path_list = (os.environ.get('PATH') or os.defpath).split(os.pathsep)
for script in scripts:
bindir = os.path.dirname(script)
if (os.path.isdir(bindir)
and os.access(script, os.X_OK) # only if it's a script
if (
os.path.isdir(bindir)
and os.access(script, os.X_OK) # only if it's a script
):
# ensure executable's dir is on PATH
# avoids missing subcommands when jupyter is run via absolute path
path_list.insert(0, bindir)
os.environ['PATH'] = os.pathsep.join(path_list)
return path_list


def main():
_path_with_self() # ensure executable is on PATH
if len(sys.argv) > 1 and not sys.argv[1].startswith('-'):
# Don't parse if a subcommand is given
# Avoids argparse gobbling up args passed to subcommand, such as `-h`.
Expand All @@ -150,7 +175,7 @@ def main():
subcommand = args.subcommand
if args.version:
print('{:<17}:'.format('jupyter core'), __version__)
for package, name in [
for package, name in [
('notebook', 'jupyter-notebook'),
('qtconsole', 'qtconsole'),
('IPython', 'ipython'),
Expand All @@ -169,8 +194,6 @@ def main():
except ImportError:
version = 'not installed'
print('{:<17}:'.format(name), version)


return
if args.json and not args.paths:
sys.exit("--json is only used with --paths")
Expand All @@ -197,12 +220,13 @@ def main():
for p in path:
print(' ' + p)
return

if not subcommand:
parser.print_usage(file=sys.stderr)
sys.exit("subcommand is required")

command = 'jupyter-' + subcommand

command = _jupyter_abspath('jupyter', subcommand)

try:
_execvp(command, sys.argv[1:])
except OSError as e:
Expand Down