Skip to content

uv: remove --no-default-features from formula #222211

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

Merged
merged 1 commit into from
Jun 1, 2025

Conversation

zanieb
Copy link
Contributor

@zanieb zanieb commented May 2, 2025

Using this flag disables the performance feature, which provides critical performance improvements for production use-cases. I am unsure why it was ever included, it's been there since the formula was added.

See https://github.com/astral-sh/uv/blob/481d05d8dfb8627612dec72840a02c17b926b263/crates/uv/Cargo.toml#L138-L146 for an enumeration of features.

See astral-sh/uv#7686 for the original motivation for splitting performance into a separate feature.

@github-actions github-actions bot added autosquash Automatically squash pull request commits according to Homebrew style. rust Rust use is a significant feature of the PR or issue labels May 2, 2025
This disables the `performance` feature, which provides critical performance improvements for production use-cases
@zanieb zanieb force-pushed the zb/uv-default-features branch from f97476b to 426781b Compare May 2, 2025 12:29
@zanieb zanieb changed the title Remove --no-default-features from uv formula uv Remove --no-default-features from formula May 2, 2025
@github-actions github-actions bot removed the autosquash Automatically squash pull request commits according to Homebrew style. label May 2, 2025
Copy link
Member

@carlocab carlocab left a comment

Choose a reason for hiding this comment

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

Using this flag disables the performance feature, which provides critical performance improvements for production use-cases.

Do you have benchmarks for these improvements?

@zanieb
Copy link
Contributor Author

zanieb commented May 2, 2025

I can generate some benchmarks, but we rigorously tested all of the performance optimizations included there when we added them in the first place. For quite some time (uv 0.1.0 -> 0.4.17), these were already enabled in Homebrew, until we moved them to a feature to allow us to toggle them off for development builds.

@zanieb
Copy link
Contributor Author

zanieb commented May 2, 2025

Separately from the argument that this will improve performance (which I think may only apply to pathological cases we've optimized), we do not intend downstream consumers to build without the default features and it is quite feasible disabling the default features like this will lead to additional unexpected regressions in the future.

konstin added a commit to astral-sh/uv that referenced this pull request May 2, 2025
#5577 fixed a bug on macos due to dynamically linking lzma/xz through static linking. In #7686, this feature was moved to the performance category.

This PR moves the `xz2/static` back to the general default features, and, inspired by Homebrew/homebrew-core#222211, it structures and documents the feature flags cleaner
@zanieb zanieb marked this pull request as ready for review May 2, 2025 15:32
zanieb added a commit to astral-sh/uv that referenced this pull request May 2, 2025
#5577 fixed a bug on macos due to dynamically linking lzma/xz through
static linking. In #7686, this feature was moved to the performance
category.

This PR moves the `xz2/static` back to the general default features,
and, inspired by Homebrew/homebrew-core#222211,
it structures and documents the feature flags cleaner.

We need to take care that this feature does not accidentally disable
features we want.

---------

Co-authored-by: Zanie Blue <contact@zanie.dev>
@zanieb
Copy link
Contributor Author

zanieb commented May 9, 2025

Our standard benchmarks actually don't show a major difference here, which is reassuring. However, I do believe there are pathological cases where the performance features are relevant. Additionally, we expect these features to be used in production and it could easily be confusing for us trying to help users.

As an upstream maintainer, I strongly urge you to remove the --no-default-features flag as it causes unnecessary differences in the distributions of uv and future maintenance burden.

Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label May 31, 2025
@zanieb
Copy link
Contributor Author

zanieb commented May 31, 2025

👋 please don't stale bot this pull request

@github-actions github-actions bot removed the stale No recent activity label May 31, 2025
@zanieb
Copy link
Contributor Author

zanieb commented May 31, 2025

cc @chenrui333 (you reviewed the original formula)

@branchvincent branchvincent added the CI-no-bottles Merge without publishing bottles label Jun 1, 2025
@branchvincent branchvincent changed the title uv Remove --no-default-features from formula uv: remove --no-default-features from formula Jun 1, 2025
@branchvincent branchvincent added this pull request to the merge queue Jun 1, 2025
Merged via the queue into Homebrew:master with commit cdd79f2 Jun 1, 2025
31 checks passed
@branchvincent
Copy link
Member

thank you @zanieb, sorry for the delay. this will ship with the next uv release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-no-bottles Merge without publishing bottles rust Rust use is a significant feature of the PR or issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants