Repository: pdm-project/pdm
Pull request #2670 (2024-03-07 13:13:17)

fix/group exclusion
## Pull Request Checklist

- [X] A news fragment is added in `news/` describing what is new.
- [X] Test cases added for changed code.

## Describe what you have changed in this PR.

Fixes unintuitive behaviour by making `--without` imply `--with :all` if no explicit groups given.

What should we do in the case of `--with foo --without foo`?  Currently if I read the logic right we silently drop it.


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/pdm-project/pdm/pull/2670?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pdm-project) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 84.84%. Comparing base [(`1bf5eb1`)](https://app.codecov.io/gh/pdm-project/pdm/commit/1bf5eb1d79304bce9c4d4a813f20470ae4411de4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pdm-project) to head [(`08c7600`)](https://app.codecov.io/gh/pdm-project/pdm/pull/2670?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pdm-project).


<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##             main    #2670   +/-   ##
=======================================
  Coverage   84.84%   84.84%           
=======================================
  Files         104      104           
  Lines       10293    10295    +2     
  Branches     2235     2236    +1     
=======================================
+ Hits         8733     8735    +2     
  Misses       1099     1099           
  Partials      461      461           
```

| [Flag](https://app.codecov.io/gh/pdm-project/pdm/pull/2670/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pdm-project) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/pdm-project/pdm/pull/2670/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pdm-project) | `84.63% <100.00%> (+<0.01%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pdm-project#carryforward-flags-in-the-pull-request-comment) to find out more.


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/pdm-project/pdm/pull/2670?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pdm-project).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pdm-project).

frostming: What groups will be installed if I run `pdm install —prod —without abc`?
2e0byo: As I understand it, only default.  I'll add a test though.
2e0byo: good catch.  Fixed.

Patch:
diff --git a/news/2670.bugfix.md b/news/2670.bugfix.md
new file mode 100644
index 00000000..035e5947
--- /dev/null
+++ b/news/2670.bugfix.md
@@ -0,0 +1,1 @@
+Made --without imply --with :all.
diff --git a/src/pdm/cli/filters.py b/src/pdm/cli/filters.py
index 0fdaf235..8c8f31dc 100644
--- a/src/pdm/cli/filters.py
+++ b/src/pdm/cli/filters.py
@@ -32,6 +32,8 @@ class GroupSelection:
 
     @classmethod
     def from_options(cls, project: Project, options: argparse.Namespace) -> GroupSelection:
+        if getattr(options, "excluded_groups", None) and not options.groups and options.dev is None:
+            options.groups = [":all"]
         if "group" in options:
             return cls(project, group=options.group, dev=options.dev)
         return cls(


Test patch:
diff --git a/tests/cli/test_install.py b/tests/cli/test_install.py
index a26a906b..4686fc97 100644
--- a/tests/cli/test_install.py
+++ b/tests/cli/test_install.py
@@ -314,22 +314,54 @@ def test_fix_package_type_and_update(fixture_project, pdm, working_set):
     assert "test-package-type-fixer" not in working_set
 
 
-def test_install_all_with_excluded_groups(project, working_set, pdm):
+exclusion_cases = [
+    pytest.param(("-G", ":all", "--without", "tz,ssl"), id="-G :all --without tz,ssl"),
+    pytest.param(("-G", ":all", "--without", "tz", "--without", "ssl"), id="-G :all --without tz --without ssl"),
+    pytest.param(("--with", ":all", "--without", "tz,ssl"), id="--with all --without tz,ssl"),
+    pytest.param(("--with", ":all", "--without", "tz", "--without", "ssl"), id="--with all --without tz --without ssl"),
+    pytest.param(("--without", "tz", "--without", "ssl"), id="--without tz --without ssl"),
+    pytest.param(("--without", "tz,ssl"), id="--without tz,ssl"),
+]
+
+
+@pytest.mark.parametrize("args", exclusion_cases)
+def test_install_all_with_excluded_groups(project, working_set, pdm, args):
     project.add_dependencies({"urllib3": parse_requirement("urllib3")}, "url")
     project.add_dependencies({"pytz": parse_requirement("pytz")}, "tz")
     project.add_dependencies({"pyopenssl": parse_requirement("pyopenssl")}, "ssl")
-    pdm(["install", "-G", ":all", "--without", "tz,ssl"], obj=project, strict=True)
+    pdm(["install", *args], obj=project, strict=True)
     assert "urllib3" in working_set
     assert "pytz" not in working_set
     assert "pyopenssl" not in working_set
 
 
-def test_sync_all_with_excluded_groups(project, working_set, pdm):
+@pytest.mark.parametrize("args", exclusion_cases)
+def test_sync_all_with_excluded_groups(project, working_set, pdm, args):
     project.add_dependencies({"urllib3": parse_requirement("urllib3")}, "url")
     project.add_dependencies({"pytz": parse_requirement("pytz")}, "tz")
     project.add_dependencies({"pyopenssl": parse_requirement("pyopenssl")}, "ssl")
     pdm(["lock", "-G:all"], obj=project, strict=True)
-    pdm(["sync", "-G", ":all", "--without", "url,tz"], obj=project, strict=True)
+    pdm(["sync", *args], obj=project, strict=True)
+    assert "urllib3" in working_set
+    assert "pytz" not in working_set
+    assert "pyopenssl" not in working_set
+
+
+def test_excluded_groups_ignored_if_prod_passed(project, working_set, pdm):
+    project.add_dependencies({"urllib3": parse_requirement("urllib3")}, "url")
+    project.add_dependencies({"pytz": parse_requirement("pytz")}, "tz")
+    project.add_dependencies({"pyopenssl": parse_requirement("pyopenssl")}, "ssl")
+    pdm(["install", "--prod", "--without", "ssl"], obj=project, strict=True)
     assert "urllib3" not in working_set
     assert "pytz" not in working_set
-    assert "pyopenssl" in working_set
+    assert "pyopenssl" not in working_set
+
+
+def test_excluded_groups_ignored_if_dev_passed(project, working_set, pdm):
+    project.add_dependencies({"urllib3": parse_requirement("urllib3")}, "url")
+    project.add_dependencies({"pytz": parse_requirement("pytz")}, "tz")
+    project.add_dependencies({"pyopenssl": parse_requirement("pyopenssl")}, "ssl")
+    pdm(["install", "--dev", "--without", "ssl"], obj=project, strict=True)
+    assert "urllib3" not in working_set
+    assert "pytz" not in working_set
+    assert "pyopenssl" not in working_set