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

feat: add py_wheel.compress to control using compression #2260

Conversation

keith
Copy link
Member

@keith keith commented Sep 27, 2024

This is useful for development where, for large wheels, a significant
portion of the time can be spent compression native binaries

Fixes #2240

This is useful for development where, for large wheels, a significant
portion of the time can be spent compression native binaries

Fixes bazelbuild#2240
@rickeylev
Copy link
Collaborator

I was going to ask for it to be an int attribute, but after zipping up some shared libraries to see how compression affects them, I'm not so sure:

  • 1, 2, 3, 4: about 57%, 58%, 58.5%, 59%
  • 5 and higher: about 60%

So a bool seems about equivalent.

I posted in our maintainer chat to see if anyone feels strongly about it being an int instead. They're probably into their weekend by now, though, so we'll give them until monday to respond.

@rickeylev rickeylev changed the title Add attribute for disabling wheel compression feat: add py_wheel.compress to control using compression Sep 27, 2024
Copy link
Collaborator

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

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

Please update CHANGELOG.md to note the new feature.

Otherwise pretty much LGTM

@keith
Copy link
Member Author

keith commented Sep 30, 2024

pushed the changelog, any feedback from folks on the bool vs int?

@keith keith requested a review from rickeylev October 1, 2024 16:08
@rickeylev
Copy link
Collaborator

There was a mild preference for int, but not strong opinions. Worst case, we can add a compress_level arg.

@rickeylev rickeylev added this pull request to the merge queue Oct 1, 2024
Merged via the queue into bazelbuild:main with commit b52f381 Oct 1, 2024
4 checks passed
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.

Support producing uncompressed wheels for development
2 participants