Skip to content

Flatten python packages#743

Merged
groodt merged 9 commits intomainfrom
groodt-flatten-py-packages
Jul 12, 2022
Merged

Flatten python packages#743
groodt merged 9 commits intomainfrom
groodt-flatten-py-packages

Conversation

@groodt
Copy link
Collaborator

@groodt groodt commented Jul 4, 2022

This PR attempts to simplify the pip_install / pip_parse python source packages. The code had grown organically and I found it difficult to follow and unnecessarily nested into packages.

In general, the code should be quite simple:

  1. A tool to install all wheels eagerly in a requirements.txt (pip_install)
  2. A tool to parse a requirements.txt to a bzl file (pip_parse & parse_requirements_to_bzl)
  3. A tool to install a single wheel (pip_parse & whl_library)

There is some additional utility around annotations to bazel rules, extracting wheels and updating namespace_pkgs, but I still believe the code doesn't need to be so fragmented across multiple packages. There is a lot of very similar / duplicated logic when extracting wheels that is probably unnecessary.

This PR simply moves existing code and imports around and should be fully backwards compatible.

Given that pip_parse seems to be the more popular choice (based on discussions and most of the recent GH issues) it is probably justified to remove the eager installation logic (pip_install) and standardise on pip_parse. It should be possible to run with both aliases. I'll do this in a follow-up refactor.

@groodt groodt marked this pull request as ready for review July 4, 2022 12:50
@groodt groodt requested review from brandjon and lberki as code owners July 4, 2022 12:50
@groodt groodt removed request for brandjon and lberki July 4, 2022 12:53
Copy link
Contributor

@hrfuller hrfuller left a comment

Choose a reason for hiding this comment

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

LGTM. I agree it was getting ungainly and all the dunder files were ugly. Can you please follow up after squashing and merging, by adding this commit to //:.git-blame-ignore-revs

@groodt groodt merged commit 4984423 into main Jul 12, 2022
@groodt groodt deleted the groodt-flatten-py-packages branch July 12, 2022 20:30
alexeagle added a commit to aspect-build/bazel-central-registry that referenced this pull request Aug 3, 2022
alexeagle added a commit to aspect-build/bazel-central-registry that referenced this pull request Aug 3, 2022
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

Successfully merging this pull request may close these issues.

2 participants