Skip to content

Make url optional via a new "cred" feature gate for credential helpers; disable default features #1168

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 3 commits into from
Aug 12, 2025

Conversation

joshtriplett
Copy link
Member

@joshtriplett joshtriplett commented Jun 18, 2025

Add a new feature flag for credential helpers, which are the only uses of the url crate. This reduces a build of git2 from 54 dependencies to 17, for a configuration that many tools (e.g. anything that only cares about local repositories) can happily use.

Make the "https" and "ssh" features depend on "cred".

Disable all of these features by default, to avoid extra dependencies in the common case of tools that only care about local repositories.

Bump the crate version to 0.21.

@rustbot rustbot added the S-waiting-on-review Status: Waiting on review label Jun 18, 2025
Add an enabled-by-default feature flag for credential helpers, which are
the only uses of the `url` crate. This reduces a
`default-features = false` build of git2 from 54 dependencies to 17, for
a configuration that many tools (e.g. anything that only cares about
local repositories) can happily use.

In addition to enabling it by default, make the "https" and "ssh"
features depend on "cred".
joshtriplett added a commit to joshtriplett/lstr that referenced this pull request Jun 18, 2025
…inary size

With current git2, this reduces lstr from 144 dependencies to 137, and
from 4.3M to 3.6M.

With rust-lang/git2-rs#1168 merged into upstream
git2-rs, it further reduces lstr to 107 dependencies (though with
negligible additional improvement to binary size as the additional
dependencies were compiled out).
joshtriplett added a commit to joshtriplett/lstr that referenced this pull request Jun 18, 2025
…inary size

With current git2, this reduces lstr from 144 dependencies to 137, and
from 4.3M to 3.6M.

With rust-lang/git2-rs#1168 merged into upstream
git2-rs, it further reduces lstr to 106 dependencies (though with
negligible additional improvement to binary size as the additional
dependencies were compiled out).
@ehuss
Copy link
Contributor

ehuss commented Jun 21, 2025

Wow, I did not know that's how url works.

This will need to be a major change, correct?

@joshtriplett
Copy link
Member Author

joshtriplett commented Jun 22, 2025

@ehuss The new cred feature is enabled by default, so this can only break if you're already using default-features = false. And both https and ssh depend on it too, so anyone enabling either of those features will have it enabled.

That said, a major version bump would still be the safest approach. And, if we bump the major version, I think we should consider disabling some or all of these features by default.

Many users of the git2 crate want to operate exclusively on local
repositories, but if they don't use `default-features = false` they'll
get a large set of additional dependencies they don't need.
@joshtriplett joshtriplett changed the title Make url optional via a new "cred" feature gate for credential helpers Make url optional via a new "cred" feature gate for credential helpers; disable default features Aug 11, 2025
@joshtriplett
Copy link
Member Author

Rather than hoping the change will have no impact on users of 0.20, I've bumped the version to 0.21, and then disabled default features (since we can do that over a semver-major release).

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

@ehuss ehuss added this pull request to the merge queue Aug 12, 2025
Merged via the queue into rust-lang:master with commit 3db61eb Aug 12, 2025
7 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Waiting on review label Aug 12, 2025
@ehuss
Copy link
Contributor

ehuss commented Aug 12, 2025

@joshtriplett I'm having some second thoughts about removing the default features. For example, this essentially breaks some of the examples that are in this repository. I don't really have any data to say whether the majority of users of this library need networking or not. Generally I believe that the difficulty of carefully including the correct set of features (features = ["https", "ssh", "cred"]) needs to significantly outweigh setting default-features=false which I think is a much easier thing to do. I worry that this is going to be a somewhat pointless annoyance for a large number of users, considering that if there are users that want to trim their dependencies, setting default-features=false is relatively much easier.

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.

3 participants