From e87e2ceb059d0d555714eceba69f4dfd221bd960 Mon Sep 17 00:00:00 2001 From: Dave Schaefer Date: Wed, 4 Oct 2023 21:35:16 -0600 Subject: [PATCH 01/12] Sort packages alphabetically inside each category When installing any package, sort all package names alphabetically inside the category, for easier reading. Unsure if this is the best place or way to implement. Small prototype to add to discussion in https://github.com/pypa/pipenv/issues/5964 Tests: before patch: ``` AssertionError: assert ['atomicwrite...ama', 'build'] == ['atomicwrite...', 'colorama'] At index 1 diff: 'colorama' != 'build' Full diff: - ['atomicwrites', 'build', 'colorama'] ? --------- + ['atomicwrites', 'colorama', 'build'] ? +++++++++ ``` after patch: pass. --- pipenv/project.py | 1 + tests/integration/test_install_basic.py | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/pipenv/project.py b/pipenv/project.py index b96487c108..cb34f689db 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -1226,6 +1226,7 @@ def add_pipfile_entry_to_pipfile(self, name, normalized_name, entry, category=No newly_added = True p[category][normalized_name] = entry + p[category] = dict(sorted(p[category].items())) # Write Pipfile. self.write_toml(p) diff --git a/tests/integration/test_install_basic.py b/tests/integration/test_install_basic.py index 02d5b855d1..bf4d2f13f9 100644 --- a/tests/integration/test_install_basic.py +++ b/tests/integration/test_install_basic.py @@ -556,3 +556,20 @@ def test_install_tarball_is_actually_installed(pipenv_instance_pypi): assert c.returncode == 0 c = p.pipenv("""run python -c "from dataclasses_json import dataclass_json" """) assert c.returncode == 0 + +@pytest.mark.basic +@pytest.mark.install +def test_packages_sorted_alphabetically_in_category(pipenv_instance_private_pypi): + with pipenv_instance_private_pypi() as p: + with open(p.pipfile_path, "w") as f: + contents = """ +[packages] +atomicwrites = "*" +colorama = "*" + """.strip() + f.write(contents) + c = p.pipenv("install build") + assert c.returncode == 0 + assert "build" in p.pipfile["packages"] + assert list(p.pipfile["packages"].keys()) == ["atomicwrites", "build", "colorama"] + From 8e189035a39f169c7c343d1ec54c96a48c654195 Mon Sep 17 00:00:00 2001 From: Dave Schaefer Date: Sat, 7 Oct 2023 11:01:53 -0600 Subject: [PATCH 02/12] Added new Pipenv option `sort_alphabetical` Sort packages alphabetically inside each category. Currently runs on `install`. --- pipenv/project.py | 4 +++- tests/integration/test_install_basic.py | 26 ++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/pipenv/project.py b/pipenv/project.py index cb34f689db..43b1bc12ff 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -1226,7 +1226,9 @@ def add_pipfile_entry_to_pipfile(self, name, normalized_name, entry, category=No newly_added = True p[category][normalized_name] = entry - p[category] = dict(sorted(p[category].items())) + + if self.settings.get("sort_alphabetical"): + p[category] = dict(sorted(p[category].items())) # Write Pipfile. self.write_toml(p) diff --git a/tests/integration/test_install_basic.py b/tests/integration/test_install_basic.py index bf4d2f13f9..856809785d 100644 --- a/tests/integration/test_install_basic.py +++ b/tests/integration/test_install_basic.py @@ -557,12 +557,16 @@ def test_install_tarball_is_actually_installed(pipenv_instance_pypi): c = p.pipenv("""run python -c "from dataclasses_json import dataclass_json" """) assert c.returncode == 0 + @pytest.mark.basic @pytest.mark.install -def test_packages_sorted_alphabetically_in_category(pipenv_instance_private_pypi): +def test_category_sorted_alphabetically_with_directive(pipenv_instance_private_pypi): with pipenv_instance_private_pypi() as p: with open(p.pipfile_path, "w") as f: contents = """ +[pipenv] +sort_alphabetical = true + [packages] atomicwrites = "*" colorama = "*" @@ -573,3 +577,23 @@ def test_packages_sorted_alphabetically_in_category(pipenv_instance_private_pypi assert "build" in p.pipfile["packages"] assert list(p.pipfile["packages"].keys()) == ["atomicwrites", "build", "colorama"] + +@pytest.mark.basic +@pytest.mark.install +def test_category_not_sorted_without_directive(pipenv_instance_private_pypi): + with pipenv_instance_private_pypi() as p: + with open(p.pipfile_path, "w") as f: + contents = """ +[packages] +atomicwrites = "*" +colorama = "*" + """.strip() + f.write(contents) + c = p.pipenv("install build") + assert c.returncode == 0 + assert "build" in p.pipfile["packages"] + assert list(p.pipfile["packages"].keys()) == [ + "atomicwrites", + "colorama", + "build", + ] From 9bc4ffa506bed8756470f776dbe82063cece9cfc Mon Sep 17 00:00:00 2001 From: Dave Schaefer Date: Sun, 8 Oct 2023 09:46:45 -0600 Subject: [PATCH 03/12] Rename sorting directive `sort_pipfile` , as requested in https://github.com/pypa/pipenv/pull/5965 --- pipenv/project.py | 2 +- tests/integration/test_install_basic.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pipenv/project.py b/pipenv/project.py index 43b1bc12ff..73dde2d923 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -1227,7 +1227,7 @@ def add_pipfile_entry_to_pipfile(self, name, normalized_name, entry, category=No p[category][normalized_name] = entry - if self.settings.get("sort_alphabetical"): + if self.settings.get("sort_pipfile"): p[category] = dict(sorted(p[category].items())) # Write Pipfile. diff --git a/tests/integration/test_install_basic.py b/tests/integration/test_install_basic.py index 856809785d..2c1a323fc1 100644 --- a/tests/integration/test_install_basic.py +++ b/tests/integration/test_install_basic.py @@ -565,7 +565,7 @@ def test_category_sorted_alphabetically_with_directive(pipenv_instance_private_p with open(p.pipfile_path, "w") as f: contents = """ [pipenv] -sort_alphabetical = true +sort_pipfile = true [packages] atomicwrites = "*" From 7b33b04374e99cc05c0c1f7ad85983a73a5a6bfa Mon Sep 17 00:00:00 2001 From: Dave Schaefer Date: Sun, 8 Oct 2023 09:51:40 -0600 Subject: [PATCH 04/12] Implement pipfile sorting for `uninstall` using sorting directive --- pipenv/project.py | 2 ++ tests/integration/test_uninstall.py | 51 +++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/pipenv/project.py b/pipenv/project.py index 73dde2d923..f9257aa487 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -1117,6 +1117,8 @@ def remove_package_from_pipfile(self, package_name, category): p = self.parsed_pipfile if name: del p[category][name] + if self.settings.get("sort_pipfile"): + p[category] = dict(sorted(p[category].items())) self.write_toml(p) return True return False diff --git a/tests/integration/test_uninstall.py b/tests/integration/test_uninstall.py index f97385982c..576029061f 100644 --- a/tests/integration/test_uninstall.py +++ b/tests/integration/test_uninstall.py @@ -243,3 +243,54 @@ def test_uninstall_multiple_categories(pipenv_instance_private_pypi): assert "six" not in p.lockfile.get("prereq", {}) assert "six" not in p.lockfile["default"] + + +@pytest.mark.uninstall +def test_category_sorted_alphabetically_with_directive(pipenv_instance_private_pypi): + with pipenv_instance_private_pypi() as p: + with open(p.pipfile_path, "w") as f: + contents = """ +[pipenv] +sort_pipfile = true + +[packages] +parse = "*" +colorama = "*" +build = "*" +atomicwrites = "*" + """.strip() + f.write(contents) + + c = p.pipenv("install") + assert c.returncode == 0 + + c = p.pipenv("uninstall build") + assert c.returncode == 0 + assert "build" not in p.pipfile["packages"] + assert list(p.pipfile["packages"].keys()) == ["atomicwrites", "colorama", "parse"] + + +@pytest.mark.uninstall +def test_category_not_sorted_without_directive(pipenv_instance_private_pypi): + with pipenv_instance_private_pypi() as p: + with open(p.pipfile_path, "w") as f: + contents = """ +[packages] +parse = "*" +colorama = "*" +build = "*" +atomicwrites = "*" + """.strip() + f.write(contents) + + c = p.pipenv("install") + assert c.returncode == 0 + + c = p.pipenv("uninstall build") + assert c.returncode == 0 + assert "build" not in p.pipfile["packages"] + assert list(p.pipfile["packages"].keys()) == [ + "parse", + "colorama", + "atomicwrites", + ] From 773fcb519de921239ae6f16f2b427e5c767d5573 Mon Sep 17 00:00:00 2001 From: Dave Schaefer Date: Sun, 8 Oct 2023 10:32:24 -0600 Subject: [PATCH 05/12] Add docs for `[pipenv]` directives Based on notes from existing code and release docs Note that `keep_outdated` has been discontinued, so no docs added for it. --- docs/pipfile.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/pipfile.md b/docs/pipfile.md index fa4e4d41a8..a260fa9dc6 100644 --- a/docs/pipfile.md +++ b/docs/pipfile.md @@ -10,6 +10,15 @@ This file is managed automatically through locking actions. You should add both `Pipfile` and `Pipfile.lock` to the project's source control. +## `[pipenv]` Directives + +`Pipfile` may contain a `[pipenv]` section to control the behaviour of pipenv itself. Some available settings include: + +* `allow_prereleases` - Tell pipenv to install pre-release versions of a package -i.e. a version with an alpha/beta/etc. suffix, such as _1.0b1_. Equivalent to passing the `--pre` flag on the command line. +* `disable_pip_input` - Prevent pipenv from asking for input. Equivalent to the `--no-input` flag. +* `install_search_all_sources` - Allow installation of packages from an existing `Pipfile.lock` to search all defined indexes for the constrained package version and hash signatures. See [Specifying Package Indexes](indexes.md). + + ## Example Pipfile Here is a simple example of a `Pipfile` and the resulting `Pipfile.lock`. From f2b7b3a8231cd6458b4f6f20041d00050bb2569b Mon Sep 17 00:00:00 2001 From: Dave Schaefer Date: Sun, 8 Oct 2023 10:39:33 -0600 Subject: [PATCH 06/12] Add doc for new sorting directive --- docs/pipfile.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/pipfile.md b/docs/pipfile.md index a260fa9dc6..e7f231b6bf 100644 --- a/docs/pipfile.md +++ b/docs/pipfile.md @@ -17,6 +17,7 @@ You should add both `Pipfile` and `Pipfile.lock` to the project's source control * `allow_prereleases` - Tell pipenv to install pre-release versions of a package -i.e. a version with an alpha/beta/etc. suffix, such as _1.0b1_. Equivalent to passing the `--pre` flag on the command line. * `disable_pip_input` - Prevent pipenv from asking for input. Equivalent to the `--no-input` flag. * `install_search_all_sources` - Allow installation of packages from an existing `Pipfile.lock` to search all defined indexes for the constrained package version and hash signatures. See [Specifying Package Indexes](indexes.md). +* `sort_pipfile` - Sort package names alphabetically inside each category. Categories will be sorted and updated on `install` and `uninstall`. This is purely cosmetic to make reading easier for humans, and has no effect on installation order or dependency resolution. Note that `Pipfile.lock` packages are always sorted alphabetically. ## Example Pipfile From 4de4d20b744ee51363454dec3de6607603ea5987 Mon Sep 17 00:00:00 2001 From: Dave Schaefer Date: Sun, 8 Oct 2023 10:48:26 -0600 Subject: [PATCH 07/12] mark uninstall tests with `install` since the test calls `install` while running. as discussed in https://github.com/pypa/pipenv/pull/5965 --- tests/integration/test_uninstall.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/test_uninstall.py b/tests/integration/test_uninstall.py index 576029061f..f83e5ae947 100644 --- a/tests/integration/test_uninstall.py +++ b/tests/integration/test_uninstall.py @@ -245,6 +245,7 @@ def test_uninstall_multiple_categories(pipenv_instance_private_pypi): assert "six" not in p.lockfile["default"] +@pytest.mark.install @pytest.mark.uninstall def test_category_sorted_alphabetically_with_directive(pipenv_instance_private_pypi): with pipenv_instance_private_pypi() as p: @@ -270,6 +271,7 @@ def test_category_sorted_alphabetically_with_directive(pipenv_instance_private_p assert list(p.pipfile["packages"].keys()) == ["atomicwrites", "colorama", "parse"] +@pytest.mark.install @pytest.mark.uninstall def test_category_not_sorted_without_directive(pipenv_instance_private_pypi): with pipenv_instance_private_pypi() as p: From 4010f5d197371fc95fc1c9549d999399efe92d3a Mon Sep 17 00:00:00 2001 From: Dave Schaefer Date: Sun, 8 Oct 2023 10:52:05 -0600 Subject: [PATCH 08/12] Mark `install` for other tests that install Since the goal of the mark is to note which tests use it. --- tests/integration/test_uninstall.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/integration/test_uninstall.py b/tests/integration/test_uninstall.py index f83e5ae947..b43eb4e60e 100644 --- a/tests/integration/test_uninstall.py +++ b/tests/integration/test_uninstall.py @@ -143,6 +143,7 @@ def test_uninstall_all_dev(pipenv_instance_private_pypi): assert c.returncode == 0 +@pytest.mark.install @pytest.mark.uninstall def test_normalize_name_uninstall(pipenv_instance_private_pypi): with pipenv_instance_private_pypi() as p: @@ -188,6 +189,7 @@ def test_uninstall_all_dev_with_shared_dependencies(pipenv_instance_pypi): assert "six" in p.lockfile["default"] +@pytest.mark.install @pytest.mark.uninstall def test_uninstall_missing_parameters(pipenv_instance_private_pypi): with pipenv_instance_private_pypi() as p: @@ -200,6 +202,7 @@ def test_uninstall_missing_parameters(pipenv_instance_private_pypi): @pytest.mark.categories +@pytest.mark.install @pytest.mark.uninstall def test_uninstall_category_with_shared_requirement(pipenv_instance_pypi): with pipenv_instance_pypi() as p: @@ -223,6 +226,7 @@ def test_uninstall_category_with_shared_requirement(pipenv_instance_pypi): @pytest.mark.categories +@pytest.mark.install @pytest.mark.uninstall def test_uninstall_multiple_categories(pipenv_instance_private_pypi): with pipenv_instance_private_pypi() as p: From 6a5cd68a2883aa6f68f4563e51034dc42ff38fa6 Mon Sep 17 00:00:00 2001 From: Dave Schaefer Date: Tue, 10 Oct 2023 20:54:50 -0600 Subject: [PATCH 09/12] Move common sorting code to function --- pipenv/project.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pipenv/project.py b/pipenv/project.py index f9257aa487..512f36446e 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -1111,6 +1111,9 @@ def get_package_name_in_pipfile(self, package_name, category): return name return None + def _sort_category(self, category): + return dict(sorted(category.items())) + def remove_package_from_pipfile(self, package_name, category): # Read and append Pipfile. name = self.get_package_name_in_pipfile(package_name, category=category) @@ -1118,7 +1121,7 @@ def remove_package_from_pipfile(self, package_name, category): if name: del p[category][name] if self.settings.get("sort_pipfile"): - p[category] = dict(sorted(p[category].items())) + p[category] = self._sort_category(p[category]) self.write_toml(p) return True return False @@ -1230,7 +1233,7 @@ def add_pipfile_entry_to_pipfile(self, name, normalized_name, entry, category=No p[category][normalized_name] = entry if self.settings.get("sort_pipfile"): - p[category] = dict(sorted(p[category].items())) + p[category] = self._sort_category(p[category]) # Write Pipfile. self.write_toml(p) From 966afee8130eb35966c42048ac9ea92d17fb5a6a Mon Sep 17 00:00:00 2001 From: Dave Schaefer Date: Tue, 10 Oct 2023 21:03:25 -0600 Subject: [PATCH 10/12] Add tests for sorting str and dict values Currently these fail. Will be fixed shortly in the next patch. Pipfiles can contain different formats for package specifications. Current default behaivour is to sort packages into groups - all string values will be sorted first, followed by all dictionary values. e.g. ``` aa = "*" bb = "*" cc = "*" aaa = {version = "*"} bbb = {version = "*"} ccc = {version = "*"} ``` This will have to be fixed. --- tests/integration/test_install_basic.py | 28 +++++++++++++++++++++++ tests/integration/test_uninstall.py | 30 +++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/tests/integration/test_install_basic.py b/tests/integration/test_install_basic.py index 2c1a323fc1..0e52be906c 100644 --- a/tests/integration/test_install_basic.py +++ b/tests/integration/test_install_basic.py @@ -578,6 +578,34 @@ def test_category_sorted_alphabetically_with_directive(pipenv_instance_private_p assert list(p.pipfile["packages"].keys()) == ["atomicwrites", "build", "colorama"] +@pytest.mark.basic +@pytest.mark.install +def test_sorting_handles_str_values_and_dict_values(pipenv_instance_private_pypi): + with pipenv_instance_private_pypi() as p: + with open(p.pipfile_path, "w") as f: + contents = """ +[pipenv] +sort_pipfile = true + +[packages] +zipp = {version = "*"} +parse = "*" +colorama = "*" +atomicwrites = {version = "*"} + """.strip() + f.write(contents) + c = p.pipenv("install build") + assert c.returncode == 0 + assert "build" in p.pipfile["packages"] + assert list(p.pipfile["packages"].keys()) == [ + "atomicwrites", + "build", + "colorama", + "parse", + "zipp", + ] + + @pytest.mark.basic @pytest.mark.install def test_category_not_sorted_without_directive(pipenv_instance_private_pypi): diff --git a/tests/integration/test_uninstall.py b/tests/integration/test_uninstall.py index b43eb4e60e..017a712042 100644 --- a/tests/integration/test_uninstall.py +++ b/tests/integration/test_uninstall.py @@ -275,6 +275,36 @@ def test_category_sorted_alphabetically_with_directive(pipenv_instance_private_p assert list(p.pipfile["packages"].keys()) == ["atomicwrites", "colorama", "parse"] +@pytest.mark.install +@pytest.mark.uninstall +def test_sorting_handles_str_values_and_dict_values(pipenv_instance_private_pypi): + with pipenv_instance_private_pypi() as p: + with open(p.pipfile_path, "w") as f: + contents = """ +[pipenv] +sort_pipfile = true + +[packages] +zipp = "*" +parse = {version = "*"} +colorama = "*" +build = "*" +atomicwrites = {version = "*"} + """.strip() + f.write(contents) + c = p.pipenv("install") + assert c.returncode == 0 + + c = p.pipenv("uninstall build") + assert c.returncode == 0 + assert "build" not in p.pipfile["packages"] + assert list(p.pipfile["packages"].keys()) == [ + "atomicwrites", + "colorama", + "parse", + "zipp", + ] + @pytest.mark.install @pytest.mark.uninstall def test_category_not_sorted_without_directive(pipenv_instance_private_pypi): From 4a292501c881f21b91a49d6819c874c16d1b2e90 Mon Sep 17 00:00:00 2001 From: Dave Schaefer Date: Tue, 10 Oct 2023 21:09:14 -0600 Subject: [PATCH 11/12] Created sort category toml tables This is not as clean as only working with dicts. `p[category]`, the parsed pipfile category, is already of type `tomlkit.items.Table` when it is passed to `_sort_category()`. Currently this is only constructing the table right before data is sent to `write_tom()`. --- pipenv/project.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pipenv/project.py b/pipenv/project.py index 512f36446e..c5173c93fb 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -1112,7 +1112,14 @@ def get_package_name_in_pipfile(self, package_name, category): return None def _sort_category(self, category): - return dict(sorted(category.items())) + # toml tables won't maintain sorted dictionary order + # so construct the table in the order that we need + sorted_category = dict(sorted(category.items())) + table = tomlkit.table() + for key, value in sorted_category.items(): + table.add(key, value) + + return table def remove_package_from_pipfile(self, package_name, category): # Read and append Pipfile. From 4802425c86d8d444b5a1fd59918a3f9af8401bbe Mon Sep 17 00:00:00 2001 From: Dave Schaefer Date: Tue, 10 Oct 2023 21:11:21 -0600 Subject: [PATCH 12/12] Add upgrade tests --- tests/integration/test_upgrade.py | 52 +++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/integration/test_upgrade.py diff --git a/tests/integration/test_upgrade.py b/tests/integration/test_upgrade.py new file mode 100644 index 0000000000..c72e29ccf7 --- /dev/null +++ b/tests/integration/test_upgrade.py @@ -0,0 +1,52 @@ +import pytest + + +@pytest.mark.upgrade +def test_category_sorted_alphabetically_with_directive(pipenv_instance_private_pypi): + with pipenv_instance_private_pypi() as p: + with open(p.pipfile_path, "w") as f: + contents = """ +[pipenv] +sort_pipfile = true + +[packages] +zipp = "*" +six = 1.11 +colorama = "*" +atomicwrites = "*" + """.strip() + f.write(contents) + + package_name = "six" + c = p.pipenv(f"upgrade {package_name}") + assert c.returncode == 0 + assert list(p.pipfile["packages"].keys()) == [ + "atomicwrites", + "colorama", + "six", + "zipp", + ] + + +@pytest.mark.upgrade +def test_category_not_sorted_without_directive(pipenv_instance_private_pypi): + with pipenv_instance_private_pypi() as p: + with open(p.pipfile_path, "w") as f: + contents = """ +[packages] +zipp = "*" +six = 1.11 +colorama = "*" +atomicwrites = "*" + """.strip() + f.write(contents) + + package_name = "six" + c = p.pipenv(f"upgrade {package_name}") + assert c.returncode == 0 + assert list(p.pipfile["packages"].keys()) == [ + "zipp", + "six", + "colorama", + "atomicwrites", + ]