Repository: pdm-project/pdm
Pull request #1977 (2023-06-05 04:38:20)

fix: reading config from env vars correctly
Signed-off-by: Frost Ming <me@frostming.com>

## Pull Request Checklist

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

## Describe what you have changed in this PR.



Patch:
diff --git a/news/1970.bugfix.md b/news/1970.bugfix.md
new file mode 100644
index 00000000..df68a998
--- /dev/null
+++ b/news/1970.bugfix.md
@@ -0,0 +1,1 @@
+Guess the project name from VCS url if it is missing when importing from requirements.txt.
diff --git a/news/1977.bugfix.md b/news/1977.bugfix.md
new file mode 100644
index 00000000..7d890a23
--- /dev/null
+++ b/news/1977.bugfix.md
@@ -0,0 +1,1 @@
+Correctly read the config from environment variables.
diff --git a/src/pdm/formats/requirements.py b/src/pdm/formats/requirements.py
index f7a32e04..c18ebb98 100644
--- a/src/pdm/formats/requirements.py
+++ b/src/pdm/formats/requirements.py
@@ -8,7 +8,7 @@ import urllib.parse
 from typing import TYPE_CHECKING, Any, Mapping
 
 from pdm.formats.base import make_array
-from pdm.models.requirements import Requirement, parse_requirement
+from pdm.models.requirements import FileRequirement, Requirement, parse_requirement
 
 if TYPE_CHECKING:
     from argparse import Namespace
@@ -53,7 +53,11 @@ class RequirementParser:
         if not line.startswith("-"):
             # Starts with a requirement, just ignore all per-requirement options
             req_string = line.split(" -", 1)[0].strip()
-            self.requirements.append(parse_requirement(req_string))
+            req = parse_requirement(req_string)
+            if not req.name:
+                assert isinstance(req, FileRequirement)
+                req.name = req.guess_name()
+            self.requirements.append(req)
             return
         args, _ = self._parser.parse_known_args(shlex.split(line))
         if args.index_url:
diff --git a/src/pdm/models/repositories.py b/src/pdm/models/repositories.py
index 8cd5a582..ded3887e 100644
--- a/src/pdm/models/repositories.py
+++ b/src/pdm/models/repositories.py
@@ -4,7 +4,7 @@ import dataclasses
 import posixpath
 import sys
 from functools import wraps
-from typing import TYPE_CHECKING, Any, Callable, Iterable, Mapping, TypeVar, cast
+from typing import TYPE_CHECKING, TypeVar, cast
 
 from pdm import termui
 from pdm.exceptions import CandidateInfoNotFound, CandidateNotFound
@@ -25,11 +25,15 @@ from pdm.utils import (
 )
 
 if TYPE_CHECKING:
+    from typing import Any, Callable, Iterable, Mapping
+
     from unearth import Link
 
     from pdm._types import CandidateInfo, RepositoryConfig, SearchResult
     from pdm.environments import BaseEnvironment
 
+    CandidateKey = tuple[str, str | None, str | None, bool]
+
 ALLOW_ALL_PYTHON = PySpecSet()
 T = TypeVar("T", bound="BaseRepository")
 
@@ -390,9 +394,9 @@ class LockedRepository(BaseRepository):
         environment: BaseEnvironment,
     ) -> None:
         super().__init__(sources, environment, ignore_compatibility=False)
-        self.packages: dict[tuple, Candidate] = {}
+        self.packages: dict[CandidateKey, Candidate] = {}
         self.file_hashes: dict[tuple[str, str], dict[Link, str]] = {}
-        self.candidate_info: dict[tuple, CandidateInfo] = {}
+        self.candidate_info: dict[CandidateKey, CandidateInfo] = {}
         self._read_lockfile(lockfile)
 
     @property
@@ -428,7 +432,7 @@ class LockedRepository(BaseRepository):
                 Link(item["url"]): item["hash"] for item in hashes if "url" in item
             }
 
-    def _identify_candidate(self, candidate: Candidate) -> tuple:
+    def _identify_candidate(self, candidate: Candidate) -> CandidateKey:
         url = getattr(candidate.req, "url", None)
         if url is not None:
             url = url_without_fragments(url)
@@ -452,17 +456,22 @@ class LockedRepository(BaseRepository):
             self._get_dependencies_from_lockfile,
         )
 
-    def _matching_keys(self, requirement: Requirement) -> Iterable[tuple]:
+    def _matching_keys(self, requirement: Requirement) -> Iterable[CandidateKey]:
+        from pdm.models.requirements import FileRequirement
+
         for key in self.candidate_info:
+            can_req = self.packages[key].req
             if requirement.name:
                 if key[0] != requirement.identify():
                     continue
-            elif key[2] is not None:
-                if key[2] != url_without_fragments(getattr(requirement, "url", "")):
-                    continue
             else:
-                can_req = self.packages[key].req
-                if can_req.path != getattr(requirement, "path", None):  # type: ignore[attr-defined]
+                assert isinstance(requirement, FileRequirement)
+                if not isinstance(can_req, FileRequirement):
+                    continue
+                if requirement.path and can_req.path:
+                    if requirement.path != can_req.path:
+                        continue
+                elif key[2] is not None and key[2] != url_without_fragments(requirement.url):
                     continue
 
             yield key
diff --git a/src/pdm/models/requirements.py b/src/pdm/models/requirements.py
index 9f1f21f6..41b6b4ee 100644
--- a/src/pdm/models/requirements.py
+++ b/src/pdm/models/requirements.py
@@ -258,6 +258,35 @@ class FileRequirement(Requirement):
     def _hash_key(self) -> tuple:
         return (*super()._hash_key(), self.get_full_url(), self.editable)
 
+    def guess_name(self) -> str | None:
+        filename = os.path.basename(urlparse.unquote(url_without_fragments(self.url))).rsplit("@", 1)[0]
+        if self.is_vcs:
+            if self.vcs == "git":  # type: ignore[attr-defined]
+                name = filename
+                if name.endswith(".git"):
+                    name = name[:-4]
+                return name
+            elif self.vcs == "hg":  # type: ignore[attr-defined]
+                return filename
+            else:  # svn and bzr
+                name, in_branch, _ = filename.rpartition("/branches/")
+                if not in_branch and name.endswith("/trunk"):
+                    return name[:-6]
+                return name
+        elif filename.endswith(".whl"):
+            return parse_wheel_filename(filename)[0]
+        else:
+            try:
+                return parse_sdist_filename(filename)[0]
+            except ValueError:
+                match = _egg_info_re.match(filename)
+                # Filename is like `<name>-<version>.tar.gz`, where name will be
+                # extracted and version will be left to be determined from
+                # the metadata.
+                if match:
+                    return match.group(1)
+        return None
+
     @classmethod
     def create(cls: type[T], **kwargs: Any) -> T:
         if kwargs.get("path"):
@@ -354,19 +383,7 @@ class FileRequirement(Requirement):
             if not self.extras:
                 self.extras = extras
         if not self.name and not self.is_vcs:
-            filename = os.path.basename(urlparse.unquote(url_without_fragments(self.url)))
-            if filename.endswith(".whl"):
-                self.name, *_ = parse_wheel_filename(filename)
-            else:
-                try:
-                    self.name, *_ = parse_sdist_filename(filename)
-                except ValueError:
-                    match = _egg_info_re.match(filename)
-                    # Filename is like `<name>-<version>.tar.gz`, where name will be
-                    # extracted and version will be left to be determined from
-                    # the metadata.
-                    if match:
-                        self.name = match.group(1)
+            self.name = self.guess_name()
 
     def _check_installable(self) -> None:
         assert self.path
diff --git a/src/pdm/project/config.py b/src/pdm/project/config.py
index 76a87d8b..6eb71132 100644
--- a/src/pdm/project/config.py
+++ b/src/pdm/project/config.py
@@ -4,7 +4,7 @@ import collections
 import dataclasses
 import os
 from pathlib import Path
-from typing import Any, Callable, Iterator, Mapping, MutableMapping
+from typing import Any, Callable, Iterator, Mapping, MutableMapping, cast
 
 import platformdirs
 import rich.theme
@@ -12,6 +12,7 @@ import tomlkit
 
 from pdm import termui
 from pdm._types import RepositoryConfig
+from pdm.compat import cached_property
 from pdm.exceptions import NoConfigError, PdmUsageError
 
 REPOSITORY = "repository"
@@ -80,7 +81,7 @@ class ConfigItem:
     replace: str | None = None
 
     def should_show(self) -> bool:
-        return self.default is not self._NOT_SET or bool(self.env_var) and self.env_var in os.environ
+        return self.default is not self._NOT_SET
 
 
 class Config(MutableMapping[str, str]):
@@ -228,6 +229,10 @@ class Config(MutableMapping[str, str]):
         defaults.update(cls.site)
         return defaults
 
+    @cached_property
+    def env_map(self) -> Mapping[str, Any]:
+        return EnvMap(self._config_map)
+
     @classmethod
     def add_config(cls, name: str, item: ConfigItem) -> None:
         """Add or modify a config item"""
@@ -238,7 +243,11 @@ class Config(MutableMapping[str, str]):
         self.config_file = config_file.resolve()
         self.deprecated = {v.replace: k for k, v in self._config_map.items() if v.replace}
         self._file_data = load_config(self.config_file)
-        self._data = collections.ChainMap(self._file_data, self.get_defaults() if is_global else {})
+        self._data = collections.ChainMap(
+            cast(MutableMapping[str, Any], self.env_map) if is_global else {},
+            self._file_data,
+            self.get_defaults() if is_global else {},
+        )
 
     def load_theme(self) -> rich.theme.Theme:
         if not self.is_global:  # pragma: no cover
@@ -286,16 +295,13 @@ class Config(MutableMapping[str, str]):
             raise NoConfigError(key)
         config_key = self.deprecated.get(key, key)
         config = self._config_map[config_key]
-        env_var = config.env_var
-        if env_var is not None and env_var in os.environ:
-            result = os.environ[env_var]
+
+        if config_key in self._data:
+            result = self._data[config_key]
+        elif config.replace:
+            result = self._data[config.replace]
         else:
-            if config_key in self._data:
-                result = self._data[config_key]
-            elif config.replace:
-                result = self._data[config.replace]
-            else:
-                raise NoConfigError(key) from None
+            raise NoConfigError(key) from None
         return config.coerce(result)
 
     def __setitem__(self, key: str, value: Any) -> None:
@@ -324,10 +330,9 @@ class Config(MutableMapping[str, str]):
             raise ValueError(f"Config item '{key}' is not allowed to set in project config.")
 
         value = config.coerce(value)
-        env_var = config.env_var
-        if env_var is not None and env_var in os.environ:
+        if key in self.env_map:
             ui.echo(
-                "WARNING: the config is shadowed by env var '{}', the value set won't take effect.".format(env_var),
+                f"WARNING: the config is shadowed by env var '{config.env_var}', the value set won't take effect.",
                 style="warning",
             )
         self._file_data[config_key] = value
@@ -411,3 +416,28 @@ class Config(MutableMapping[str, str]):
             if config.name == "__unknown__":
                 config.name = name
         return config
+
+
+class EnvMap(Mapping[str, Any]):
+    def __init__(self, config_items: Mapping[str, ConfigItem]) -> None:
+        self._config_map = config_items
+
+    def __repr__(self) -> str:
+        return repr(dict(self))
+
+    def __getitem__(self, k: str) -> Any:
+        try:
+            item = self._config_map[k]
+            if item.env_var:
+                return item.coerce(os.environ[item.env_var])
+        except KeyError:
+            pass
+        raise KeyError(k)
+
+    def __iter__(self) -> Iterator[str]:
+        for key, item in self._config_map.items():
+            if item.env_var and item.env_var in os.environ:
+                yield key
+
+    def __len__(self) -> int:
+        return sum(1 for _ in self)
diff --git a/src/pdm/project/core.py b/src/pdm/project/core.py
index 5a7b0fe9..dd1fe853 100644
--- a/src/pdm/project/core.py
+++ b/src/pdm/project/core.py
@@ -7,7 +7,7 @@ import re
 import shutil
 import sys
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, Iterable, cast
+from typing import TYPE_CHECKING, Any, Iterable, Mapping, cast
 
 import tomlkit
 from tomlkit.items import Array
@@ -118,12 +118,12 @@ class Project:
     def set_lockfile(self, path: str | Path) -> None:
         self._lockfile = Lockfile(path, ui=self.core.ui)
 
-    @property
-    def config(self) -> dict[str, Any]:
-        """A read-only dict configuration, any modifications won't land in the file."""
-        result = dict(self.global_config)
-        result.update(self.project_config)
-        return result
+    @cached_property
+    def config(self) -> Mapping[str, Any]:
+        """A read-only dict configuration"""
+        import collections
+
+        return collections.ChainMap(self.project_config, self.global_config)
 
     @property
     def scripts(self) -> dict[str, str | dict[str, str]]:


Test patch:
diff --git a/tests/test_formats.py b/tests/test_formats.py
index 289bc203..85b7c605 100644
--- a/tests/test_formats.py
+++ b/tests/test_formats.py
@@ -52,7 +52,7 @@ def test_convert_requirements_file_without_name(project, vcs):
     assert requirements.check_fingerprint(project, str(req_file))
     result, _ = requirements.convert(project, str(req_file), Namespace(dev=False, group=None))
 
-    assert result["dependencies"] == ["git+https://github.com/test-root/demo.git"]
+    assert result["dependencies"] == ["demo @ git+https://github.com/test-root/demo.git"]
 
 
 def test_convert_poetry(project):