diff --git a/docs/checkout-internals.md b/docs/checkout-internals.md index 6147ffdd8..e0b2583b5 100644 --- a/docs/checkout-internals.md +++ b/docs/checkout-internals.md @@ -66,6 +66,8 @@ Key - Bi - ignored blob (WD only) - T1,T2,T3 - trees with different SHAs, - Ti - ignored tree (WD only) +- S1,S2 - submodules with different SHAs +- Sd - dirty submodule (WD only) - x - nothing Diff with 2 non-workdir iterators @@ -162,6 +164,27 @@ Checkout From 3 Iterators (2 not workdir, 1 workdir) | 35+ | T1 | T2 | x | update locally deleted tree (SAFE+MISSING) | | 36* | T1 | T2 | B1/Bi | update to tree with typechanged tree->blob conflict (F-1) | | 37 | T1 | T2 | T1/T2/T3 | update to existing tree (MAYBE SAFE) | +| 38+ | x | S1 | x | add submodule (SAFE) | +| 39 | x | S1 | S1/Sd | independently added submodule (SUBMODULE) | +| 40* | x | S1 | B1 | add submodule with blob confilct (FORCEABLE) | +| 41* | x | S1 | T1 | add submodule with tree conflict (FORCEABLE) | +| 42 | S1 | x | S1/Sd | deleted submodule (SUBMODULE) | +| 43 | S1 | x | x | independently deleted submodule (SUBMODULE) | +| 44 | S1 | x | B1 | independently deleted submodule with added blob (SAFE+MISSING) | +| 45 | S1 | x | T1 | independently deleted submodule with added tree (SAFE+MISSING) | +| 46 | S1 | S1 | x | locally deleted submodule (SUBMODULE) | +| 47+ | S1 | S2 | x | update locally deleted submodule (SAFE) | +| 48 | S1 | S1 | S2 | locally updated submodule commit (SUBMODULE) | +| 49 | S1 | S2 | S1 | updated submodule commit (SUBMODULE) | +| 50+ | S1 | B1 | x | add blob with locally deleted submodule (SAFE+MISSING) | +| 51* | S1 | B1 | S1 | typechange submodule->blob (SAFE) | +| 52* | S1 | B1 | Sd | typechange dirty submodule->blob (SAFE!?!?) | +| 53+ | S1 | T1 | x | add tree with locally deleted submodule (SAFE+MISSING) | +| 54* | S1 | T1 | S1/Sd | typechange submodule->tree (MAYBE SAFE) | +| 55+ | B1 | S1 | x | add submodule with locally deleted blob (SAFE+MISSING) | +| 56* | B1 | S1 | B1 | typechange blob->submodule (SAFE) | +| 57+ | T1 | S1 | x | add submodule with locally deleted tree (SAFE+MISSING) | +| 58* | T1 | S1 | T1 | typechange tree->submodule (SAFE) | The number is followed by ' ' if no change is needed or '+' if the case @@ -176,6 +199,8 @@ There are four tiers of safe cases: content, which is unknown at this point * FORCEABLE == conflict unless FORCE is given * DIRTY == no conflict but change is not applied unless FORCE +* SUBMODULE == no conflict and no change is applied unless a deleted + submodule dir is empty Some slightly unusual circumstances: @@ -198,7 +223,9 @@ Some slightly unusual circumstances: cases, if baseline == target, we don't touch the workdir (it is either already right or is "dirty"). However, since this case also implies that a ?/B1/x case will exist as well, it can be skipped. +* 41 - It's not clear how core git distinguishes this case from 39 (mode?). +* 52 - Core git makes destructive changes without any warning when the + submodule is dirty and the type changes to a blob. Cases 3, 17, 24, 26, and 29 are all considered conflicts even though none of them will require making any updates to the working directory. - diff --git a/src/checkout.c b/src/checkout.c index d84b46ba7..b3e95dff8 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -482,7 +482,8 @@ static int checkout_action_with_wd( *action = CHECKOUT_ACTION_IF(SAFE, REMOVE, NONE); break; case GIT_DELTA_MODIFIED: /* case 16, 17, 18 (or 36 but not really) */ - if (checkout_is_workdir_modified(data, &delta->old_file, &delta->new_file, wd)) + if (wd->mode != GIT_FILEMODE_COMMIT && + checkout_is_workdir_modified(data, &delta->old_file, &delta->new_file, wd)) *action = CHECKOUT_ACTION_IF(FORCE, UPDATE_BLOB, CONFLICT); else *action = CHECKOUT_ACTION_IF(SAFE, UPDATE_BLOB, NONE); diff --git a/src/merge.c b/src/merge.c index a0f2405ff..b93851b7e 100644 --- a/src/merge.c +++ b/src/merge.c @@ -2827,6 +2827,7 @@ static int merge_check_workdir(size_t *conflicts, git_repository *repo, git_inde opts.flags |= GIT_DIFF_DISABLE_PATHSPEC_MATCH; opts.pathspec.count = merged_paths->length; opts.pathspec.strings = (char **)merged_paths->contents; + opts.ignore_submodules = GIT_SUBMODULE_IGNORE_ALL; if ((error = git_diff_index_to_workdir(&wd_diff_list, repo, NULL, &opts)) < 0) goto done; diff --git a/tests/checkout/typechange.c b/tests/checkout/typechange.c index b4959a351..c2949e3da 100644 --- a/tests/checkout/typechange.c +++ b/tests/checkout/typechange.c @@ -6,6 +6,36 @@ static git_repository *g_repo = NULL; +/* +From the test repo used for this test: +-------------------------------------- + +This is a test repo for libgit2 where tree entries have type changes + +The key types that could be found in tree entries are: + +1 - GIT_FILEMODE_NEW = 0000000 +2 - GIT_FILEMODE_TREE = 0040000 +3 - GIT_FILEMODE_BLOB = 0100644 +4 - GIT_FILEMODE_BLOB_EXECUTABLE = 0100755 +5 - GIT_FILEMODE_LINK = 0120000 +6 - GIT_FILEMODE_COMMIT = 0160000 + +I will try to have every type of transition somewhere in the history +of this repo. + +Commits +------- +Initial commit - a(1) b(1) c(1) d(1) e(1) +Create content - a(1->2) b(1->3) c(1->4) d(1->5) e(1->6) +Changes #1 - a(2->3) b(3->4) c(4->5) d(5->6) e(6->2) +Changes #2 - a(3->5) b(4->6) c(5->2) d(6->3) e(2->4) +Changes #3 - a(5->3) b(6->4) c(2->5) d(3->6) e(4->2) +Changes #4 - a(3->2) b(4->3) c(5->4) d(6->5) e(2->6) +Changes #5 - a(2->1) b(3->1) c(4->1) d(5->1) e(6->1) + +*/ + static const char *g_typechange_oids[] = { "79b9f23e85f55ea36a472a902e875bc1121a94cb", "9bdb75b73836a99e3dbeea640a81de81031fdc29", @@ -21,6 +51,14 @@ static bool g_typechange_empty[] = { true, false, false, false, false, false, true, true }; +static const int g_typechange_expected_conflicts[] = { + 1, 2, 3, 3, 2, 3, 2 +}; + +static const int g_typechange_expected_untracked[] = { + 6, 4, 3, 2, 3, 2, 5 +}; + void test_checkout_typechange__initialize(void) { g_repo = cl_git_sandbox_init("typechanges"); @@ -112,12 +150,7 @@ void test_checkout_typechange__checkout_typechanges_safe(void) for (i = 0; g_typechange_oids[i] != NULL; ++i) { cl_git_pass(git_revparse_single(&obj, g_repo, g_typechange_oids[i])); - opts.checkout_strategy = GIT_CHECKOUT_FORCE; - - /* There are bugs in some submodule->tree changes that prevent - * SAFE from passing here, even though the following should work: - */ - /* !i ? GIT_CHECKOUT_FORCE : GIT_CHECKOUT_SAFE; */ + opts.checkout_strategy = !i ? GIT_CHECKOUT_FORCE : GIT_CHECKOUT_SAFE; cl_git_pass(git_checkout_tree(g_repo, obj, &opts)); @@ -190,6 +223,35 @@ static void force_create_file(const char *file) cl_git_rewritefile(file, "yowza!!"); } +static int make_submodule_dirty(git_submodule *sm, const char *name, void *payload) +{ + git_buf submodulepath = GIT_BUF_INIT; + git_buf dirtypath = GIT_BUF_INIT; + git_repository *submodule_repo; + + /* remove submodule directory in preparation for init and repo_init */ + cl_git_pass(git_buf_joinpath( + &submodulepath, + git_repository_workdir(g_repo), + git_submodule_path(sm) + )); + git_futils_rmdir_r(git_buf_cstr(&submodulepath), NULL, GIT_RMDIR_REMOVE_FILES); + + /* initialize submodule and its repository */ + cl_git_pass(git_submodule_init(sm, 1)); + cl_git_pass(git_submodule_repo_init(&submodule_repo, sm, 0)); + + /* create a file in the submodule workdir to make it dirty */ + cl_git_pass( + git_buf_joinpath(&dirtypath, git_repository_workdir(submodule_repo), "dirty")); + force_create_file(git_buf_cstr(&dirtypath)); + + git_buf_free(&dirtypath); + git_buf_free(&submodulepath); + + return 0; +} + void test_checkout_typechange__checkout_with_conflicts(void) { int i; @@ -211,13 +273,17 @@ void test_checkout_typechange__checkout_with_conflicts(void) git_futils_rmdir_r("typechanges/d", NULL, GIT_RMDIR_REMOVE_FILES); p_mkdir("typechanges/d", 0777); /* intentionally empty dir */ force_create_file("typechanges/untracked"); + cl_git_pass(git_submodule_foreach(g_repo, make_submodule_dirty, NULL)); opts.checkout_strategy = GIT_CHECKOUT_SAFE; memset(&cts, 0, sizeof(cts)); cl_git_fail(git_checkout_tree(g_repo, obj, &opts)); - cl_assert(cts.conflicts > 0); - cl_assert(cts.untracked > 0); + cl_assert_equal_i(cts.conflicts, g_typechange_expected_conflicts[i]); + cl_assert_equal_i(cts.untracked, g_typechange_expected_untracked[i]); + cl_assert_equal_i(cts.dirty, 0); + cl_assert_equal_i(cts.updates, 0); + cl_assert_equal_i(cts.ignored, 0); opts.checkout_strategy = GIT_CHECKOUT_FORCE | GIT_CHECKOUT_REMOVE_UNTRACKED;