From 252be3257823fd13e722c8aa8c944be898fc37bb Mon Sep 17 00:00:00 2001 From: Max Krivich Date: Wed, 20 Jun 2018 13:17:13 +0300 Subject: [PATCH 1/6] Fix cli option usage error Fix IndexError exception when `more_packages` is empty and add the more informal message for argument usage. To reproduce this issue `pipenv install -e` --- pipenv/core.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pipenv/core.py b/pipenv/core.py index be67c8f5..faa54807 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -1889,6 +1889,8 @@ def do_install( # Capture -e argument and assign it to following package_name. more_packages = list(more_packages) if package_name == '-e': + if not more_packages: + raise click.BadArgumentUsage('Please provide path to setup.py') package_name = ' '.join([package_name, more_packages.pop(0)]) # capture indexes and extra indexes line = [package_name] + more_packages From ebb07002d2399f8c22e703be05a4ee9b3108d886 Mon Sep 17 00:00:00 2001 From: Max Krivich Date: Wed, 20 Jun 2018 13:51:03 +0300 Subject: [PATCH 2/6] Fix empty indexes in cli param Add extra check for -i option for fix `AttributeError: 'NoneType'` To reproduce this bug `pipenv install -i` --- pipenv/core.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pipenv/core.py b/pipenv/core.py index faa54807..d98ca6e4 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -1897,6 +1897,8 @@ def do_install( index_indicators = ['-i', '--index', '--extra-index-url'] index, extra_indexes = None, None if more_packages and any(more_packages[0].startswith(s) for s in index_indicators): + if len(more_packages) < 2: + raise click.BadArgumentUsage('Please provide index value') line, index = split_argument(' '.join(line), short='i', long_='index', num=1) line, extra_indexes = split_argument(line, long_='extra-index-url') package_names = line.split() From 64c5ad9907a8437550e6a4ca703e527799bb6959 Mon Sep 17 00:00:00 2001 From: Max Krivich Date: Wed, 20 Jun 2018 16:43:24 +0300 Subject: [PATCH 3/6] Change exception message --- pipenv/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pipenv/core.py b/pipenv/core.py index d98ca6e4..3f8cdc17 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -1890,7 +1890,7 @@ def do_install( more_packages = list(more_packages) if package_name == '-e': if not more_packages: - raise click.BadArgumentUsage('Please provide path to setup.py') + raise click.BadArgumentUsage('Please provide path to editable package') package_name = ' '.join([package_name, more_packages.pop(0)]) # capture indexes and extra indexes line = [package_name] + more_packages From 0353b81b98a4fddf818c47b58d421246f86b6cf9 Mon Sep 17 00:00:00 2001 From: Max Krivich Date: Wed, 20 Jun 2018 17:54:20 +0300 Subject: [PATCH 4/6] Update too complex if statement Quick refactor for improve readability of code --- pipenv/core.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pipenv/core.py b/pipenv/core.py index 3f8cdc17..0dcc7ed2 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -1893,13 +1893,14 @@ def do_install( raise click.BadArgumentUsage('Please provide path to editable package') package_name = ' '.join([package_name, more_packages.pop(0)]) # capture indexes and extra indexes - line = [package_name] + more_packages + line = ' '.join([package_name] + more_packages).strip() index_indicators = ['-i', '--index', '--extra-index-url'] index, extra_indexes = None, None - if more_packages and any(more_packages[0].startswith(s) for s in index_indicators): - if len(more_packages) < 2: - raise click.BadArgumentUsage('Please provide index value') - line, index = split_argument(' '.join(line), short='i', long_='index', num=1) + if any(line.endswith(s) for s in index_indicators): + # check if cli option is not end of command + raise click.BadArgumentUsage('Please provide index value') + if any(s in line for s in index_indicators): + line, index = split_argument(line, short='i', long_='index', num=1) line, extra_indexes = split_argument(line, long_='extra-index-url') package_names = line.split() package_name = package_names[0] From 0ef4929098fa1df236ed8ba63f0572900d4795ea Mon Sep 17 00:00:00 2001 From: Max Krivich Date: Wed, 20 Jun 2018 18:24:36 +0300 Subject: [PATCH 5/6] Fix CI errors --- pipenv/core.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pipenv/core.py b/pipenv/core.py index 0dcc7ed2..d97e89a7 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -1893,7 +1893,8 @@ def do_install( raise click.BadArgumentUsage('Please provide path to editable package') package_name = ' '.join([package_name, more_packages.pop(0)]) # capture indexes and extra indexes - line = ' '.join([package_name] + more_packages).strip() + line = [package_name] + more_packages + line = ' '.join(str(s) for s in line).strip() index_indicators = ['-i', '--index', '--extra-index-url'] index, extra_indexes = None, None if any(line.endswith(s) for s in index_indicators): From c76b95db34b8aecae63636695b17961736a7e8d9 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Fri, 22 Jun 2018 02:04:37 -0400 Subject: [PATCH 6/6] Add test for `install -e` Signed-off-by: Dan Ryan --- news/2388.feature | 1 + tests/integration/test_install_basic.py | 10 ++++++++++ 2 files changed, 11 insertions(+) create mode 100644 news/2388.feature diff --git a/news/2388.feature b/news/2388.feature new file mode 100644 index 00000000..2286baf7 --- /dev/null +++ b/news/2388.feature @@ -0,0 +1 @@ +Massive internal improvements to requirements parsing codebase, resolver, and error messaging. diff --git a/tests/integration/test_install_basic.py b/tests/integration/test_install_basic.py index bd0b96c3..a6798aa3 100644 --- a/tests/integration/test_install_basic.py +++ b/tests/integration/test_install_basic.py @@ -299,3 +299,13 @@ name = 'mockpi' assert c.return_code == 0 assert p.pipfile['source'][0]['url'] == '${PYPI_URL}/simple' assert p.lockfile['_meta']['sources'][0]['url'] == '${PYPI_URL}/simple' + + +@pytest.mark.editable +@pytest.mark.badparameter +@pytest.mark.install +def test_editable_no_args(PipenvInstance): + with PipenvInstance() as p: + c = p.pipenv('install -e') + assert c.return_code != 0 + assert 'Please provide path to editable package' in c.err