checkout: handle dirty submodules correctly Don't generate conflicts when checking out a modified submodule and the submodule is dirty or modified in the workdir.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201
diff --git a/docs/checkout-internals.md b/docs/checkout-internals.md
index 6147ffd..e0b2583 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 a7dddce..a406159 100644
--- a/src/checkout.c
+++ b/src/checkout.c
@@ -464,7 +464,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/tests/checkout/typechange.c b/tests/checkout/typechange.c
index b4959a3..c2949e3 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;