Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pipenv 2023.8.21 parse error with packages from git #5849

Closed
jedie opened this issue Aug 22, 2023 · 19 comments
Closed

Pipenv 2023.8.21 parse error with packages from git #5849

jedie opened this issue Aug 22, 2023 · 19 comments

Comments

@jedie
Copy link

jedie commented Aug 22, 2023

I have this in my Pipfile:

foobar_pkg_name = {ref = "v2023.08.18.1", git = "https://gitlab%2Bdeploy-token:[email protected]/group_name/foobar_pkg_name.git"}

Results in Pipfile.lock to:

"foobar_pkg_name": {
    "git": "https://gitlab+deploy-token:[email protected]/group_name/foobar_pkg_name.git",
    "ref": "0000-the-pkg-hash-0000"
},

The installation results in a error, because of completely wrong URI:

[pipenv.exceptions.InstallError]: Looking in indexes: https://pypi.python.org/simple
[pipenv.exceptions.InstallError]: Collecting foobar_pkg_name@ git+https://gitlab+deploy-token:TheToken@0000-the-pkg-hash-0000 (from -r /tmp/pipenv-cn09v7m7-requirements/pipenv-98f1ayok-reqs.txt (line 1))
[pipenv.exceptions.InstallError]:   Cloning https://gitlab%2Bdeploy-token:****@0000-the-pkg-hash-0000 to /tmp/pip-install-5uk38bc4/foobar_pkg_name_4300623bc17942a0ae003fe8d1f0500e

The same works with older Pipenv v2023.7.23

@marceltschoppch
Copy link

marceltschoppch commented Aug 22, 2023

I have the same problem with editable GitHub dependecies.

Pipfile:

mypackage = {editable = true,git = "ssh://[email protected]/myorg/mypackage",ref = "master"}

Pipfile.lock:

"mypackage": {
    "editable": true,
    "git": "ssh://[email protected]/myorg/mypackage",
    "ref": "c807****d402"
},

Error:

Installing dependencies from Pipfile.lock (057240)...
[pipenv.exceptions.InstallError]: Collecting mypackage@ git+ssh://git@c807****d402 (from -r /var/folders/2y/4s08qm216w53054z5h2jl4zh0000gn/T/pipenv-q8sxrgdh-requirements/pipenv-q27ppm60-reqs.txt (line 1))
[pipenv.exceptions.InstallError]:   Cloning ssh://****@c807****d402 to /private/var/folders/2y/4s08qm216w53054z5h2jl4zh0000gn/T/pip-install-lv7r7wwo/mypackage_9814293569e9490e8c7f19e26dd67e7c
[pipenv.exceptions.InstallError]: Running command git clone --filter=blob:none --quiet 'ssh://****@c807****d402' /private/var/folders/2y/4s08qm216w53054z5h2jl4zh0000gn/T/pip-install-lv7r7wwo/mypackage_9814293569e9490e8c7f19e26dd67e7c
[pipenv.exceptions.InstallError]:   fatal: No path specified. See 'man git-pull' for valid url syntax
[pipenv.exceptions.InstallError]:   error: subprocess-exited-with-error
[pipenv.exceptions.InstallError]:
[pipenv.exceptions.InstallError]:   × git clone --filter=blob:none --quiet 'ssh://****@c807****d402' /private/var/folders/2y/4s08qm216w53054z5h2jl4zh0000gn/T/pip-install-lv7r7wwo/mypackage_9814293569e9490e8c7f19e26dd67e7c did not run successfully.
[pipenv.exceptions.InstallError]:   │ exit code: 128
[pipenv.exceptions.InstallError]:   ╰─> See above for output.
[pipenv.exceptions.InstallError]:
[pipenv.exceptions.InstallError]:   note: This error originates from a subprocess, and is likely not a problem with pip.
[pipenv.exceptions.InstallError]: error: subprocess-exited-with-error
[pipenv.exceptions.InstallError]:
[pipenv.exceptions.InstallError]: × git clone --filter=blob:none --quiet 'ssh://****@c807****d402' /private/var/folders/2y/4s08qm216w53054z5h2jl4zh0000gn/T/pip-install-lv7r7wwo/mypackage_9814293569e9490e8c7f19e26dd67e7c did not run successfully.
[pipenv.exceptions.InstallError]: │ exit code: 128
[pipenv.exceptions.InstallError]: ╰─> See above for output.
[pipenv.exceptions.InstallError]:
[pipenv.exceptions.InstallError]: note: This error originates from a subprocess, and is likely not a problem with pip.

If I'm not mistaken the bug came in with this PR by @matteius:
#5845

@matteius
Copy link
Member

Ah bummer, sorry about this! This is tricky because some folks have the @ symbol as an extra ref and so I was thinking that would be easier to peel back off, but that breaks ssh:// without the extra @ symbol. 🤔

@matteius
Copy link
Member

matteius commented Aug 22, 2023

I just opened a PR that I think accounts for this extra edge case of ssh:// dependencies needing at least one @ in the URL -- would be great if someone could help verify (l am letting the test CI run but that clearly doesn't tell us everything, especially regarding ssh://).

Note: While if someone has a password in the vcs ref with an @, this could break in theory, but also in theory to hide the password it should be extracted out as a env var so I think this will still work.

@marceltschoppch
Copy link

Thanks @matteius , your PR fixed the issue for my case 👍

@matteius
Copy link
Member

2023.8.22 has been released -- sorry again for the trouble!

@jedie
Copy link
Author

jedie commented Aug 22, 2023

Sorry, i stil see the same error with v2023.8.22

@matteius matteius reopened this Aug 22, 2023
@bpicardat
Copy link

The issue is not only with ssh, there can also be a second @ sign with http[s] url, for basic authentication, like in the first message here.

@matteius
Copy link
Member

Ah I see now @jedie your example is not an ssh url.

@matteius
Copy link
Member

I am honestly not sure yet of how to handle the case of there could be 1 @ symbol in the URL and its formed correctly without a ref vs 1 @ symbol that is the ref without an authentication one. How to detect the @ symbol pertains to auth in these types of strings and leave it alone? 🤔

@bpicardat
Copy link

Maybe with urllib.parse.urlparse? Only remove the trailing @ and ref string from the "path" but don't touch the "netloc"

@matteius
Copy link
Member

matteius commented Aug 22, 2023

@bpicardat That might be viable, but if I recall when working on the original refactor I was having some trouble with urllib parse but I can't recall specifically if it was about evaluating the netloc env vars and loosing them in the evaluated string (they get masked) or something similar that makes it hard to reconstruct back to the original line after parsing it. I opened an alternative PR that I think might solve it, assuming there aren't examples that lack a slash after the netloc @

@matteius
Copy link
Member

If anyone has time to verify the latest PR -- I'll try to wrap up merging it later today if its a reasonable fix.

@wesley-harding
Copy link

Not sure if it's helpful, but we're noticing an error handling unauthenticated zip files over https in 2023.8.20. This issue is not present in 2023.7.23 (which my colleague is running without issue).

# Example, but this is a public repo without any authentication.
xlsx2html = { path = 'https://github.com/our-organization/public-repo/archive/refs/tags/some-release.zip' }

I wanted to share as the comment thread seems to indicate some concern around authentication symbols in the url. In our case, this is a basic, public URL without any auth.

@matteius
Copy link
Member

matteius commented Aug 22, 2023

@wesley-harding That feels like possibly a different issue -- Could you verify its still happening on 2023.8.22 and if so open a new report tracking it? I just tested with fastapi https://github.com/tiangolo/fastapi/archive/refs/heads/master.zip and it got added to pipfile under key file and lock successful, I updated key to path and re-locked, was still successful.

@julio-tl
Copy link

We are also experiencing regressions with 2023.8.22. This started happening today, when we pull a dependency from a private repo via git+https: in the requirements.txt file, we get what appears to be a parsing related error (I added * to the commit hash but you get the point):

Installing dependencies from Pipfile.lock (ab0588)...
...
[pipenv.exceptions.InstallError]:   fatal: unable to access 'https://***b4a5/': Could not resolve host: ***b4a5

I had to clamp pipenv==2023.8.20 in our Docker build to get it to work again.

@matteius
Copy link
Member

@julio-tl Are you able to check the tagged draft PR to see if it fixes the regression in your case? Also, I apologize for the inconvenience.

@julio-tl
Copy link

julio-tl commented Aug 22, 2023

@matteius Yes I switched to pip install git+https://github.com/pypa/pipenv@issue-5849b which I believe carries #5856 and it worked!

Edit: And no worries at all, thanks for jumping right in, I appreciate the effort. You are the MVP here

@marceltschoppch
Copy link

@matteius I can confirm that the alternative PR still works. Thanks for your effort!

@matteius
Copy link
Member

2023.8.23 has been cut for the vcs issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants