Commit dfda2f68 by Carlos Martín Nieto

submodule: remove the per-repo cache

Having this cache and giving them out goes against our multithreading
guarantees and it makes it impossible to use submodules in a
multi-threaded environment, as any thread can ask for a refresh which
may reallocate some string in the submodule struct which we've accessed
in a different one via a getter.

This makes the submodules behave more like remotes, where each object is
created upon request and not shared except explicitly by the user. This
means that some tests won't pass yet, as they assume they can affect the
submodule objects in the cache and that will affect later operations.
parent 0c94deb9
...@@ -1860,11 +1860,6 @@ static int checkout_create_submodules( ...@@ -1860,11 +1860,6 @@ static int checkout_create_submodules(
git_diff_delta *delta; git_diff_delta *delta;
size_t i; size_t i;
/* initial reload of submodules if .gitmodules was changed */
if (data->reload_submodules &&
(error = git_submodule_reload_all(data->repo, 1)) < 0)
return error;
git_vector_foreach(&data->diff->deltas, i, delta) { git_vector_foreach(&data->diff->deltas, i, delta) {
if (actions[i] & CHECKOUT_ACTION__DEFER_REMOVE) { if (actions[i] & CHECKOUT_ACTION__DEFER_REMOVE) {
/* this has a blocker directory that should only be removed iff /* this has a blocker directory that should only be removed iff
...@@ -1885,8 +1880,7 @@ static int checkout_create_submodules( ...@@ -1885,8 +1880,7 @@ static int checkout_create_submodules(
} }
} }
/* final reload once submodules have been updated */ return 0;
return git_submodule_reload_all(data->repo, 1);
} }
static int checkout_lookup_head_tree(git_tree **out, git_repository *repo) static int checkout_lookup_head_tree(git_tree **out, git_repository *repo)
......
...@@ -109,7 +109,6 @@ void git_repository__cleanup(git_repository *repo) ...@@ -109,7 +109,6 @@ void git_repository__cleanup(git_repository *repo)
git_cache_clear(&repo->objects); git_cache_clear(&repo->objects);
git_attr_cache_flush(repo); git_attr_cache_flush(repo);
git_submodule_cache_free(repo);
set_config(repo, NULL); set_config(repo, NULL);
set_index(repo, NULL); set_index(repo, NULL);
......
...@@ -121,7 +121,6 @@ struct git_repository { ...@@ -121,7 +121,6 @@ struct git_repository {
git_refdb *_refdb; git_refdb *_refdb;
git_config *_config; git_config *_config;
git_index *_index; git_index *_index;
git_submodule_cache *_submodules;
git_cache objects; git_cache objects;
git_attr_cache *attrcache; git_attr_cache *attrcache;
......
...@@ -99,23 +99,6 @@ struct git_submodule { ...@@ -99,23 +99,6 @@ struct git_submodule {
git_oid wd_oid; git_oid wd_oid;
}; };
/**
* The git_submodule_cache stores known submodules along with timestamps,
* etc. about when they were loaded
*/
typedef struct {
git_repository *repo;
git_strmap *submodules;
git_mutex lock;
/* cache invalidation data */
git_oid head_id;
git_oid index_checksum;
git_buf gitmodules_path;
git_futils_filestamp gitmodules_stamp;
git_futils_filestamp config_stamp;
} git_submodule_cache;
/* Force revalidation of submodule data cache (alloc as needed) */ /* Force revalidation of submodule data cache (alloc as needed) */
extern int git_submodule_cache_refresh(git_repository *repo); extern int git_submodule_cache_refresh(git_repository *repo);
...@@ -137,9 +120,6 @@ enum { ...@@ -137,9 +120,6 @@ enum {
#define GIT_SUBMODULE_STATUS__CLEAR_INTERNAL(S) \ #define GIT_SUBMODULE_STATUS__CLEAR_INTERNAL(S) \
((S) & ~(0xFFFFFFFFu << 20)) ((S) & ~(0xFFFFFFFFu << 20))
/* Internal submodule check does not attempt to refresh cached data */
extern bool git_submodule__is_submodule(git_repository *repo, const char *name);
/* Internal lookup does not attempt to refresh cached data */ /* Internal lookup does not attempt to refresh cached data */
extern int git_submodule__lookup( extern int git_submodule__lookup(
git_submodule **out, git_repository *repo, const char *path); git_submodule **out, git_repository *repo, const char *path);
......
...@@ -50,8 +50,6 @@ void test_submodule_modify__init(void) ...@@ -50,8 +50,6 @@ void test_submodule_modify__init(void)
/* call init and see that settings are copied */ /* call init and see that settings are copied */
cl_git_pass(git_submodule_foreach(g_repo, init_one_submodule, NULL)); cl_git_pass(git_submodule_foreach(g_repo, init_one_submodule, NULL));
git_submodule_reload_all(g_repo, 1);
/* confirm submodule data in config */ /* confirm submodule data in config */
cl_git_pass(git_repository_config_snapshot(&cfg, g_repo)); cl_git_pass(git_repository_config_snapshot(&cfg, g_repo));
cl_git_pass(git_config_get_string(&str, cfg, "submodule.sm_unchanged.url")); cl_git_pass(git_config_get_string(&str, cfg, "submodule.sm_unchanged.url"));
......
...@@ -21,19 +21,11 @@ void test_submodule_nosubs__lookup(void) ...@@ -21,19 +21,11 @@ void test_submodule_nosubs__lookup(void)
cl_assert_equal_i(GIT_EEXISTS, git_submodule_lookup(&sm, repo, "subrepo")); cl_assert_equal_i(GIT_EEXISTS, git_submodule_lookup(&sm, repo, "subrepo"));
cl_git_pass(git_submodule_reload_all(repo, 0));
cl_assert_equal_i(GIT_ENOTFOUND, git_submodule_lookup(&sm, repo, "subdir")); cl_assert_equal_i(GIT_ENOTFOUND, git_submodule_lookup(&sm, repo, "subdir"));
cl_assert_equal_i(GIT_EEXISTS, git_submodule_lookup(&sm, repo, "subrepo")); cl_assert_equal_i(GIT_EEXISTS, git_submodule_lookup(&sm, repo, "subrepo"));
} }
void test_submodule_nosubs__immediate_reload(void)
{
git_repository *repo = cl_git_sandbox_init("status");
cl_git_pass(git_submodule_reload_all(repo, 0));
}
static int fake_submod_cb(git_submodule *sm, const char *n, void *p) static int fake_submod_cb(git_submodule *sm, const char *n, void *p)
{ {
GIT_UNUSED(sm); GIT_UNUSED(n); GIT_UNUSED(p); GIT_UNUSED(sm); GIT_UNUSED(n); GIT_UNUSED(p);
...@@ -57,41 +49,7 @@ void test_submodule_nosubs__add(void) ...@@ -57,41 +49,7 @@ void test_submodule_nosubs__add(void)
git_submodule_free(sm2); git_submodule_free(sm2);
cl_git_pass(git_submodule_foreach(repo, fake_submod_cb, NULL)); cl_git_pass(git_submodule_foreach(repo, fake_submod_cb, NULL));
cl_git_pass(git_submodule_reload_all(repo, 0));
git_submodule_free(sm);
}
void test_submodule_nosubs__reload_add_reload(void)
{
git_repository *repo = cl_git_sandbox_init("status");
git_submodule *sm;
cl_git_pass(git_submodule_reload_all(repo, 0));
/* try one add with a reload (to make sure no errors happen) */
cl_git_pass(git_submodule_add_setup(&sm, repo,
"https://github.com/libgit2/libgit2.git", "submodules/libgit2", 1));
cl_git_pass(git_submodule_reload_all(repo, 0));
cl_assert_equal_s("submodules/libgit2", git_submodule_name(sm));
git_submodule_free(sm);
cl_git_pass(git_submodule_lookup(&sm, repo, "submodules/libgit2"));
cl_assert_equal_s("submodules/libgit2", git_submodule_name(sm));
git_submodule_free(sm);
/* try one add without a reload (to make sure cache inval works, too) */
cl_git_pass(git_submodule_add_setup(&sm, repo,
"https://github.com/libgit2/libgit2.git", "libgit2-again", 1));
cl_assert_equal_s("libgit2-again", git_submodule_name(sm));
git_submodule_free(sm);
cl_git_pass(git_submodule_lookup(&sm, repo, "libgit2-again"));
cl_assert_equal_s("libgit2-again", git_submodule_name(sm));
git_submodule_free(sm); git_submodule_free(sm);
} }
...@@ -100,10 +58,8 @@ void test_submodule_nosubs__bad_gitmodules(void) ...@@ -100,10 +58,8 @@ void test_submodule_nosubs__bad_gitmodules(void)
git_repository *repo = cl_git_sandbox_init("status"); git_repository *repo = cl_git_sandbox_init("status");
cl_git_mkfile("status/.gitmodules", "[submodule \"foobar\"]\tpath=blargle\n\turl=\n\tbranch=\n\tupdate=flooble\n\n"); cl_git_mkfile("status/.gitmodules", "[submodule \"foobar\"]\tpath=blargle\n\turl=\n\tbranch=\n\tupdate=flooble\n\n");
cl_git_fail(git_submodule_reload_all(repo, 0));
cl_git_rewritefile("status/.gitmodules", "[submodule \"foobar\"]\tpath=blargle\n\turl=\n\tbranch=\n\tupdate=rebase\n\n"); cl_git_rewritefile("status/.gitmodules", "[submodule \"foobar\"]\tpath=blargle\n\turl=\n\tbranch=\n\tupdate=rebase\n\n");
cl_git_pass(git_submodule_reload_all(repo, 0));
cl_git_pass(git_submodule_lookup(NULL, repo, "foobar")); cl_git_pass(git_submodule_lookup(NULL, repo, "foobar"));
cl_assert_equal_i(GIT_ENOTFOUND, git_submodule_lookup(NULL, repo, "subdir")); cl_assert_equal_i(GIT_ENOTFOUND, git_submodule_lookup(NULL, repo, "subdir"));
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment