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 doesn't throw error, when extras are a string instead of a list-like #5440

Closed
con-f-use opened this issue Oct 24, 2022 · 15 comments
Closed
Labels
good first issue Issues suitable as a newcomer to get familiar with Pipenv! Type: Possible Bug This issue describes a possible bug in pipenv.

Comments

@con-f-use
Copy link

con-f-use commented Oct 24, 2022

[packages]
msal = {version="==1.20.0", extras="broker"}

Something like text above in a Pipfile, during locking, results in the bug evidenced by output below:

There are incompatible versions in the resolved dependencies:
  msal[b,e,k,o,r]==1.20.0 (from -r /run/user/1000/pipenvji73rdf6requirements/pipenv-mcbzzrng-constraints.txt (line 22))

It should error. Note that is not immediately obvious what is going wrong, as the letters in [b,e,k,o,r] are not even in the same order as the extra dependency "broker".

@con-f-use con-f-use changed the title pipenv doesn't throw error, when extras are a string insted of a container type pipenv doesn't throw error, when extras are a string instead of a list-like Oct 24, 2022
@oz123
Copy link
Contributor

oz123 commented Oct 24, 2022

Question: have you added extras="broker" manually ?

@con-f-use
Copy link
Author

con-f-use commented Oct 24, 2022

I assume you mean as opposed to via pip install msal[broker]? Then yes, I have used an editor to change the Pipfile manually. Still pipenv should not mis-interpret that and iterate over a string because it expects a list here. If it expects a list, then it should throw an error "extras should be a list, not a string".

@oz123
Copy link
Contributor

oz123 commented Oct 24, 2022

I agree with you, but still, I wanted to know what the root cause was.

@oz123
Copy link
Contributor

oz123 commented Oct 25, 2022

Also, I think this should be fixed in https://github.com/sarugaku/plette

@oz123 oz123 added the good first issue Issues suitable as a newcomer to get familiar with Pipenv! label Oct 25, 2022
@aidencullo
Copy link
Contributor

If this issue hasn't been resolved, I'd like to be assigned it. Especially considering it is a good first issue :)

@oz123
Copy link
Contributor

oz123 commented Oct 25, 2022

There you go 🙂 I think the actual fix is in plette but I might be wrong.

@aidencullo
Copy link
Contributor

aidencullo commented Oct 25, 2022 via email

@oz123
Copy link
Contributor

oz123 commented Oct 25, 2022

The PR should be in that repository. You can prototype and test your changes on pipenv/vendor/plette but eventually the PR will go there.

@aidencullo
Copy link
Contributor

Oz, is there some way I can contact you? I have some questions regarding this issue and pipenv in general that I'm not sure belong on this issue thread, my email is on my profile. Thanks

@oz123
Copy link
Contributor

oz123 commented Nov 19, 2022

Sent you and email. Feel to answer there.

@matteius matteius added the Type: Possible Bug This issue describes a possible bug in pipenv. label Dec 3, 2022
@oz123
Copy link
Contributor

oz123 commented Dec 13, 2022

I was able to pin-point this bug. It's not about syncing, it's about requirements generation.

oznt@dell-gentoo:/tmp/pipenv-ncbhfaai-project $ cat Pipfile
[packages]
msal = {version="==1.20.0", extras="broker"}
oznt@dell-gentoo:/tmp/pipenv-ncbhfaai-project $ pipenv lock
Locking [packages] dependencies...
Locking [dev-packages] dependencies...
Updated Pipfile.lock (d5418ae7afddb195bc750352ac1c92fb7c0040ad281c7a4ca32db6c2a4ad2c7c)!
oznt@dell-gentoo:/tmp/pipenv-ncbhfaai-project $ pipenv requirements 
-i https://pypi.org/simple
certifi==2022.12.7 ; python_version >= '3.6'
cffi==1.15.1
charset-normalizer==2.1.1 ; python_full_version >= '3.6.0'
cryptography==38.0.4 ; python_version >= '3.6'
idna==3.4 ; python_version >= '3.5'
msal[b,e,k,o,r]==1.20.0
pycparser==2.21
pyjwt[crypto]==2.6.0 ; python_version >= '3.7'
requests==2.28.1 ; python_version >= '3.7' and python_version < '4'
urllib3==1.26.13 ; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4, 3.5'

The underlying problem is Pipfile.lock actually:

...
        "msal": {
            "extras": [
                "b",
                "e",
                "k",
                "o",
                "r"
            ],
...

oz123 added a commit to sarugaku/plette that referenced this issue Dec 13, 2022
Until now one could write anything in package extras.
If one happend to write a string, e.g:

```
msal = {version="==1.20.0", extras="broker"}
```

It would silently pass, and result in a Pipfile.lock containing a list
of characters:

```
     "msal": {
            "extras": [
                "b",
                "e",
                "k",
                "o",
                "r"
            ],
```

With this change, a validation is added to check that extras
are a list.
Also added is a check that packages specifiers are in a dictionary
and not a list.

This is a potential fix for pypa/pipenv#5440.
@aidencullo
Copy link
Contributor

aidencullo commented Dec 14, 2022 via email

@matteius
Copy link
Member

I think Oz did a fix in palette for this today.

@oz123
Copy link
Contributor

oz123 commented Dec 14, 2022

It's already fixed in plette.

@aidencullo aidencullo removed their assignment Dec 14, 2022
@matteius
Copy link
Member

Please try pipenv==2022.12.17 which includes this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues suitable as a newcomer to get familiar with Pipenv! Type: Possible Bug This issue describes a possible bug in pipenv.
Projects
None yet
Development

No branches or pull requests

4 participants