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

Implement missing METADATA parsing via pkginfo #198

Closed
wants to merge 3 commits into from

Conversation

mdvorsky
Copy link

@mdvorsky mdvorsky commented Jun 29, 2019

Replaces the incomplete METADATA parsing with pkginfo. Adds tests using real-world packages.

Fixes #102

@rogerhub
Copy link

rogerhub commented Aug 1, 2019

@brandjon Can you take a look? I think this patch is an important fix.

@mdvorsky
Copy link
Author

mdvorsky commented Oct 3, 2019

@brandjon @lberki any thoughts at all? I realize my fix may not be the preferred way to address this, but I'd love to see this addressed one way or another, so that we can get rid of our local fork of rules_python.

@cristifalcas
Copy link

Hi @brandjon. Please let us know if this project is maintained, or we should move to something else. There are multiple PRs and issues that are not answered for months.

Importing pip packages doesn't work, using python3 is even in a worst state.

@cgordoncarroll
Copy link

Any updates on this pull request? Using Bazel in conjunction with python3/pip seems pretty painful without this. @brandjon

@ppodolsky
Copy link

I second @cgordoncarroll. Is there a way we could help to push this PR forward?

@smamessier
Copy link

Would be very interested in this being merged as well. Trying to use Bazel with Python3 deps.

@thundergolfer
Copy link
Collaborator

@smamessier

Have you tried any of these?

They all support Python 3. We are using the 1st with Python 3.6 at work. It was created after bazelbuild/rules_python just wouldn't work for us.

@smamessier
Copy link

@thundergolfer Thanks I ended up using https://github.com/dillon-giacoppo/rules_python_external and it works perfectly.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@mdvorsky
Copy link
Author

@googlebot I fixed it.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@jakebiesinger-onduo
Copy link

👋 1-year anniversary for this PR is coming up. Any chance we could get it merged?

@mdvorsky
Copy link
Author

/cc @thundergolfer @andyscott

Copy link
Collaborator

@andyscott andyscott left a comment

Choose a reason for hiding this comment

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

My only context for this code change is a close reading of the diff (unfortunately 😢 ), but with that in mind, this looks good to me.

@mdvorsky
Copy link
Author

Thanks a lot @andyscott! Would you like me to do anything else here? (The build of your merge failed because of an unrelated issue already fixed at head, afaict).

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no and removed cla: yes labels Aug 1, 2020
@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels Aug 1, 2020
@mdvorsky
Copy link
Author

mdvorsky commented Aug 1, 2020

Hi! @thundergolfer @andyscott I removed the .par files from the PR, and squashed the commits. Is there anything else I should do here? Thank you!

@horkyada
Copy link

Hi rules_python folks. Is there any chance this gets merged?

@github-actions
Copy link

This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days.
Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!

@github-actions github-actions bot added the Can Close? Will close in 30 days if there is no new activity label Apr 14, 2021
@mdvorsky mdvorsky closed this Apr 18, 2021
@xhuoroku
Copy link

xhuoroku commented Jul 7, 2021

@mdvorsky Thank you for the great work!

Is it possible to also make the code change for pip_import so that issue 102 can be fixed when using pip_import?

It's the only packaging rule that supports Python 2, would be super helpful if it can be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Can Close? Will close in 30 days if there is no new activity cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failure on pip dependency