dan-t / rusty-tags

Create ctags/etags for a cargo project
Other
405 stars 32 forks source link

Add cargo.lock file back to version control #81

Closed sgued closed 9 months ago

sgued commented 3 years ago

Hello, thanks for your work on rusty-tags.

Commit 3bfc1a3f4fa7f48b8f3072fea2c6a456c6736106 removes the Cargo.lock file from version control. However, it is recommended by the rust book to keep the Cargo.lock file in releases. It seems that the reason it was removed isn't justified. Newer versions of Cargo still understand the old Cargo.lock format. It just has to be regenerated by using an older version of Cargo. Future calls to cargo update, even with a recent version of Cargo wont change the format.

If you want I can make a PR adding a new lockfile with the old format.

dan-t commented 3 years ago

On Sun, Mar 21, 2021 at 01:26:20PM -0700, Sosthene-Guedon wrote:

However, it is recommended by the rust book to keep the Cargo.lock file in releases.

To my knowledge 'cargo install' doesn't consider the Cargo.lock file, but only builds on a checked out source. So what's the point of keeping the Cargo.lock?

If the build fails because of a dependency, then either the versions in the Cargo.toml are wrong or the dependency violated the Semver policy, and both would I consider to be a bug.

And also the Cargo.lock has its own issues, like not getting updates of bugfixes in dependencies.

sgued commented 3 years ago

cargo install might not (I'm not sure and haven't checked) but cargo build --release --locked does, and it's the recommended way to build releases, that way, there only needs a single combo of dependencies that has to be tested. It is also vital if you want to have reproducible builds.

And also the Cargo.lock has its own issues, like not getting updates of bugfixes in dependencies.

Updates to dependencies should lead to new releases. Given that dependencies are statically linked, it is unreasonable to expect users to regularly recompile in the aim of getting up to date dependencies.

I'm the maintainer of the AUR package for rusty-tags, and the AUR packaging guidelines recommend using --locked for building Rust packages, but it is not possible if the Cargo.lock file isn't part of the release. This is what lead me to open this issue.

robinkrahl commented 3 years ago

cargo install does indeed ignore the lock file by default, see the Cargo Book:

By default, the Cargo.lock file that is included with the package will be ignored. This means that Cargo will recompute which versions of dependencies to use, possibly using newer versions that have been released since the package was published. The --locked flag can be used to force Cargo to use the packaged Cargo.lock file if it is available. This may be useful for ensuring reproducible builds, to use the exact same set of dependencies that were available when the package was published. It may also be useful if a newer version of a dependency is published that no longer builds on your system, or has other problems. The downside to using --locked is that you will not receive any fixes or updates to any dependency. Note that Cargo did not start publishing Cargo.lock files until version 1.37, which means packages published with prior versions will not have a Cargo.lock file available.

But I agree that Cargo.lock should be included nevertheless to at least give users the option to use the locked versions.

If the build fails because of a dependency, then either the versions in the Cargo.toml are wrong or the dependency violated the Semver policy, and both would I consider to be a bug.

That’s true, but bugs will always happen. Having a Cargo.lock file gives users the option to work around these bugs, and from my experience, it makes it easier for developers to track down the reason for a build failure.

sgued commented 2 years ago

Hi, do you intend to put the lockfile back in version control? If not, maybe closing the issue would be better.

Here are a couple more arguments regarding why the Lockfile should be kept under version control.