Commit 4ece3e225b566816598238902667552dee4c7deb
2014-04-01T12:19:11
Fix submodule accounting for name and path changes Wrote tests that try adding, removing, and updating the name of submodules which showed a number of problems with how we account for changes when incrementally updating the submodule info. Most of these issues didn't exist before because reloading would always blow away the old submodule data.
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
diff --git a/src/submodule.c b/src/submodule.c
index 5061cad..913c97a 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -1345,8 +1345,9 @@ static int submodule_load_from_config(
const git_config_entry *entry, void *payload)
{
git_submodule_cache *cache = payload;
- const char *namestart, *property, *alternate = NULL;
+ const char *namestart, *property;
const char *key = entry->name, *value = entry->value, *path;
+ char *alternate = NULL, *replaced = NULL;
git_buf name = GIT_BUF_INIT;
git_submodule *sm = NULL;
int error = 0;
@@ -1377,17 +1378,39 @@ static int submodule_load_from_config(
* should be strcasecmp
*/
- if (strcmp(sm->name, name.ptr) != 0) {
- alternate = sm->name = git_buf_detach(&name);
- } else if (path && strcmp(path, sm->path) != 0) {
- alternate = sm->path = git__strdup(value);
- if (!sm->path) {
- error = -1;
- goto done;
+ if (strcmp(sm->name, name.ptr) != 0) { /* name changed */
+ if (!strcmp(sm->path, name.ptr)) { /* already set as path */
+ replaced = sm->name;
+ sm->name = sm->path;
+ } else {
+ if (sm->name != sm->path)
+ replaced = sm->name;
+ alternate = sm->name = git_buf_detach(&name);
+ }
+ }
+ else if (path && strcmp(path, sm->path) != 0) { /* path changed */
+ if (!strcmp(sm->name, value)) { /* already set as name */
+ replaced = sm->path;
+ sm->path = sm->name;
+ } else {
+ if (sm->path != sm->name)
+ replaced = sm->path;
+ if ((alternate = git__strdup(value)) == NULL) {
+ error = -1;
+ goto done;
+ }
+ sm->path = alternate;
}
}
- /* Found a alternate key for the submodule */
+ /* Deregister under name being replaced */
+ if (replaced) {
+ git_strmap_delete(cache->submodules, replaced);
+ git_submodule_free(sm);
+ git__free(replaced);
+ }
+
+ /* Insert under alternate key */
if (alternate) {
void *old_sm = NULL;
git_strmap_insert2(cache->submodules, alternate, sm, old_sm, error);
@@ -1684,6 +1707,8 @@ static int submodule_cache_refresh(git_submodule_cache *cache, int refresh)
GIT_SUBMODULE_STATUS__WD_SCANNED |
GIT_SUBMODULE_STATUS__WD_FLAGS |
GIT_SUBMODULE_STATUS__WD_OID_VALID;
+ else
+ goto cleanup; /* nothing to do */
submodule_cache_clear_flags(cache, mask);
@@ -1726,7 +1751,12 @@ static int submodule_cache_refresh(git_submodule_cache *cache, int refresh)
/* remove submodules that no longer exist */
git_strmap_foreach_value(cache->submodules, sm, {
- if (sm && (sm->flags & GIT_SUBMODULE_STATUS__IN_FLAGS) == 0)
+ /* purge unless in HEAD, index, or .gitmodules; no sm for wd only */
+ if (sm != NULL &&
+ !(sm->flags &
+ (GIT_SUBMODULE_STATUS_IN_HEAD |
+ GIT_SUBMODULE_STATUS_IN_INDEX |
+ GIT_SUBMODULE_STATUS_IN_CONFIG)))
submodule_cache_remove_item(cache, sm, true);
});
diff --git a/tests/submodule/nosubs.c b/tests/submodule/nosubs.c
index 5ef4f42..cabb53e 100644
--- a/tests/submodule/nosubs.c
+++ b/tests/submodule/nosubs.c
@@ -2,6 +2,7 @@
#include "clar_libgit2.h"
#include "posix.h"
+#include "fileops.h"
void test_submodule_nosubs__cleanup(void)
{
@@ -93,3 +94,69 @@ void test_submodule_nosubs__bad_gitmodules(void)
cl_git_pass(git_submodule_lookup(NULL, repo, "foobar"));
cl_assert_equal_i(GIT_ENOTFOUND, git_submodule_lookup(NULL, repo, "subdir"));
}
+
+void test_submodule_nosubs__add_and_delete(void)
+{
+ git_repository *repo = cl_git_sandbox_init("status");
+ git_submodule *sm;
+ git_buf buf = GIT_BUF_INIT;
+
+ /* note the lack of calls to git_submodule_reload - this *should* work */
+
+ cl_git_fail(git_submodule_lookup(NULL, repo, "libgit2"));
+ cl_git_fail(git_submodule_lookup(NULL, repo, "submodules/libgit2"));
+
+ /* create */
+
+ cl_git_pass(git_submodule_add_setup(
+ &sm, repo, "https://github.com/libgit2/libgit2.git", "submodules/libgit2", 1));
+ cl_assert_equal_s("submodules/libgit2", git_submodule_name(sm));
+ cl_assert_equal_s("submodules/libgit2", git_submodule_path(sm));
+ git_submodule_free(sm);
+
+ cl_git_pass(git_futils_readbuffer(&buf, "status/.gitmodules"));
+ cl_assert(strstr(buf.ptr, "[submodule \"submodules/libgit2\"]") != NULL);
+ cl_assert(strstr(buf.ptr, "path = submodules/libgit2") != NULL);
+ git_buf_free(&buf);
+
+ /* lookup */
+
+ cl_git_fail(git_submodule_lookup(&sm, repo, "libgit2"));
+ cl_git_pass(git_submodule_lookup(&sm, repo, "submodules/libgit2"));
+ cl_assert_equal_s("submodules/libgit2", git_submodule_name(sm));
+ cl_assert_equal_s("submodules/libgit2", git_submodule_path(sm));
+ git_submodule_free(sm);
+
+ /* update name */
+
+ cl_git_rewritefile(
+ "status/.gitmodules",
+ "[submodule \"libgit2\"]\n"
+ " path = submodules/libgit2\n"
+ " url = https://github.com/libgit2/libgit2.git\n");
+
+ cl_git_pass(git_submodule_lookup(&sm, repo, "libgit2"));
+ cl_assert_equal_s("libgit2", git_submodule_name(sm));
+ cl_assert_equal_s("submodules/libgit2", git_submodule_path(sm));
+ git_submodule_free(sm);
+ cl_git_pass(git_submodule_lookup(&sm, repo, "submodules/libgit2"));
+ git_submodule_free(sm);
+
+ /* revert name update */
+
+ cl_git_rewritefile(
+ "status/.gitmodules",
+ "[submodule \"submodules/libgit2\"]\n"
+ " path = submodules/libgit2\n"
+ " url = https://github.com/libgit2/libgit2.git\n");
+
+ cl_git_fail(git_submodule_lookup(&sm, repo, "libgit2"));
+ cl_git_pass(git_submodule_lookup(&sm, repo, "submodules/libgit2"));
+ git_submodule_free(sm);
+
+ /* remove completely */
+
+ cl_must_pass(p_unlink("status/.gitmodules"));
+ cl_git_fail(git_submodule_lookup(&sm, repo, "libgit2"));
+ cl_git_fail(git_submodule_lookup(&sm, repo, "submodules/libgit2"));
+}