Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check in package/publish if a git submodule is dirty. #7245

Merged
merged 2 commits into from
Aug 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,14 +267,32 @@ fn check_repo_state(
allow_dirty: bool,
) -> CargoResult<Option<String>> {
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| {
let relative = file.strip_prefix(workdir).unwrap();
if let Ok(status) = repo.status_file(relative) {
status != git2::Status::CURRENT
} else {
false
submodule_dirty(file)
}
})
.map(|path| {
Expand All @@ -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
Expand Down
114 changes: 114 additions & 0 deletions tests/testsuite/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
8 changes: 6 additions & 2 deletions tests/testsuite/support/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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", "[email protected]"));
t!(cfg.set_str("user.name", "Foo Bar"));
drop(cfg);
repo
}

/// Create a new git repository with a project.
Expand Down Expand Up @@ -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));
Expand Down