Auto merge of #12382 - epage:lock, r=weihanglo

fix: Change the defaults to always check-in `Cargo.lock`

### What does this PR try to resolve?

Having libraries leave `Cargo.lock` "on the float" has been serving the ecosystem well, including
- Encouraging people to validate that latest dependencies work
- Encouraging ecosystem-wide health by acting as a "distributed crater"

These benefits are limited though.  The policy is inconsistent between workspaces with or without `[[bin]]`s, reducing the affect of testing the latest.  This is also subject to when CI last ran; for passively maintained projects, there is little coverage of new dependencies.

There are also costs associated with this policy
- `git bisect` is using an unpredictable set of dependencies, affecting the ability to identify root cause
- This is another potential cause for Red CI / broken local development if version is yanked or a bug is introduced
  - Impacting the perceived level of quality for a project
  - Confusing to new contributors who might not recognize why CI failed and assume its their fault
  - Requiring context switching from maintainers to get fixes in

In particular, since this policy was decided, there has been an increased interest in supporting an MSRV (as recently as v1.56.0, cargo gained support for specifying a package's MSRV).   This has led to long discussions on *what* MSRV a package should use (e.g. rust-lang/libs-team#72,. time-rs/time#535).  Worst, there has been a growing trend for people to set an non-semver upper bound on dependencies, making it so packages can't work well with other packages (see #12323).  Tooling support would help with this (#9930) but the sooner we address this, the less entrenched bad practices will be.

On the positive side, since the policy was decided
- In general, CI became easier to setup and maintain with Github Actions compared to TravisCI
- Dependabot went GA on Github in 2021 (https://github.blog/changelog/2021-03-31-dependabot-version-updates-are-now-generally-available)
  - I believe Dependabot will post security update PRs even when Dependabot is not more generally enabled

So to get some of the benefit from not checking in `Cargo.lock`, we can recommend either automatically applying updates or having CI check the latest dependencies in a way to get this out of the critical path of PRs and releases.

Since there is no one right answer on how to solve all of these problems, we're documenting these trade offs so people can make the choice that is most appropriate for them.  However, we are changing the default to a consistent "always check it in" as the answer for those who don't want to think about it.

Prior art
- [Yarn (Javascript)](https://yarnpkg.com/getting-started/qa#should-lockfiles-be-committed-to-the-repository)
- [Poetry (Python)](https://python-poetry.org/docs/basic-usage/#committing-your-poetrylock-file-to-version-control)
- [Bundler (Ruby)](https://bundler.io/guides/faq.html#using-gemfiles-inside-gems)

Fixes #8728

### How should we test and review this PR?

Please review per-commit.  I tried to minimize changes I made to the structure of the CI document

In #8728, I brought up having a CI reference page.  I keep going back and forth on whether this is guide-level material or reference-level material.  Obviously, right now I'm leaning towards it being guide-level.

### Additional information

This changes cargo from telling people what to do to giving them a starting point, or default, and giving them the information to make their own choice, if needed.

So the question for defaults is who are we targeting? For a default path in documentation, it would be for new to intermediate users. As for `cargo new`, we've been prioritizing new users over those that run it frequently (boiler plate comment, bin is default, etc).

See #8728 for the FCP on this policy change

Author: bors

src/cargo/ops/cargo_new.rs

@@ -93,7 +93,6 @@ struct MkOptions<'a> {
     path: &'a Path,
     name: &'a str,
     source_files: Vec<SourceFileInformation>,
-    bin: bool,
     edition: Option<&'a str>,
     registry: Option<&'a str>,
 }
@@ -448,7 +447,6 @@ pub fn new(opts: &NewOptions, config: &Config) -> CargoResult<()> {
         path,
         name,
         source_files: vec![plan_new_source_file(opts.kind.is_bin(), name.to_string())],
-        bin: is_bin,
         edition: opts.edition.as_deref(),
         registry: opts.registry.as_deref(),
     };
@@ -553,7 +551,6 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<NewProjectKind> {
         version_control,
         path,
         name,
-        bin: has_bin,
         source_files: src_paths_types,
         edition: opts.edition.as_deref(),
         registry: opts.registry.as_deref(),
@@ -745,9 +742,6 @@ fn mk(config: &Config, opts: &MkOptions<'_>) -> CargoResult<()> {
     // for all mutually-incompatible VCS in terms of syntax are in sync.
     let mut ignore = IgnoreList::new();
     ignore.push("/target", "^target$", "target");
-    if !opts.bin {
-        ignore.push("/Cargo.lock", "^Cargo.lock$", "Cargo.lock");
-    }
 
     let vcs = opts.version_control.unwrap_or_else(|| {
         let in_existing_vcs = existing_vcs_repo(path.parent().unwrap_or(path), config.cwd());

src/doc/src/faq.md

@@ -102,33 +102,47 @@ issue][cargo-issues].
 
 [cargo-issues]: https://github.com/rust-lang/cargo/issues
 
-### Why do binaries have `Cargo.lock` in version control, but not libraries?
+### Why have `Cargo.lock` in version control?
+
+While [`cargo new`] defaults to tracking `Cargo.lock` in version control,
+whether you do is dependent on the needs of your package.
 
 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 on different times and different systems, by ensuring that
-the exact same dependencies and versions are used as when the `Cargo.lock` file
-was originally generated.
-
-This property is most desirable from applications and packages which are at the
-very end of the dependency chain (binaries). As a result, it is recommended that
-all binaries check in their `Cargo.lock`.
-
-For libraries the situation is somewhat different. A library is not only used by
-the library developers, but also any downstream consumers of the library. Users
-dependent on the library will not inspect the library’s `Cargo.lock` (even if it
-exists). This is precisely because a library should **not** be deterministically
-recompiled for all users of the library.
-
-If a library ends up being used transitively by several dependencies, it’s
-likely that just a single copy of the library is desired (based on semver
-compatibility). If Cargo used all of the dependencies' `Cargo.lock` files,
-then multiple copies of the library could be used, and perhaps even a version
-conflict.
-
-In other words, libraries specify SemVer requirements for their dependencies but
-cannot see the full picture. Only end products like binaries have a full
-picture to decide what versions of dependencies should be used.
+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.
+
+Deterministic builds help with
+- Running `git bisect` to find the root cause of a bug
+- Ensuring CI only fails due to new commits and not external factors
+- Reducing confusion when contributors see different behavior as compared to
+  other contributors or CI
+
+Having this snapshot of dependencies can also help when projects need to be
+verified against consistent versions of dependencies, like when
+- Verifying a minimum-supported Rust version (MSRV) that is less than the latest
+  version of a dependency supports
+- Verifying human readable output which won't have compatibility guarantees
+  (e.g. snapshot testing error messages to ensure they are "understandable", a
+  metric too fuzzy to automate)
+
+However, this determinism can give a false sense of security because
+`Cargo.lock` does not affect the consumers of your package, only `Cargo.toml` does that.
+For example:
+- [`cargo install`] will select the latest dependencies unless `--locked` is
+  passed in.
+- New dependencies, like those added with [`cargo add`], will be locked to the latest version
+
+The lockfile can also be a source of merge conflicts.
+
+For strategies to verify newer versions of dependencies via CI,
+see [Verifying Latest Dependencies](guide/continuous-integration.md#verifying-latest-dependencies).
+
+[`cargo new`]: commands/cargo-new.md
+[`cargo add`]: commands/cargo-add.md
+[`cargo install`]: commands/cargo-install.md
 
 ### Can libraries use `*` as a version for their dependencies?
 

src/doc/src/guide/cargo-toml-vs-cargo-lock.md

@@ -8,14 +8,11 @@ about them, here’s a summary:
 * `Cargo.lock` contains exact information about your dependencies. It is
   maintained by Cargo and should not be manually edited.
 
-If you’re building a non-end product, such as a rust library that other rust
-[packages][def-package] will depend on, put `Cargo.lock` in your
-`.gitignore`. If you’re building an end product, which are executable like
-command-line tool or an application, or a system library with crate-type of
-`staticlib` or `cdylib`, check `Cargo.lock` into `git`. If you're curious
-about why that is, see
-["Why do binaries have `Cargo.lock` in version control, but not libraries?" in the
-FAQ](../faq.md#why-do-binaries-have-cargolock-in-version-control-but-not-libraries).
+When in doubt, check `Cargo.lock` into git.
+For a better understanding of why and what the alternatives might be, see
+[“Why have Cargo.lock in version control?” in the FAQ](../faq.md#why-have-cargolock-in-version-control).
+We recommend pairing this with
+[Verifying Latest Dependencies](continuous-integration.md#verifying-latest-dependencies)
 
 Let’s dig in a little bit more.
 

src/doc/src/guide/continuous-integration.md

@@ -1,25 +1,8 @@
 ## Continuous Integration
 
-### Travis CI
+### Getting Started
 
-To test your [package][def-package] on Travis CI, here is a sample
-`.travis.yml` file:
-
-```yaml
-language: rust
-rust:
-  - stable
-  - beta
-  - nightly
-matrix:
-  allow_failures:
-    - rust: nightly
-```
-
-This will test all three release channels, but any breakage in nightly
-will not fail your overall build. Please see the [Travis CI Rust
-documentation](https://docs.travis-ci.com/user/languages/rust/) for more
-information.
+A basic CI will build and test your projects:
 
 ### GitHub Actions
 
@@ -122,4 +105,58 @@ channel, but any breakage in nightly will not fail your overall build. Please
 see the [builds.sr.ht documentation](https://man.sr.ht/builds.sr.ht/) for more
 information.
 
-[def-package]:  ../appendix/glossary.md#package  '"package" (glossary entry)'
+### Verifying Latest Dependencies
+
+When [specifying dependencies](../reference/specifying-dependencies.md) in
+`Cargo.toml`, they generally match a range of versions.
+Exhaustively testing all version combination would be unwieldy.
+Verifying the latest versions would at least test for users who run [`cargo
+add`] or [`cargo install`].
+
+When testing the latest versions some considerations are:
+- Minimizing external factors affecting local development or CI
+- Rate of new dependencies being published
+- Level of risk a project is willing to accept
+- CI costs, including indirect costs like if a CI service has a maximum for
+  parallel runners, causing new jobs to be serialized when at the maxium.
+
+Some potential solutions include:
+- [Not checking in the `Cargo.lock`](../faq.md#why-have-cargolock-in-version-control)
+  - Depending on PR velocity, many versions may go untested
+  - This comes at the cost of determinism
+- Have a CI job verify the latest dependencies but mark it to "continue on failure"
+  - Depending on the CI service, failures might not be obvious
+  - Depending on PR velocity, may use more resources than necessary
+- Have a scheduled CI job to verify latest dependencies
+  - A hosted CI service may disable scheduled jobs for repositories that
+    haven't been touched in a while, affecting passively maintained packages
+  - Depending on the CI service, notifications might not be routed to people
+    who can act on the failure
+  - If not balanced with dependency publish rate, may not test enough versions
+    or may do redundant testing
+- Regularly update dependencies through PRs, like with [Dependabot] or [RenovateBot]
+  - Can isolate dependencies to their own PR or roll them up into a single PR
+  - Only uses the resources necessary
+  - Can configure the frequency to balance CI resources and coverage of dependency versions
+
+An example CI job to verify latest dependencies, using Github Actions:
+```yaml
+jobs:
+  latest_deps:
+    name: Latest Dependencies
+    runs-on: ubuntu-latest
+    continue-on-error: true
+    steps:
+      - uses: actions/checkout@v3
+      - run: rustup update stable && rustup default stable
+      - run: cargo update --verbose
+      - run: cargo build --verbose
+      - run: cargo test --verbose
+```
+For projects with higher risks of per-platform or per-Rust version failures,
+more combinations may want to be tested.
+
+[`cargo add`]: ../commands/cargo-add.md
+[`cargo install`]: ../commands/cargo-install.md
+[Dependabot]: https://docs.github.com/en/code-security/dependabot/working-with-dependabot
+[RenovateBot]: https://renovatebot.com/

tests/testsuite/cargo_init/auto_git/out/.gitignore

@@ -1,2 +1 @@
 /target
-/Cargo.lock

tests/testsuite/cargo_init/fossil_autodetect/out/.fossil-settings/clean-glob

@@ -1,2 +1 @@
 target
-Cargo.lock

tests/testsuite/cargo_init/fossil_autodetect/out/.fossil-settings/ignore-glob

@@ -1,2 +1 @@
 target
-Cargo.lock

tests/testsuite/cargo_init/git_autodetect/out/.gitignore

@@ -1,2 +1 @@
 /target
-/Cargo.lock

tests/testsuite/cargo_init/git_ignore_exists_no_conflicting_entries/out/.gitignore

@@ -3,4 +3,3 @@
 # Added by cargo
 
 /target
-/Cargo.lock

tests/testsuite/cargo_init/inferred_lib_with_git/out/.gitignore

@@ -1,2 +1 @@
 /target
-/Cargo.lock

tests/testsuite/cargo_init/mercurial_autodetect/out/.hgignore

@@ -1,2 +1 @@
 ^target$
-^Cargo.lock$

tests/testsuite/cargo_init/pijul_autodetect/out/.ignore

@@ -1,2 +1 @@
 /target
-/Cargo.lock

tests/testsuite/cargo_init/simple_git/out/.gitignore

@@ -1,2 +1 @@
 /target
-/Cargo.lock

tests/testsuite/cargo_init/simple_git_ignore_exists/out/.gitignore

@@ -6,4 +6,3 @@
 # already existing elements were commented out
 
 #/target
-/Cargo.lock

tests/testsuite/cargo_init/simple_hg/out/.hgignore

@@ -1,2 +1 @@
 ^target$
-^Cargo.lock$

tests/testsuite/cargo_init/simple_hg_ignore_exists/out/.hgignore

@@ -3,4 +3,3 @@
 # Added by cargo
 
 ^target$
-^Cargo.lock$

tests/testsuite/new.rs

@@ -95,7 +95,7 @@ fn simple_git() {
 
     let fp = paths::root().join("foo/.gitignore");
     let contents = fs::read_to_string(&fp).unwrap();
-    assert_eq!(contents, "/target\n/Cargo.lock\n",);
+    assert_eq!(contents, "/target\n",);
 
     cargo_process("build").cwd(&paths::root().join("foo")).run();
 }
@@ -112,7 +112,7 @@ fn simple_hg() {
 
     let fp = paths::root().join("foo/.hgignore");
     let contents = fs::read_to_string(&fp).unwrap();
-    assert_eq!(contents, "^target$\n^Cargo.lock$\n",);
+    assert_eq!(contents, "^target$\n",);
 
     cargo_process("build").cwd(&paths::root().join("foo")).run();
 }