We clone the package twice in spite of the cache on

From that point of view the alternative fix that I tried in #6874 (comment) does better, but perhaps it's the wrong direction or has other disadvantages.

Right, that makes more sense. What do we think about introducing a new datatype here to avoid the overloaded strings and to encapsulate the HEAD == None logic? The tests look good; we just need to realize that None and HEAD are the same thing (and to avoid the entropy of a string conversion from HEAD causing the cache to miss and clone a second time).

@neersighted I kind of feel that adding a new datatype here might be overkill at present, but happy to be told otherwise. Alternatively, we could do something like this.

diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py
index 4d2e2d30..97abfb8c 100644
--- a/src/poetry/puzzle/provider.py
+++ b/src/poetry/puzzle/provider.py
@@ -86,7 +86,7 @@ class Indicator(ProgressIndicator):  # type: ignore[misc]
 
 
 @functools.lru_cache(maxsize=None)
-def _get_package_from_git(
+def _get_package_from_git_cached(
     url: str,
     branch: str | None = None,
     tag: str | None = None,
@@ -118,6 +118,24 @@ def _get_package_from_git(
     return package
 
 
+def _get_package_from_git(
+    url: str,
+    branch: str | None = None,
+    tag: str | None = None,
+    rev: str | None = None,
+    subdirectory: str | None = None,
+    source_root: Path | None = None,
+) -> Package:
+    return _get_package_from_git_cached(
+            url,
+            branch if branch != "HEAD" else None,
+            tag,
+            rev if rev != "HEAD" else None,
+            subdirectory,
+            source_root,
+        )
+
+
 class Provider:
     UNSAFE_PACKAGES: set[str] = set()
 

Either way, I can raise another PR to deal with the double clone issue. Let's solve the functionality issue by merging this?

Edit: Also there already is

cf python-poetry/poetry-plugin-export#172 in which the HEAD reference is again escaping where it oughtn't.

I wonder whether HEAD simply isn't a good reference to use and this fix is just papering over that (and requiring more of the same elsewhere).

Perhaps the alternative at #6874 (comment) is better after all - though I seem to recall there was more unit test noise to work through in that case.

Is there any plan to merge / fix this?

@Secrus I have rebased this and fixed the mypy issue.

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.