fix: revert the behavior checking lockfile's VCS

Lockfile might be gitignore'd,
So it never shows up in `git status`.

However, `cargo packag` vcs checks actually performs
`git status --untracked --ignored`.
It is a bit confusing that lockfile is ignored but still counts as dirty
from the report of `cargo package`.

There are some more nuances:
We check lockfile's VCS status if the lockfile is ouside the current
package root. That means a non-workspace Cargo package will not have
this VCS check. I don't think it is good that we have diverged behaviors
Hence this revert.

We can always re-evaluate later,
as we've reserved rooms for doing more dirty checks.

https://github.com/rust-lang/cargo/issues/14967#issuecomment-2651329783
This commit is contained in:
Weihang Lo 2025-03-21 13:14:33 -04:00
parent e3b5faaaef
commit 259128d270
No known key found for this signature in database
GPG Key ID: D7DBF189825E82E7
2 changed files with 0 additions and 163 deletions

View File

@ -11,7 +11,6 @@ use tracing::debug;
use crate::core::Package;
use crate::core::Workspace;
use crate::ops::lockfile::LOCKFILE_NAME;
use crate::sources::PathEntry;
use crate::CargoResult;
use crate::GlobalContext;
@ -236,15 +235,10 @@ fn git(
/// * `package.readme` and `package.license-file` pointing to paths outside package root
/// * symlinks targets reside outside package root
/// * Any change in the root workspace manifest, regardless of what has changed.
/// * Changes in the lockfile [^1].
///
/// This is required because those paths may link to a file outside the
/// current package root, but still under the git workdir, affecting the
/// final packaged `.crate` file.
///
/// [^1]: Lockfile might be re-generated if it is too out of sync with the manifest.
/// Therefore, even you have a modified lockfile,
/// you might still get a new fresh one that matches what is in git index.
fn dirty_files_outside_pkg_root(
ws: &Workspace<'_>,
pkg: &Package,
@ -263,41 +257,12 @@ fn dirty_files_outside_pkg_root(
.map(|path| paths::normalize_path(&pkg_root.join(path)))
.collect();
// Unlike other files, lockfile is allowed to be missing,
// and can be generated during packaging.
// We skip checking when it is missing in both workdir and git index,
// otherwise cargo will fail with git2 not found error.
let lockfile_path = ws.lock_root().as_path_unlocked().join(LOCKFILE_NAME);
let lockfile_path = if lockfile_path.exists() {
Some(lockfile_path)
} else if let Ok(rel_path) = paths::normalize_path(&lockfile_path).strip_prefix(workdir) {
// We don't canonicalize here because non-existing path can't be canonicalized.
match repo.status_file(&rel_path) {
Ok(s) if s != git2::Status::CURRENT => {
dirty_files.insert(lockfile_path);
}
// Unmodified
Ok(_) => {}
Err(e) => {
debug!(
"check git status failed for `{}` in workdir `{}`: {e}",
rel_path.display(),
workdir.display(),
);
}
}
None
} else {
None
};
for rel_path in src_files
.iter()
.filter(|p| p.is_symlink_or_under_symlink())
.map(|p| p.as_ref().as_path())
.chain(metadata_paths.iter().map(AsRef::as_ref))
.chain([ws.root_manifest()])
.chain(lockfile_path.as_deref().into_iter())
// If inside package root. Don't bother checking git status.
.filter(|p| paths::strip_prefix_canonical(p, pkg_root).is_err())
// Handle files outside package root but under git workdir,

View File

@ -3,7 +3,6 @@
use std::fs::{self, read_to_string, File};
use std::path::Path;
use cargo_test_support::compare::assert_e2e;
use cargo_test_support::prelude::*;
use cargo_test_support::publish::validate_crate_contents;
use cargo_test_support::registry::{self, Package};
@ -1426,133 +1425,6 @@ edition = "2021"
);
}
#[cargo_test]
fn dirty_ws_lockfile_dirty() {
let (p, repo) = git::new_repo("foo", |p| {
p.file(
"Cargo.toml",
r#"
[workspace]
members = ["isengard"]
resolver = "2"
[workspace.package]
edition = "2015"
"#,
)
.file(
"isengard/Cargo.toml",
r#"
[package]
name = "isengard"
edition.workspace = true
homepage = "saruman"
description = "saruman"
license = "MIT"
"#,
)
.file("isengard/src/lib.rs", "")
});
git::commit(&repo);
p.cargo("package --workspace --no-verify")
.with_stderr_data(str![[r#"
[PACKAGING] isengard v0.0.0 ([ROOT]/foo/isengard)
[PACKAGED] 5 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
"#]])
.run();
// lockfile is untracked.
p.cargo("generate-lockfile").run();
p.cargo("package --workspace --no-verify")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] 1 files in the working directory contain changes that were not yet committed into git:
Cargo.lock
to proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag
"#]])
.run();
// lockfile is tracked.
p.cargo("clean").run();
git::add(&repo);
git::commit(&repo);
p.cargo("package --workspace --no-verify")
.with_stderr_data(str![[r#"
[PACKAGING] isengard v0.0.0 ([ROOT]/foo/isengard)
[PACKAGED] 5 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
"#]])
.run();
// Simulate that Cargo.lock had some outdated but SemVer compat dependency changes.
Package::new("dep", "1.0.0").publish();
Package::new("dep", "1.1.0").publish();
p.cargo("clean").run();
p.cargo("add dep@1").run();
git::add(&repo);
git::commit(&repo);
// make sure we have dep@1.1.0 in lockfile
assert_e2e().eq(
&p.read_lockfile(),
str![[r##"
# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
version = 4
[[package]]
name = "dep"
version = "1.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "77d3d6a4f2203d590707cc803c94afbe36393bbdba757ef66986f39159eaab51"
[[package]]
name = "isengard"
version = "0.0.0"
dependencies = [
"dep",
]
"##]],
);
p.cargo("update dep --precise 1.0.0")
.with_stderr_data(str![[r#"
[UPDATING] `dummy-registry` index
[DOWNGRADING] dep v1.1.0 -> v1.0.0
"#]])
.run();
p.cargo("package --workspace --no-verify")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] 1 files in the working directory contain changes that were not yet committed into git:
Cargo.lock
to proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag
"#]])
.run();
// Now check if it is considered dirty when removed.
p.cargo("clean").run();
fs::remove_file(p.root().join("Cargo.lock")).unwrap();
p.cargo("package --workspace --no-verify")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] 1 files in the working directory contain changes that were not yet committed into git:
Cargo.lock
to proceed despite this and include the uncommitted changes, pass the `--allow-dirty` flag
"#]])
.run();
}
#[cargo_test]
fn issue_13695_allow_dirty_vcs_info() {
let p = project()