hyperium / hyper

An HTTP library for Rust
https://hyper.rs
MIT License
14.6k stars 1.6k forks source link

Include a Cargo.lock #3788

Open getchoo opened 4 days ago

getchoo commented 4 days ago

Is your feature request related to a problem? Please describe. Without a Cargo.lock, building the hyper crate itself (i.e., as a cdylib for curl) can be difficult to do in a reproducible manner and/or in environments where offline building is required.

Describe the solution you'd like Cargo.lock being removed from .gitignore and distributed upstream.

Describe alternatives you've considered Downstream vendors can make their own Cargo.lock files. This is currently being done in https://github.com/NixOS/nixpkgs/pull/357552.

Additional context Previously, the Cargo team recommended against having Cargo.lock files distributed by libraries. As of last year, they have changed this advice to "recommend people do what is best for their project".

seanmonstar commented 3 days ago

Hm, I think I'd still defer having a lock file checked in. My understanding of the Cargo team's recommendation is that it can help with CI. That hasn't been an issue for me.

It's likely we wouldn't remember to update it frequently. I wouldn't want people who are building it to depend on that file. I'd rather those building it to have their own lock file and control exact dependency versions.

Is that a problem, such as for NixOS?

milibopp commented 3 days ago

Is that a problem, such as for NixOS?

We prefer relying on upstream for lock files over shipping our own, but it is possible to do have our own and is done in quite a few packages. Nix by design has very strong reproducibility guarantees, so we must lock down dependencies eventually.

If we do that ourselves, we do that later than you as a maintainer when you release a new version. This causes a discrepancy between the known-good dependency tree when you released and whatever we ship later. If everybody follows semantic versioning or the test suite is complete enough to catch potential problems, this should not cause issues, but it could in principle.

I personally think there is value in having a central authority on the lock file, rather than decentralizing that concern towards the various downstream distributions as any dependency bugs only have to be fixed once. In particular, it would be so easy to overlook important patches somewhere in the dependency tree that could be fixed by running cargo update. Besides, users doing custom builds can still ignore the lock file in the repository and generate their own.

getchoo commented 3 days ago

My understanding of the Cargo team's recommendation is that it can help with CI.

I feel this is a bit of a simplification. From the Cargo book FAQ:

The purpose of a Cargo.lock lockfile is to describe the state of the world at the time of a successful build. Cargo uses the lockfile to provide deterministic builds at different times and on different systems, by ensuring that the exact same dependencies and versions are used as when the Cargo.lock file was originally generated.

This may help in CI, but it has a much greater benefit in environments not regularly accounted for, such as those that would usually ship much older or extremely new versions of libraries.

I would also like to put a lot of emphasis on @milibopp's point above:

I personally think there is value in having a central authority on the lock file, rather than decentralizing that concern towards the various downstream distributions as any dependency bugs only have to be fixed once.

If hyper is to ever become a common backend for curl among distributions, this would create a lot of needlessly duplicated work across the entire ecosystem -- putting yet another obstacle in the way of adoption. Of course this is by no means a dealbreaker as packagers have adapted to these kinds of cases over the years, but can create some friction as explained here and above

JohnRTitor commented 3 days ago

It's likely we wouldn't remember to update it frequently

A CI job could be setup to update cargo locks automatically, or perhaps a pre commit hook that runs cargo update.

Former is better I believe.

lu-zero commented 3 days ago

IMHO having the Cargo.lock in git is an endless source of churn for the developers with little gains for the distributors if the releases are frequent enough.

If the releases are infrequent distributions HAVE to bump the dependencies on their own.

Having the release process provide a known good Cargo.lock is enough for all the intended purposes.