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

Fix METADATA generation for git dependencies with extras #1129

Merged
merged 13 commits into from
Sep 3, 2019
Merged

Fix METADATA generation for git dependencies with extras #1129

merged 13 commits into from
Sep 3, 2019

Conversation

fsal
Copy link
Contributor

@fsal fsal commented May 27, 2019

Fixes #1114

Insert trailing space at the end of the github URL before the semicolon when extra field id specified. This trick prevents pip parser to include the semicolon (that is a avalid URL character) in the URL. In this way pip can install the wheel properly.
Tested with poetry 0.12.16,poetry 1.0.0a3 and pip 19.1.1.


Pull Request Check List

  • Added tests for changed code.
  • Updated documentation for changed code: small bug fix, no need for updated docs.

@fsal fsal changed the title fix METADATA generation for git dependencies with extras Fix METADATA generation for git dependencies with extras May 28, 2019
@iwoloschin
Copy link

Any chance we could get this merged soon?

@@ -71,6 +71,11 @@ def base_pep_508_name(self): # type: () -> str

requirement += " @ {}+{}@{}".format(self._vcs, self._source, self.reference)

# add trailing space if 'extra' field is specified between URL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The base_pep_508_name property should return the base name without any extra space since this is the pre-marker part of a dependency.

This code should instead by but inside the to_pep_508() method.

@fsal
Copy link
Contributor Author

fsal commented Jul 16, 2019

@sdispater I moved the code to the to_pep_508() method in vcs_dependency.py. I also made the changes python2 compliant, but the appveyor job on python2.7 fails because the typing module is not present inside the environment.

Explicitly import io.open (#1212)
# add trailing space if 'extra' field is specified between URL
# and ';'
if self.in_extras:
requirement = requirement.replace("; extra", " ; extra")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That won't work if there are other markers (like python_version)

@iwoloschin
Copy link

Any update on this? Thanks!

@brycedrennan brycedrennan added the kind/bug Something isn't working as expected label Aug 17, 2019
@brycedrennan brycedrennan added the area/build-system Related to PEP 517 packaging (see poetry-core) label Sep 2, 2019
@brycedrennan brycedrennan self-requested a review September 2, 2019 21:38
Copy link
Contributor

@brycedrennan brycedrennan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fsal Here is an idea that feels more explicit than finding the first semicolon.

in dependency.py Line 202

    if markers:
        if self.is_vcs():
            # url based requirements should have a space before semicolon
            # https://www.python.org/dev/peps/pep-0508/#grammar
            requirement += " "

        if len(markers) > 1:
            markers = ["({})".format(m) for m in markers]
            requirement += "; {}".format(" and ".join(markers))
        else:
            requirement += "; {}".format(markers[0])

    return requirement

Could you implement this and also If add a test case with multiple markers? If so I think we can get this merged in.

Also if you could squash all your commits together that would be nice. If not we'll do it at merge time. Thanks!

Copy link

github-actions bot commented Mar 1, 2024

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/build-system Related to PEP 517 packaging (see poetry-core) kind/bug Something isn't working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Malformed METADATA when using optional git dependencies
5 participants