Auto merge of #13960 - tweag:issue-13695, r=weihanglo

bors · bors · commit 95d16bed8ccf · 2024-06-24T13:43:15.000Z
Include vcs_info even if workspace is dirty ### What does this PR try to resolve? Related to #13695. Generates and packages the `.cargo_vcs_info.json` file even if the worktree is dirty, as long as `--allow-dirty` is passed. Also added a `dirty` field to the file to record if the Git repository status is dirty. Tests are included.
diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs
@@ -81,6 +81,9 @@ struct VcsInfo {
 #[derive(Serialize)]
 struct GitVcsInfo {
     sha1: String,
+    /// Indicate whether or not the Git worktree is dirty.
+    #[serde(skip_serializing_if = "std::ops::Not::not")]
+    dirty: bool,
 }
 
 /// Packages a single package in a workspace, returning the resulting tar file.
@@ -235,14 +238,8 @@ fn prepare_archive(
     }
     let src_files = src.list_files(pkg)?;
 
-    // Check (git) repository state, getting the current commit hash if not
-    // dirty.
-    let vcs_info = if !opts.allow_dirty {
-        // This will error if a dirty repo is found.
-        check_repo_state(pkg, &src_files, gctx)?
-    } else {
-        None
-    };
+    // Check (git) repository state, getting the current commit hash.
+    let vcs_info = check_repo_state(pkg, &src_files, gctx, &opts)?;
 
     build_ar_list(ws, pkg, src_files, vcs_info)
 }
@@ -559,13 +556,15 @@ fn check_metadata(pkg: &Package, gctx: &GlobalContext) -> CargoResult<()> {
 }
 
 /// Checks if the package source is in a *git* DVCS repository. If *git*, and
-/// the source is *dirty* (e.g., has uncommitted changes) then `bail!` with an
-/// informative message. Otherwise return the sha1 hash of the current *HEAD*
-/// commit, or `None` if no repo is found.
+/// the source is *dirty* (e.g., has uncommitted changes), and `--allow-dirty`
+/// has not been passed, then `bail!` with an informative message. Otherwise
+/// return the sha1 hash of the current *HEAD* commit, or `None` if no repo is
+/// found.
 fn check_repo_state(
     p: &Package,
     src_files: &[PathBuf],
     gctx: &GlobalContext,
+    opts: &PackageOpts<'_>,
 ) -> CargoResult<Option<VcsInfo>> {
     if let Ok(repo) = git2::Repository::discover(p.root()) {
         if let Some(workdir) = repo.workdir() {
@@ -585,7 +584,7 @@ fn check_repo_state(
                         .unwrap_or("")
                         .replace("\\", "/");
                     return Ok(Some(VcsInfo {
-                        git: git(p, src_files, &repo)?,
+                        git: git(p, src_files, &repo, &opts)?,
                         path_in_vcs,
                     }));
                 }
@@ -608,7 +607,12 @@ fn check_repo_state(
     // directory is dirty or not, thus we have to assume that it's clean.
     return Ok(None);
 
-    fn git(p: &Package, src_files: &[PathBuf], repo: &git2::Repository) -> CargoResult<GitVcsInfo> {
+    fn git(
+        p: &Package,
+        src_files: &[PathBuf],
+        repo: &git2::Repository,
+        opts: &PackageOpts<'_>,
+    ) -> CargoResult<GitVcsInfo> {
         // This is a collection of any dirty or untracked files. This covers:
         // - new/modified/deleted/renamed/type change (index or worktree)
         // - untracked files (which are "new" worktree files)
@@ -633,10 +637,12 @@ fn check_repo_state(
                     .to_string()
             })
             .collect();
-        if dirty_src_files.is_empty() {
+        let dirty = !dirty_src_files.is_empty();
+        if !dirty || opts.allow_dirty {
             let rev_obj = repo.revparse_single("HEAD")?;
             Ok(GitVcsInfo {
                 sha1: rev_obj.id().to_string(),
+                dirty,
             })
         } else {
             anyhow::bail!(
diff --git a/src/doc/man/cargo-package.md b/src/doc/man/cargo-package.md
@@ -31,8 +31,8 @@ steps:
       executable binary or example target. {{man "cargo-install" 1}} will use the
       packaged lock file if the `--locked` flag is used.
     - A `.cargo_vcs_info.json` file is included that contains information
-      about the current VCS checkout hash if available (not included with
-      `--allow-dirty`).
+      about the current VCS checkout hash if available, as well as a flag if the
+      worktree is dirty.
 3. Extract the `.crate` file and build it to verify it can build.
     - This will rebuild your package from scratch to ensure that it can be
       built from a pristine state. The `--no-verify` flag can be used to skip
@@ -52,12 +52,16 @@ Will generate a `.cargo_vcs_info.json` in the following format
 ```javascript
 {
  "git": {
-   "sha1": "aac20b6e7e543e6dd4118b246c77225e3a3a1302"
+   "sha1": "aac20b6e7e543e6dd4118b246c77225e3a3a1302",
+   "dirty": true
  },
  "path_in_vcs": ""
 }
 ```
 
+`dirty` indicates that the Git worktree was dirty when the package
+was built.
+
 `path_in_vcs` will be set to a repo-relative path for packages
 in subdirectories of the version control repository.
 
diff --git a/src/doc/man/generated_txt/cargo-package.txt b/src/doc/man/generated_txt/cargo-package.txt
@@ -29,8 +29,8 @@ DESCRIPTION
              packaged lock file if the --locked flag is used.
 
           o  A .cargo_vcs_info.json file is included that contains information
-             about the current VCS checkout hash if available (not included
-             with --allow-dirty).
+             about the current VCS checkout hash if available, as well as a
+             flag if the worktree is dirty.
 
        3. Extract the .crate file and build it to verify it can build.
           o  This will rebuild your package from scratch to ensure that it can
@@ -51,11 +51,15 @@ DESCRIPTION
 
            {
             "git": {
-              "sha1": "aac20b6e7e543e6dd4118b246c77225e3a3a1302"
+              "sha1": "aac20b6e7e543e6dd4118b246c77225e3a3a1302",
+              "dirty": true
             },
             "path_in_vcs": ""
            }
 
+       dirty indicates that the Git worktree was dirty when the package was
+       built.
+
        path_in_vcs will be set to a repo-relative path for packages in
        subdirectories of the version control repository.
 
diff --git a/src/doc/src/commands/cargo-package.md b/src/doc/src/commands/cargo-package.md
@@ -26,8 +26,8 @@ steps:
       executable binary or example target. [cargo-install(1)](cargo-install.html) will use the
       packaged lock file if the `--locked` flag is used.
     - A `.cargo_vcs_info.json` file is included that contains information
-      about the current VCS checkout hash if available (not included with
-      `--allow-dirty`).
+      about the current VCS checkout hash if available, as well as a flag if the
+      worktree is dirty.
 3. Extract the `.crate` file and build it to verify it can build.
     - This will rebuild your package from scratch to ensure that it can be
       built from a pristine state. The `--no-verify` flag can be used to skip
@@ -47,12 +47,16 @@ Will generate a `.cargo_vcs_info.json` in the following format
 ```javascript
 {
  "git": {
-   "sha1": "aac20b6e7e543e6dd4118b246c77225e3a3a1302"
+   "sha1": "aac20b6e7e543e6dd4118b246c77225e3a3a1302",
+   "dirty": true
  },
  "path_in_vcs": ""
 }
 ```
 
+`dirty` indicates that the Git worktree was dirty when the package
+was built.
+
 `path_in_vcs` will be set to a repo-relative path for packages
 in subdirectories of the version control repository.
 
diff --git a/src/etc/man/cargo-package.1 b/src/etc/man/cargo-package.1
@@ -43,8 +43,8 @@ packaged lock file if the \fB\-\-locked\fR flag is used.
 .sp
 .RS 4
 \h'-04'\(bu\h'+02'A \fB\&.cargo_vcs_info.json\fR file is included that contains information
-about the current VCS checkout hash if available (not included with
-\fB\-\-allow\-dirty\fR).
+about the current VCS checkout hash if available, as well as a flag if the
+worktree is dirty.
 .RE
 .RE
 .sp
@@ -74,13 +74,17 @@ Will generate a \fB\&.cargo_vcs_info.json\fR in the following format
 .nf
 {
  "git": {
-   "sha1": "aac20b6e7e543e6dd4118b246c77225e3a3a1302"
+   "sha1": "aac20b6e7e543e6dd4118b246c77225e3a3a1302",
+   "dirty": true
  },
  "path_in_vcs": ""
 }
 .fi
 .RE
 .sp
+\fBdirty\fR indicates that the Git worktree was dirty when the package
+was built.
+.sp
 \fBpath_in_vcs\fR will be set to a repo\-relative path for packages
 in subdirectories of the version control repository.
 .sp
diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs
@@ -2610,6 +2610,7 @@ fn include_overrides_gitignore() {
     p.cargo("package --list --allow-dirty")
         .with_stdout(
             "\
+.cargo_vcs_info.json
 Cargo.toml
 Cargo.toml.orig
 ignored.txt
diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs
@@ -703,6 +703,7 @@ fn no_duplicates_from_modified_tracked_files() {
     p.cargo("package --list --allow-dirty")
         .with_stdout(
             "\
+.cargo_vcs_info.json
 Cargo.lock
 Cargo.toml
 Cargo.toml.orig
@@ -1011,6 +1012,7 @@ src/main.rs
         .with_stderr("")
         .with_stdout(
             "\
+.cargo_vcs_info.json
 .gitignore
 Cargo.lock
 Cargo.toml
@@ -1171,6 +1173,111 @@ src/lib.rs
         .run();
 }
 
+#[cargo_test]
+fn issue_13695_allow_dirty_vcs_info() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            edition = "2015"
+            description = "foo"
+            license = "foo"
+            documentation = "foo"
+        "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    let repo = git::init(&p.root());
+    // Initial commit, with no files added.
+    git::commit(&repo);
+
+    // Allowing a dirty worktree results in the vcs file still being included.
+    p.cargo("package --allow-dirty").run();
+
+    let f = File::open(&p.root().join("target/package/foo-0.1.0.crate")).unwrap();
+    validate_crate_contents(
+        f,
+        "foo-0.1.0.crate",
+        &[
+            ".cargo_vcs_info.json",
+            "Cargo.toml",
+            "Cargo.toml.orig",
+            "src/lib.rs",
+        ],
+        &[(
+            ".cargo_vcs_info.json",
+            r#"{
+  "git": {
+    "sha1": "[..]",
+    "dirty": true
+  },
+  "path_in_vcs": ""
+}"#,
+        )],
+    );
+
+    // Listing provides a consistent result.
+    p.cargo("package --list --allow-dirty")
+        .with_stderr("")
+        .with_stdout(
+            "\
+.cargo_vcs_info.json
+Cargo.toml
+Cargo.toml.orig
+src/lib.rs
+",
+        )
+        .run();
+}
+
+#[cargo_test]
+fn issue_13695_allowing_dirty_vcs_info_but_clean() {
+    let p = project().build();
+    let _ = git::repo(&paths::root().join("foo"))
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            edition = "2015"
+            description = "foo"
+            license = "foo"
+            documentation = "foo"
+        "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    // Allowing a dirty worktree despite it being clean.
+    p.cargo("package --allow-dirty").run();
+
+    let f = File::open(&p.root().join("target/package/foo-0.1.0.crate")).unwrap();
+    validate_crate_contents(
+        f,
+        "foo-0.1.0.crate",
+        &[
+            ".cargo_vcs_info.json",
+            "Cargo.toml",
+            "Cargo.toml.orig",
+            "src/lib.rs",
+        ],
+        &[(
+            ".cargo_vcs_info.json",
+            r#"{
+  "git": {
+    "sha1": "[..]"
+  },
+  "path_in_vcs": ""
+}"#,
+        )],
+    );
+}
+
 #[cargo_test]
 fn generated_manifest() {
     let registry = registry::alt_init();
@@ -2333,6 +2440,7 @@ fn finds_git_in_parent() {
     p.cargo("package --list --allow-dirty")
         .with_stdout(
             "\
+.cargo_vcs_info.json
 Cargo.toml
 Cargo.toml.orig
 ignoreme
@@ -2346,6 +2454,7 @@ src/lib.rs
     p.cargo("package --list --allow-dirty")
         .with_stdout(
             "\
+.cargo_vcs_info.json
 .gitignore
 Cargo.toml
 Cargo.toml.orig
@@ -2359,6 +2468,7 @@ src/lib.rs
     p.cargo("package --list --allow-dirty")
         .with_stdout(
             "\
+.cargo_vcs_info.json
 .gitignore
 Cargo.toml
 Cargo.toml.orig
@@ -2621,6 +2731,7 @@ fn deleted_git_working_tree() {
     p.cargo("package --allow-dirty --list")
         .with_stdout(
             "\
+.cargo_vcs_info.json
 Cargo.lock
 Cargo.toml
 Cargo.toml.orig
@@ -2635,6 +2746,7 @@ src/main.rs
     p.cargo("package --allow-dirty --list")
         .with_stdout(
             "\
+.cargo_vcs_info.json
 Cargo.lock
 Cargo.toml
 Cargo.toml.orig
diff --git a/tests/testsuite/publish_lockfile.rs b/tests/testsuite/publish_lockfile.rs
@@ -249,6 +249,7 @@ fn note_resolve_changes() {
 [NOTE] package `multi v0.1.0` added to the packaged Cargo.lock file, was originally sourced from `[..]/foo/multi`
 [NOTE] package `patched v1.0.0` added to the packaged Cargo.lock file, was originally sourced from `[..]/foo/patched`
 [PACKAGED] [..] files, [..] ([..] compressed)
+[WARNING] no (git) Cargo.toml found at `target/tmp/[..]/foo/Cargo.toml` in workdir `[..]`
 ",
         )
         .run();

Original file line number	Diff line number	Diff line change
`@@ -249,6 +249,7 @@ fn note_resolve_changes() {`
`249`	`249`	[NOTE] package `multi v0.1.0` added to the packaged Cargo.lock file, was originally sourced from `[..]/foo/multi`
`250`	`250`	[NOTE] package `patched v1.0.0` added to the packaged Cargo.lock file, was originally sourced from `[..]/foo/patched`
`251`	`251`	`[PACKAGED] [..] files, [..] ([..] compressed)`
	`252`	+[WARNING] no (git) Cargo.toml found at `target/tmp/[..]/foo/Cargo.toml` in workdir `[..]`
`252`	`253`	`",`
`253`	`254`	`)`
`254`	`255`	`.run();`