diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index f24332db8..16667fcdf 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -267,6 +267,24 @@ fn check_repo_state( allow_dirty: bool, ) -> CargoResult> { let workdir = repo.workdir().unwrap(); + + let mut sub_repos = Vec::new(); + open_submodules(repo, &mut sub_repos)?; + // Sort so that longest paths are first, to check nested submodules first. + sub_repos.sort_unstable_by(|a, b| b.0.as_os_str().len().cmp(&a.0.as_os_str().len())); + let submodule_dirty = |path: &Path| -> bool { + sub_repos + .iter() + .filter(|(sub_path, _sub_repo)| path.starts_with(sub_path)) + .any(|(sub_path, sub_repo)| { + let relative = path.strip_prefix(sub_path).unwrap(); + sub_repo + .status_file(relative) + .map(|status| status != git2::Status::CURRENT) + .unwrap_or(false) + }) + }; + let dirty = src_files .iter() .filter(|file| { @@ -274,7 +292,7 @@ fn check_repo_state( if let Ok(status) = repo.status_file(relative) { status != git2::Status::CURRENT } else { - false + submodule_dirty(file) } }) .map(|path| { @@ -300,6 +318,22 @@ fn check_repo_state( Ok(None) } } + + /// Helper to recursively open all submodules. + fn open_submodules( + repo: &git2::Repository, + sub_repos: &mut Vec<(PathBuf, git2::Repository)>, + ) -> CargoResult<()> { + for submodule in repo.submodules()? { + // Ignore submodules that don't open, they are probably not initialized. + // If its files are required, then the verification step should fail. + if let Ok(sub_repo) = submodule.open() { + open_submodules(&sub_repo, sub_repos)?; + sub_repos.push((sub_repo.workdir().unwrap().to_owned(), sub_repo)); + } + } + Ok(()) + } } // Checks for and `bail!` if a source file matches `ROOT/VCS_INFO_FILE`, since diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index db2f4687a..cdc2dd4ef 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -2691,3 +2691,117 @@ fn git_fetch_cli_env_clean() { .env("GIT_DIR", git_proj.root().join(".git")) .run(); } + +#[cargo_test] +fn dirty_submodule() { + // `cargo package` warns for dirty file in submodule. + let (git_project, repo) = git::new_repo("foo", |project| { + project + .file("Cargo.toml", &basic_manifest("foo", "0.5.0")) + // This is necessary because `git::add` is too eager. + .file(".gitignore", "/target") + }); + let git_project2 = git::new("src", |project| { + project.no_manifest().file("lib.rs", "pub fn f() {}") + }); + + let url = path2url(git_project2.root()).to_string(); + git::add_submodule(&repo, &url, Path::new("src")); + + // Submodule added, but not committed. + git_project + .cargo("package --no-verify") + .with_status(101) + .with_stderr( + "\ +[WARNING] manifest has no [..] +See [..] +[ERROR] 1 files in the working directory contain changes that were not yet committed into git: + +.gitmodules + +to proceed despite [..] +", + ) + .run(); + + git::commit(&repo); + git_project.cargo("package --no-verify").run(); + + // Modify file, check for warning. + git_project.change_file("src/lib.rs", ""); + git_project + .cargo("package --no-verify") + .with_status(101) + .with_stderr( + "\ +[WARNING] manifest has no [..] +See [..] +[ERROR] 1 files in the working directory contain changes that were not yet committed into git: + +src/lib.rs + +to proceed despite [..] +", + ) + .run(); + // Commit the change. + let sub_repo = git2::Repository::open(git_project.root().join("src")).unwrap(); + git::add(&sub_repo); + git::commit(&sub_repo); + git::add(&repo); + git::commit(&repo); + git_project.cargo("package --no-verify").run(); + + // Try with a nested submodule. + let git_project3 = git::new("bar", |project| project.no_manifest().file("mod.rs", "")); + let url = path2url(git_project3.root()).to_string(); + git::add_submodule(&sub_repo, &url, Path::new("bar")); + git_project + .cargo("package --no-verify") + .with_status(101) + .with_stderr( + "\ +[WARNING] manifest has no [..] +See [..] +[ERROR] 1 files in the working directory contain changes that were not yet committed into git: + +src/.gitmodules + +to proceed despite [..] +", + ) + .run(); + + // Commit the submodule addition. + git::commit(&sub_repo); + git::add(&repo); + git::commit(&repo); + git_project.cargo("package --no-verify").run(); + // Modify within nested submodule. + git_project.change_file("src/bar/mod.rs", "//test"); + git_project + .cargo("package --no-verify") + .with_status(101) + .with_stderr( + "\ +[WARNING] manifest has no [..] +See [..] +[ERROR] 1 files in the working directory contain changes that were not yet committed into git: + +src/bar/mod.rs + +to proceed despite [..] +", + ) + .run(); + // And commit the change. + let sub_sub_repo = git2::Repository::open(git_project.root().join("src/bar")).unwrap(); + git::add(&sub_sub_repo); + git::commit(&sub_sub_repo); + git::add(&sub_repo); + git::commit(&sub_repo); + git::add(&repo); + git::commit(&repo); + git_project.cargo("package --no-verify").run(); +} diff --git a/tests/testsuite/support/git.rs b/tests/testsuite/support/git.rs index b9b1c515b..1213594c2 100644 --- a/tests/testsuite/support/git.rs +++ b/tests/testsuite/support/git.rs @@ -129,11 +129,14 @@ impl Repository { /// Initialize a new repository at the given path. pub fn init(path: &Path) -> git2::Repository { let repo = t!(git2::Repository::init(path)); + default_repo_cfg(&repo); + repo +} + +fn default_repo_cfg(repo: &git2::Repository) { let mut cfg = t!(repo.config()); t!(cfg.set_str("user.email", "foo@bar.com")); t!(cfg.set_str("user.name", "Foo Bar")); - drop(cfg); - repo } /// Create a new git repository with a project. @@ -193,6 +196,7 @@ pub fn add_submodule<'a>( let path = path.to_str().unwrap().replace(r"\", "/"); let mut s = t!(repo.submodule(url, Path::new(&path), false)); let subrepo = t!(s.open()); + default_repo_cfg(&subrepo); t!(subrepo.remote_add_fetch("origin", "refs/heads/*:refs/heads/*")); let mut origin = t!(subrepo.find_remote("origin")); t!(origin.fetch(&[], None, None));