From 0105b55e8f7e4d8194400b341b6e0425182ff636 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 27 Jun 2013 15:26:31 -0700 Subject: [PATCH 01/15] Add another submodule test of dirty wd --- tests-clar/diff/submodules.c | 39 ++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests-clar/diff/submodules.c b/tests-clar/diff/submodules.c index 5de46732b..1c9940cc5 100644 --- a/tests-clar/diff/submodules.c +++ b/tests-clar/diff/submodules.c @@ -121,6 +121,45 @@ void test_diff_submodules__dirty_submodule(void) git_diff_list_free(diff); } +void test_diff_submodules__dirty_submodule_2(void) +{ + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + git_diff_list *diff = NULL; + char *smpath = "testrepo"; + static const char *expected_none[] = { NULL, "" }; + static const char *expected_dirty[] = { + "diff --git a/testrepo b/testrepo\nindex a65fedf..a65fedf 160000\n--- a/testrepo\n+++ b/testrepo\n@@ -1 +1 @@\n-Subproject commit a65fedf39aefe402d3bb6e24df4d4f5fe4547750\n+Subproject commit a65fedf39aefe402d3bb6e24df4d4f5fe4547750-dirty\n", /* testrepo.git */ + "" + }; + + setup_submodules(); + + cl_git_pass(git_submodule_reload_all(g_repo)); + + opts.flags = GIT_DIFF_INCLUDE_IGNORED | + GIT_DIFF_INCLUDE_UNTRACKED | + GIT_DIFF_INCLUDE_UNMODIFIED; + opts.pathspec.count = 1; + opts.pathspec.strings = &smpath; + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_none); + git_diff_list_free(diff); + + cl_git_rewritefile("submodules/testrepo/README", "heyheyhey"); + cl_git_mkfile("submodules/testrepo/all_new.txt", "never seen before"); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_dirty); + git_diff_list_free(diff); + + cl_git_pass(git_submodule_reload_all(g_repo)); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_dirty); + git_diff_list_free(diff); +} + void test_diff_submodules__submod2_index_to_wd(void) { git_diff_options opts = GIT_DIFF_OPTIONS_INIT; From 12f8fe0054505d6c3228a5186516f7c5f28d5165 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 27 Jun 2013 15:43:12 -0700 Subject: [PATCH 02/15] More improvements to submodule diff tests This controls for the diff.mnemonicprefix setting so that can't break the tests. Also, this expands one test to emulate an ObjectiveGit test more closely. --- tests-clar/diff/submodules.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/tests-clar/diff/submodules.c b/tests-clar/diff/submodules.c index 1c9940cc5..e3c4e66a3 100644 --- a/tests-clar/diff/submodules.c +++ b/tests-clar/diff/submodules.c @@ -86,6 +86,7 @@ void test_diff_submodules__unmodified_submodule(void) opts.flags = GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED | GIT_DIFF_INCLUDE_UNMODIFIED; + opts.old_prefix = "a"; opts.new_prefix = "b"; cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); check_diff_patches(diff, expected); @@ -115,6 +116,7 @@ void test_diff_submodules__dirty_submodule(void) opts.flags = GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED | GIT_DIFF_INCLUDE_UNMODIFIED; + opts.old_prefix = "a"; opts.new_prefix = "b"; cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); check_diff_patches(diff, expected); @@ -124,9 +126,9 @@ void test_diff_submodules__dirty_submodule(void) void test_diff_submodules__dirty_submodule_2(void) { git_diff_options opts = GIT_DIFF_OPTIONS_INIT; - git_diff_list *diff = NULL; + git_diff_list *diff = NULL, *diff2 = NULL; char *smpath = "testrepo"; - static const char *expected_none[] = { NULL, "" }; + static const char *expected_none[] = { "" }; static const char *expected_dirty[] = { "diff --git a/testrepo b/testrepo\nindex a65fedf..a65fedf 160000\n--- a/testrepo\n+++ b/testrepo\n@@ -1 +1 @@\n-Subproject commit a65fedf39aefe402d3bb6e24df4d4f5fe4547750\n+Subproject commit a65fedf39aefe402d3bb6e24df4d4f5fe4547750-dirty\n", /* testrepo.git */ "" @@ -136,9 +138,11 @@ void test_diff_submodules__dirty_submodule_2(void) cl_git_pass(git_submodule_reload_all(g_repo)); - opts.flags = GIT_DIFF_INCLUDE_IGNORED | - GIT_DIFF_INCLUDE_UNTRACKED | - GIT_DIFF_INCLUDE_UNMODIFIED; + opts.flags = GIT_DIFF_INCLUDE_UNTRACKED | + GIT_DIFF_INCLUDE_UNTRACKED_CONTENT | + GIT_DIFF_RECURSE_UNTRACKED_DIRS | + GIT_DIFF_DISABLE_PATHSPEC_MATCH; + opts.old_prefix = "a"; opts.new_prefix = "b"; opts.pathspec.count = 1; opts.pathspec.strings = &smpath; @@ -151,6 +155,18 @@ void test_diff_submodules__dirty_submodule_2(void) cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); check_diff_patches(diff, expected_dirty); + + { + git_tree *head; + + cl_git_pass(git_repository_head_tree(&head, g_repo)); + cl_git_pass(git_diff_tree_to_index(&diff2, g_repo, head, NULL, &opts)); + cl_git_pass(git_diff_merge(diff, diff2)); + git_diff_list_free(diff2); + + check_diff_patches(diff, expected_dirty); + } + git_diff_list_free(diff); cl_git_pass(git_submodule_reload_all(g_repo)); @@ -179,6 +195,7 @@ void test_diff_submodules__submod2_index_to_wd(void) setup_submodules2(); opts.flags = GIT_DIFF_INCLUDE_UNTRACKED; + opts.old_prefix = "a"; opts.new_prefix = "b"; cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); check_diff_patches(diff, expected); @@ -201,6 +218,7 @@ void test_diff_submodules__submod2_head_to_index(void) cl_git_pass(git_repository_head_tree(&head, g_repo)); opts.flags = GIT_DIFF_INCLUDE_UNTRACKED; + opts.old_prefix = "a"; opts.new_prefix = "b"; cl_git_pass(git_diff_tree_to_index(&diff, g_repo, head, NULL, &opts)); check_diff_patches(diff, expected); From 49621a34af7160d4afbf08a5be15e0e4f3a47791 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 27 Jun 2013 15:46:46 -0700 Subject: [PATCH 03/15] Fix memory leak in test --- tests-clar/diff/submodules.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests-clar/diff/submodules.c b/tests-clar/diff/submodules.c index e3c4e66a3..ffaae3ce4 100644 --- a/tests-clar/diff/submodules.c +++ b/tests-clar/diff/submodules.c @@ -163,6 +163,7 @@ void test_diff_submodules__dirty_submodule_2(void) cl_git_pass(git_diff_tree_to_index(&diff2, g_repo, head, NULL, &opts)); cl_git_pass(git_diff_merge(diff, diff2)); git_diff_list_free(diff2); + git_tree_free(head); check_diff_patches(diff, expected_dirty); } From 3e7d7100e2be8f6c1fe290842ee9a19b854a7046 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 27 Jun 2013 16:12:00 -0700 Subject: [PATCH 04/15] Fix diff test helper to show parent file/line --- tests-clar/diff/submodules.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tests-clar/diff/submodules.c b/tests-clar/diff/submodules.c index ffaae3ce4..c7bdf6d75 100644 --- a/tests-clar/diff/submodules.c +++ b/tests-clar/diff/submodules.c @@ -37,7 +37,8 @@ void test_diff_submodules__cleanup(void) cl_fixture_cleanup("submod2_target"); } -static void check_diff_patches(git_diff_list *diff, const char **expected) +static void check_diff_patches_at_line( + git_diff_list *diff, const char **expected, const char *file, int line) { const git_diff_delta *delta; git_diff_patch *patch = NULL; @@ -48,24 +49,30 @@ static void check_diff_patches(git_diff_list *diff, const char **expected) cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); if (delta->status == GIT_DELTA_UNMODIFIED) { - cl_assert(expected[d] == NULL); + clar__assert(expected[d] == NULL, file, line, "found UNMODIFIED delta where modified was expected", NULL, 1); continue; } if (expected[d] && !strcmp(expected[d], "")) continue; - if (expected[d] && !strcmp(expected[d], "")) - cl_assert(0); + if (expected[d] && !strcmp(expected[d], "")) { + cl_git_pass(git_diff_patch_to_str(&patch_text, patch)); + clar__assert(0, file, line, "expected end of deltas, but found more", patch_text, 1); + } cl_git_pass(git_diff_patch_to_str(&patch_text, patch)); - cl_assert_equal_s(expected[d], patch_text); + clar__assert_equal_s(expected[d], patch_text, file, line, + "expected diff did not match actual diff", 1); git__free(patch_text); } - cl_assert(expected[d] && !strcmp(expected[d], "")); + clar__assert(expected[d] && !strcmp(expected[d], ""), file, line, "found fewer deltas than expected", expected[d], 1); } +#define check_diff_patches(diff, exp) \ + check_diff_patches_at_line(diff, exp, __FILE__, __LINE__) + void test_diff_submodules__unmodified_submodule(void) { git_diff_options opts = GIT_DIFF_OPTIONS_INIT; From 4535f04409a9379ace7cdda0b53a62d3cdf20676 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 27 Jun 2013 16:12:44 -0700 Subject: [PATCH 05/15] More diff submodule tests for cache issues The submodules code caches data about submodules in a way that can cause problems. This adds some tests that try making various modifications to the state of a submodule to see where we can catch out problems in the submodule caching. Right now, I've put in an extra git_submodule_reload_all so that the test will pass, but with that commented out, the test fails. I'm working on fixing the broken version of the test at which point I'll commit the fix and delete the extra reload that makes the test pass. --- tests-clar/diff/submodules.c | 150 +++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/tests-clar/diff/submodules.c b/tests-clar/diff/submodules.c index c7bdf6d75..9b77897b7 100644 --- a/tests-clar/diff/submodules.c +++ b/tests-clar/diff/submodules.c @@ -234,3 +234,153 @@ void test_diff_submodules__submod2_head_to_index(void) git_tree_free(head); } + +void test_diff_submodules__invalid_cache(void) +{ + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + git_diff_list *diff = NULL; + git_submodule *sm; + char *smpath = "sm_changed_head"; + git_repository *smrepo; + git_index *smindex; + static const char *expected_baseline[] = { + "diff --git a/sm_changed_head b/sm_changed_head\nindex 4800958..3d9386c 160000\n--- a/sm_changed_head\n+++ b/sm_changed_head\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 3d9386c507f6b093471a3e324085657a3c2b4247\n", /* sm_changed_head */ + "" + }; + static const char *expected_unchanged[] = { "" }; + static const char *expected_dirty[] = { + "diff --git a/sm_changed_head b/sm_changed_head\nindex 3d9386c..3d9386c 160000\n--- a/sm_changed_head\n+++ b/sm_changed_head\n@@ -1 +1 @@\n-Subproject commit 3d9386c507f6b093471a3e324085657a3c2b4247\n+Subproject commit 3d9386c507f6b093471a3e324085657a3c2b4247-dirty\n", + "" + }; + static const char *expected_moved[] = { + "diff --git a/sm_changed_head b/sm_changed_head\nindex 3d9386c..0910a13 160000\n--- a/sm_changed_head\n+++ b/sm_changed_head\n@@ -1 +1 @@\n-Subproject commit 3d9386c507f6b093471a3e324085657a3c2b4247\n+Subproject commit 0910a13dfa2210496f6c590d75bc360dd11b2a1b\n", + "" + }; + static const char *expected_moved_dirty[] = { + "diff --git a/sm_changed_head b/sm_changed_head\nindex 3d9386c..0910a13 160000\n--- a/sm_changed_head\n+++ b/sm_changed_head\n@@ -1 +1 @@\n-Subproject commit 3d9386c507f6b093471a3e324085657a3c2b4247\n+Subproject commit 0910a13dfa2210496f6c590d75bc360dd11b2a1b-dirty\n", + "" + }; + + setup_submodules2(); + + opts.flags = GIT_DIFF_INCLUDE_UNTRACKED; + opts.old_prefix = "a"; opts.new_prefix = "b"; + opts.pathspec.count = 1; + opts.pathspec.strings = &smpath; + + /* baseline */ + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_baseline); + git_diff_list_free(diff); + + /* update index with new HEAD */ + cl_git_pass(git_submodule_lookup(&sm, g_repo, smpath)); + cl_git_pass(git_submodule_add_to_index(sm, 1)); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_unchanged); + git_diff_list_free(diff); + + /* create untracked file in submodule working directory */ + cl_git_mkfile("submod2/sm_changed_head/new_around_here", "hello"); + git_submodule_set_ignore(sm, GIT_SUBMODULE_IGNORE_NONE); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_dirty); + git_diff_list_free(diff); + + git_submodule_set_ignore(sm, GIT_SUBMODULE_IGNORE_UNTRACKED); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_unchanged); + git_diff_list_free(diff); + + /* modify tracked file in submodule working directory */ + cl_git_append2file( + "submod2/sm_changed_head/file_to_modify", "\nmore stuff\n"); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_dirty); + git_diff_list_free(diff); + + cl_git_pass(git_submodule_reload_all(g_repo)); + cl_git_pass(git_submodule_lookup(&sm, g_repo, smpath)); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_dirty); + git_diff_list_free(diff); + + git_submodule_set_ignore(sm, GIT_SUBMODULE_IGNORE_DIRTY); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_unchanged); + git_diff_list_free(diff); + + /* add file to index in submodule */ + cl_git_pass(git_submodule_open(&smrepo, sm)); + cl_git_pass(git_repository_index(&smindex, smrepo)); + cl_git_pass(git_index_add_bypath(smindex, "file_to_modify")); + + git_submodule_set_ignore(sm, GIT_SUBMODULE_IGNORE_UNTRACKED); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_dirty); + git_diff_list_free(diff); + + git_submodule_set_ignore(sm, GIT_SUBMODULE_IGNORE_DIRTY); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_unchanged); + git_diff_list_free(diff); + + /* commit changed index of submodule */ + { + git_object *parent; + git_oid tree_id, commit_id; + git_tree *tree; + git_signature *sig; + + cl_git_pass(git_revparse_single(&parent, smrepo, "HEAD")); + cl_git_pass(git_index_write_tree(&tree_id, smindex)); + cl_git_pass(git_index_write(smindex)); + cl_git_pass(git_tree_lookup(&tree, smrepo, &tree_id)); + cl_git_pass(git_signature_new(&sig, "Sm Test", "sm@tester.test", 1372350000, 480)); + cl_git_pass(git_commit_create_v( + &commit_id, smrepo, "HEAD", sig, sig, NULL, + "Move it", tree, 1, parent)); + git_object_free(parent); + git_tree_free(tree); + git_signature_free(sig); + } + + /* THIS RELOAD SHOULD NOT BE REQUIRED */ + cl_git_pass(git_submodule_reload_all(g_repo)); + cl_git_pass(git_submodule_lookup(&sm, g_repo, smpath)); + + git_submodule_set_ignore(sm, GIT_SUBMODULE_IGNORE_DIRTY); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_moved); + git_diff_list_free(diff); + + git_submodule_set_ignore(sm, GIT_SUBMODULE_IGNORE_ALL); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_unchanged); + git_diff_list_free(diff); + + git_submodule_set_ignore(sm, GIT_SUBMODULE_IGNORE_NONE); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_moved_dirty); + git_diff_list_free(diff); + + p_unlink("submod2/sm_changed_head/new_around_here"); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_moved); + git_diff_list_free(diff); + + git_index_free(smindex); + git_repository_free(smrepo); +} From 41f1f9d732a7cdd50d58948224ca0693a68995dc Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 27 Jun 2013 16:52:00 -0700 Subject: [PATCH 06/15] Add API to get path to index file --- include/git2/index.h | 8 ++++++++ src/index.c | 6 ++++++ 2 files changed, 14 insertions(+) diff --git a/include/git2/index.h b/include/git2/index.h index 1fb77efa3..b44535601 100644 --- a/include/git2/index.h +++ b/include/git2/index.h @@ -239,6 +239,14 @@ GIT_EXTERN(int) git_index_read(git_index *index); */ GIT_EXTERN(int) git_index_write(git_index *index); +/** + * Get the full path to the index file on disk. + * + * @param index an existing index object + * @return path to index file or NULL for in-memory index + */ +GIT_EXTERN(const char *) git_index_path(git_index *index); + /** * Read a tree into the index file with stats * diff --git a/src/index.c b/src/index.c index 21efd2c63..bd5e192f3 100644 --- a/src/index.c +++ b/src/index.c @@ -516,6 +516,12 @@ int git_index_write(git_index *index) return 0; } +const char * git_index_path(git_index *index) +{ + assert(index); + return index->index_file_path; +} + int git_index_write_tree(git_oid *oid, git_index *index) { git_repository *repo; From e807860fa9b5278932a0ba25f84b4035b0ebda84 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 27 Jun 2013 16:52:38 -0700 Subject: [PATCH 07/15] Add timestamp check to submodule status This is probably not the final form of this change, but this is a preliminary version of checking a timestamp to see if the cached working directory HEAD OID matches the current. Right now, this uses the timestamp on the index and is, like most of our timestamp checking, subject to having only second accuracy. --- src/submodule.c | 42 +++++++++++++++++++++++++++---- src/submodule.h | 48 +++++++++++++++++++++++------------- tests-clar/diff/submodules.c | 13 +++++++--- 3 files changed, 77 insertions(+), 26 deletions(-) diff --git a/src/submodule.c b/src/submodule.c index dcd58d016..e6ed7e33e 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -9,9 +9,7 @@ #include "git2/config.h" #include "git2/sys/config.h" #include "git2/types.h" -#include "git2/repository.h" #include "git2/index.h" -#include "git2/submodule.h" #include "buffer.h" #include "buf_text.h" #include "vector.h" @@ -544,6 +542,15 @@ const git_oid *git_submodule_wd_id(git_submodule *submodule) { assert(submodule); + /* if we know the submodule index timestamp and it has moved, then + * let's reload the working directory information for the submodule + */ + if (submodule->wd_head_path != NULL && + git_futils_filestamp_check( + &submodule->wd_stamp, submodule->wd_head_path)) + submodule->flags &= ~GIT_SUBMODULE_STATUS__WD_OID_VALID; + + /* load unless we think we have a valid oid */ if (!(submodule->flags & GIT_SUBMODULE_STATUS__WD_OID_VALID)) { git_repository *subrepo; @@ -702,11 +709,36 @@ int git_submodule_open( /* if we have opened the submodule successfully, let's grab the HEAD OID */ if (!error) { + git_buf buf = GIT_BUF_INIT; + + /* For now, let's just the index timestamp... + * + * git_buf_joinpath(&buf, git_repository_path(*subrepo), GIT_HEAD_FILE); + * if (!git_path_exists(buf.ptr)) { + */ + git_index *index; + if (!git_repository_index__weakptr(&index, *subrepo)) + git_buf_sets(&buf, git_index_path(index)); + else + git_buf_free(&buf); + /* } */ + + if (git_buf_len(&buf) > 0) { + git__free(submodule->wd_head_path); + submodule->wd_head_path = git_buf_detach(&buf); + } + if (!git_reference_name_to_id( - &submodule->wd_oid, *subrepo, GIT_HEAD_FILE)) + &submodule->wd_oid, *subrepo, GIT_HEAD_FILE)) { + submodule->flags |= GIT_SUBMODULE_STATUS__WD_OID_VALID; - else - giterr_clear(); + + if (submodule->wd_head_path) + git_futils_filestamp_check( + &submodule->wd_stamp, submodule->wd_head_path); + } + + giterr_clear(); } return error; diff --git a/src/submodule.h b/src/submodule.h index ba8e2518e..88d4f97c7 100644 --- a/src/submodule.h +++ b/src/submodule.h @@ -7,6 +7,10 @@ #ifndef INCLUDE_submodule_h__ #define INCLUDE_submodule_h__ +#include "git2/submodule.h" +#include "git2/repository.h" +#include "fileops.h" + /* Notes: * * Submodule information can be in four places: the index, the config files @@ -44,43 +48,53 @@ * an entry for every submodule found in the HEAD and index, and for every * submodule described in .gitmodules. The fields are as follows: * - * - `owner` is the git_repository containing this submodule * - `name` is the name of the submodule from .gitmodules. * - `path` is the path to the submodule from the repo root. It is almost * always the same as `name`. * - `url` is the url for the submodule. - * - `tree_oid` is the SHA1 for the submodule path in the repo HEAD. - * - `index_oid` is the SHA1 for the submodule recorded in the index. - * - `workdir_oid` is the SHA1 for the HEAD of the checked out submodule. * - `update` is a git_submodule_update_t value - see gitmodules(5) update. + * - `update_default` is the update value from the config * - `ignore` is a git_submodule_ignore_t value - see gitmodules(5) ignore. + * - `ignore_default` is the ignore value from the config * - `fetch_recurse` is 0 or 1 - see gitmodules(5) fetchRecurseSubmodules. - * - `refcount` tracks how many hashmap entries there are for this submodule. - * It only comes into play if the name and path of the submodule differ. - * - `flags` is for internal use, tracking where this submodule has been - * found (head, index, config, workdir) and other misc info about it. + * + * - `owner` is the git_repository containing this submodule + * - `flags` after for internal use, tracking where this submodule has been + * found (head, index, config, workdir) and known status info, etc. + * - `head_oid` is the SHA1 for the submodule path in the repo HEAD. + * - `index_oid` is the SHA1 for the submodule recorded in the index. + * - `wd_oid` is the SHA1 for the HEAD of the checked out submodule. + * - `wd_index_path` is the path to the index of the checked out submodule + * - `wd_last_index` is a timestamp of that submodule index so we can + * quickly check if the `wd_oid` should be rechecked + * - `refcount` tracks how many hash table entries in the + * git_submodule_cache there are for this submodule. It only comes into + * play if the name and path of the submodule differ. * * If the submodule has been added to .gitmodules but not yet git added, - * then the `index_oid` will be valid and zero. If the submodule has been - * deleted, but the delete has not been committed yet, then the `index_oid` - * will be set, but the `url` will be NULL. + * then the `index_oid` will be zero but still marked valid. If the + * submodule has been deleted, but the delete has not been committed yet, + * then the `index_oid` will be set, but the `url` will be NULL. */ struct git_submodule { - git_repository *owner; + /* information from config */ char *name; char *path; /* important: may point to same string data as "name" */ char *url; - uint32_t flags; - git_oid head_oid; - git_oid index_oid; - git_oid wd_oid; - /* information from config */ git_submodule_update_t update; git_submodule_update_t update_default; git_submodule_ignore_t ignore; git_submodule_ignore_t ignore_default; int fetch_recurse; + /* internal information */ + git_repository *owner; + uint32_t flags; + git_oid head_oid; + git_oid index_oid; + git_oid wd_oid; + char *wd_head_path; + git_futils_filestamp wd_stamp; int refcount; }; diff --git a/tests-clar/diff/submodules.c b/tests-clar/diff/submodules.c index 9b77897b7..c94fd57c6 100644 --- a/tests-clar/diff/submodules.c +++ b/tests-clar/diff/submodules.c @@ -333,29 +333,34 @@ void test_diff_submodules__invalid_cache(void) check_diff_patches(diff, expected_unchanged); git_diff_list_free(diff); + sleep(2); + /* commit changed index of submodule */ { git_object *parent; git_oid tree_id, commit_id; git_tree *tree; git_signature *sig; + git_reference *ref; - cl_git_pass(git_revparse_single(&parent, smrepo, "HEAD")); + cl_git_pass(git_revparse_ext(&parent, &ref, smrepo, "HEAD")); cl_git_pass(git_index_write_tree(&tree_id, smindex)); cl_git_pass(git_index_write(smindex)); cl_git_pass(git_tree_lookup(&tree, smrepo, &tree_id)); cl_git_pass(git_signature_new(&sig, "Sm Test", "sm@tester.test", 1372350000, 480)); cl_git_pass(git_commit_create_v( - &commit_id, smrepo, "HEAD", sig, sig, NULL, - "Move it", tree, 1, parent)); + &commit_id, smrepo, git_reference_name(ref), sig, sig, + NULL, "Move it", tree, 1, parent)); git_object_free(parent); git_tree_free(tree); + git_reference_free(ref); git_signature_free(sig); } - /* THIS RELOAD SHOULD NOT BE REQUIRED */ + /* THIS RELOAD SHOULD NOT BE REQUIRED cl_git_pass(git_submodule_reload_all(g_repo)); cl_git_pass(git_submodule_lookup(&sm, g_repo, smpath)); + */ git_submodule_set_ignore(sm, GIT_SUBMODULE_IGNORE_DIRTY); From 302a04b09ca706eeda9b8cceb50694f37973e348 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Sat, 29 Jun 2013 12:41:39 -0700 Subject: [PATCH 08/15] Add accessors for refcount value --- src/thread-utils.h | 5 +++++ src/util.h | 3 +++ 2 files changed, 8 insertions(+) diff --git a/src/thread-utils.h b/src/thread-utils.h index f19a2ba2c..f8c4dc66d 100644 --- a/src/thread-utils.h +++ b/src/thread-utils.h @@ -43,6 +43,11 @@ GIT_INLINE(void) git_atomic_set(git_atomic *a, int val) a->val = val; } +GIT_INLINE(int) git_atomic_get(git_atomic *a) +{ + return (int)a->val; +} + #ifdef GIT_THREADS #define git_thread pthread_t diff --git a/src/util.h b/src/util.h index e0088399c..a97c9bf39 100644 --- a/src/util.h +++ b/src/util.h @@ -221,6 +221,9 @@ typedef void (*git_refcount_freeptr)(void *r); #define GIT_REFCOUNT_OWNER(r) (((git_refcount *)(r))->owner) +#define GIT_REFCOUNT_VAL(r) git_atomic_get(&((git_refcount *)(r))->refcount) + + static signed char from_hex[] = { -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, /* 00 */ -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, /* 10 */ From 3fe046cfdba414f69b09e76da2a550be96eeab7e Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Sat, 29 Jun 2013 13:13:38 -0700 Subject: [PATCH 09/15] Add BARE option to git_repository_open_ext This adds a BARE option to git_repository_open_ext which allows a fast open path that still knows how to read gitlinks and to search for the actual .git directory from a subdirectory. `git_repository_open_bare` is still simpler and faster, but having a gitlink aware fast open is very useful for submodules where we want to quickly be able to peek at the HEAD and index data without doing any other meaningful repo operations. --- include/git2/repository.h | 4 +++ src/repository.c | 15 +++++---- tests-clar/repo/open.c | 65 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 74 insertions(+), 10 deletions(-) diff --git a/include/git2/repository.h b/include/git2/repository.h index c81051969..807d834fe 100644 --- a/include/git2/repository.h +++ b/include/git2/repository.h @@ -94,10 +94,14 @@ GIT_EXTERN(int) git_repository_discover( * changes from the `stat` system call). (E.g. Searching in a user's home * directory "/home/user/source/" will not return "/.git/" as the found * repo if "/" is a different filesystem than "/home".) + * * GIT_REPOSITORY_OPEN_BARE - Open repository as a bare repo regardless + * of core.bare config, and defer loading config file for faster setup. + * Unlike `git_repository_open_bare`, this can follow gitlinks. */ typedef enum { GIT_REPOSITORY_OPEN_NO_SEARCH = (1 << 0), GIT_REPOSITORY_OPEN_CROSS_FS = (1 << 1), + GIT_REPOSITORY_OPEN_BARE = (1 << 2), } git_repository_open_flag_t; /** diff --git a/src/repository.c b/src/repository.c index ed9469c59..bd7ef5476 100644 --- a/src/repository.c +++ b/src/repository.c @@ -266,7 +266,7 @@ static int find_ceiling_dir_offset( buf[--len] = '\0'; if (!strncmp(path, buf2, len) && - path[len] == '/' && + (path[len] == '/' || !path[len]) && len > max_len) { max_len = len; @@ -322,17 +322,18 @@ static int find_repo( git_buf path = GIT_BUF_INIT; struct stat st; dev_t initial_device = 0; - bool try_with_dot_git = false; + bool try_with_dot_git = ((flags & GIT_REPOSITORY_OPEN_BARE) != 0); int ceiling_offset; git_buf_free(repo_path); - if ((error = git_path_prettify_dir(&path, start_path, NULL)) < 0) + if ((error = git_path_prettify(&path, start_path, NULL)) < 0) return error; ceiling_offset = find_ceiling_dir_offset(path.ptr, ceiling_dirs); - if ((error = git_buf_joinpath(&path, path.ptr, DOT_GIT)) < 0) + if (!try_with_dot_git && + (error = git_buf_joinpath(&path, path.ptr, DOT_GIT)) < 0) return error; while (!error && !git_buf_len(repo_path)) { @@ -384,7 +385,7 @@ static int find_repo( try_with_dot_git = !try_with_dot_git; } - if (!error && parent_path != NULL) { + if (!error && parent_path && !(flags & GIT_REPOSITORY_OPEN_BARE)) { if (!git_buf_len(repo_path)) git_buf_clear(parent_path); else { @@ -460,7 +461,9 @@ int git_repository_open_ext( repo->path_repository = git_buf_detach(&path); GITERR_CHECK_ALLOC(repo->path_repository); - if ((error = load_config_data(repo)) < 0 || + if ((flags & GIT_REPOSITORY_OPEN_BARE) != 0) + repo->is_bare = 1; + else if ((error = load_config_data(repo)) < 0 || (error = load_workdir(repo, &parent)) < 0) { git_repository_free(repo); diff --git a/tests-clar/repo/open.c b/tests-clar/repo/open.c index 840858586..f386612a7 100644 --- a/tests-clar/repo/open.c +++ b/tests-clar/repo/open.c @@ -69,14 +69,23 @@ void test_repo_open__open_with_discover(void) cl_fixture_cleanup("attr"); } +static void make_gitlink_dir(const char *dir, const char *linktext) +{ + git_buf path = GIT_BUF_INIT; + + cl_git_pass(git_futils_mkdir(dir, NULL, 0777, GIT_MKDIR_VERIFY_DIR)); + cl_git_pass(git_buf_joinpath(&path, dir, ".git")); + cl_git_rewritefile(path.ptr, linktext); + git_buf_free(&path); +} + void test_repo_open__gitlinked(void) { /* need to have both repo dir and workdir set up correctly */ git_repository *repo = cl_git_sandbox_init("empty_standard_repo"); git_repository *repo2; - cl_must_pass(p_mkdir("alternate", 0777)); - cl_git_mkfile("alternate/.git", "gitdir: ../empty_standard_repo/.git"); + make_gitlink_dir("alternate", "gitdir: ../empty_standard_repo/.git"); cl_git_pass(git_repository_open(&repo2, "alternate")); @@ -193,12 +202,11 @@ void test_repo_open__bad_gitlinks(void) cl_git_sandbox_init("attr"); - cl_git_pass(p_mkdir("alternate", 0777)); cl_git_pass(p_mkdir("invalid", 0777)); cl_git_pass(git_futils_mkdir_r("invalid2/.git", NULL, 0777)); for (scan = bad_links; *scan != NULL; scan++) { - cl_git_rewritefile("alternate/.git", *scan); + make_gitlink_dir("alternate", *scan); cl_git_fail(git_repository_open_ext(&repo, "alternate", 0, NULL)); } @@ -315,3 +323,52 @@ void test_repo_open__no_config(void) git_repository_free(repo); cl_fixture_cleanup("empty_standard_repo"); } + +void test_repo_open__force_bare(void) +{ + /* need to have both repo dir and workdir set up correctly */ + git_repository *repo = cl_git_sandbox_init("empty_standard_repo"); + git_repository *barerepo; + + make_gitlink_dir("alternate", "gitdir: ../empty_standard_repo/.git"); + + cl_assert(!git_repository_is_bare(repo)); + + cl_git_pass(git_repository_open(&barerepo, "alternate")); + cl_assert(!git_repository_is_bare(barerepo)); + git_repository_free(barerepo); + + cl_git_pass(git_repository_open_bare( + &barerepo, "empty_standard_repo/.git")); + cl_assert(git_repository_is_bare(barerepo)); + git_repository_free(barerepo); + + cl_git_fail(git_repository_open_bare(&barerepo, "alternate/.git")); + + cl_git_pass(git_repository_open_ext( + &barerepo, "alternate/.git", GIT_REPOSITORY_OPEN_BARE, NULL)); + cl_assert(git_repository_is_bare(barerepo)); + git_repository_free(barerepo); + + cl_git_pass(p_mkdir("empty_standard_repo/subdir", 0777)); + cl_git_mkfile("empty_standard_repo/subdir/something.txt", "something"); + + cl_git_fail(git_repository_open_bare( + &barerepo, "empty_standard_repo/subdir")); + + cl_git_pass(git_repository_open_ext( + &barerepo, "empty_standard_repo/subdir", GIT_REPOSITORY_OPEN_BARE, NULL)); + cl_assert(git_repository_is_bare(barerepo)); + git_repository_free(barerepo); + + cl_git_pass(p_mkdir("alternate/subdir", 0777)); + cl_git_pass(p_mkdir("alternate/subdir/sub2", 0777)); + cl_git_mkfile("alternate/subdir/sub2/something.txt", "something"); + + cl_git_fail(git_repository_open_bare(&barerepo, "alternate/subdir/sub2")); + + cl_git_pass(git_repository_open_ext( + &barerepo, "alternate/subdir/sub2", GIT_REPOSITORY_OPEN_BARE, NULL)); + cl_assert(git_repository_is_bare(barerepo)); + git_repository_free(barerepo); +} From 1aad6137d218830bc280c4327595182019164515 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Sat, 29 Jun 2013 13:16:33 -0700 Subject: [PATCH 10/15] Submodule status improvements This fixes the way that submodule status is checked to bypass just about all of the caching in the submodule object. Based on the ignore value, it will try to do the minimum work necessary to find the current status of the submodule - but it will actually go to disk to get all of the current values. This also removes the custom refcounting stuff in favor of the common git_refcount style. Right now, it is still for internal purposes only, but it should make it easier to add true submodule refcounting in the future with a public git_submodule_free call that will allow bindings not to worry about the submodule object getting freed from underneath them. --- include/git2/submodule.h | 16 +- src/submodule.c | 654 +++++++++++++++++++-------------------- src/submodule.h | 38 ++- 3 files changed, 355 insertions(+), 353 deletions(-) diff --git a/include/git2/submodule.h b/include/git2/submodule.h index 91b5300ae..3267bcd0f 100644 --- a/include/git2/submodule.h +++ b/include/git2/submodule.h @@ -119,19 +119,9 @@ typedef enum { GIT_SUBMODULE_STATUS_WD_UNTRACKED = (1u << 13), } git_submodule_status_t; -#define GIT_SUBMODULE_STATUS__IN_FLAGS \ - (GIT_SUBMODULE_STATUS_IN_HEAD | \ - GIT_SUBMODULE_STATUS_IN_INDEX | \ - GIT_SUBMODULE_STATUS_IN_CONFIG | \ - GIT_SUBMODULE_STATUS_IN_WD) - -#define GIT_SUBMODULE_STATUS__INDEX_FLAGS \ - (GIT_SUBMODULE_STATUS_INDEX_ADDED | \ - GIT_SUBMODULE_STATUS_INDEX_DELETED | \ - GIT_SUBMODULE_STATUS_INDEX_MODIFIED) - -#define GIT_SUBMODULE_STATUS__WD_FLAGS \ - ~(GIT_SUBMODULE_STATUS__IN_FLAGS | GIT_SUBMODULE_STATUS__INDEX_FLAGS) +#define GIT_SUBMODULE_STATUS__IN_FLAGS 0x000Fu +#define GIT_SUBMODULE_STATUS__INDEX_FLAGS 0x0070u +#define GIT_SUBMODULE_STATUS__WD_FLAGS 0x3F80u #define GIT_SUBMODULE_STATUS_IS_UNMODIFIED(S) \ (((S) & ~GIT_SUBMODULE_STATUS__IN_FLAGS) == 0) diff --git a/src/submodule.c b/src/submodule.c index e6ed7e33e..0beedeb42 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -71,15 +71,11 @@ static int load_submodule_config(git_repository *repo); static git_config_backend *open_gitmodules(git_repository *, bool, const git_oid *); static int lookup_head_remote(git_buf *url, git_repository *repo); static int submodule_get(git_submodule **, git_repository *, const char *, const char *); -static void submodule_release(git_submodule *sm, int decr); -static int submodule_load_from_index(git_repository *, const git_index_entry *); -static int submodule_load_from_head(git_repository*, const char*, const git_oid*); static int submodule_load_from_config(const git_config_entry *, void *); static int submodule_load_from_wd_lite(git_submodule *, const char *, void *); static int submodule_update_config(git_submodule *, const char *, const char *, bool, bool); -static void submodule_mode_mismatch(git_repository *, const char *, unsigned int); -static int submodule_index_status(unsigned int *status, git_submodule *sm); -static int submodule_wd_status(unsigned int *status, git_submodule *sm); +static void submodule_get_index_status(unsigned int *, git_submodule *); +static void submodule_get_wd_status(unsigned int *, git_submodule *, git_repository *, git_submodule_ignore_t); static int submodule_cmp(const void *a, const void *b) { @@ -161,7 +157,7 @@ int git_submodule_foreach( * us from issuing a callback twice for a submodule where the name * and path are not the same. */ - if (sm->refcount > 1) { + if (GIT_REFCOUNT_VAL(sm) > 1) { if (git_vector_bsearch(NULL, &seen, sm) != GIT_ENOTFOUND) continue; if ((error = git_vector_insert(&seen, sm)) < 0) @@ -193,9 +189,7 @@ void git_submodule_config_free(git_repository *repo) if (smcfg == NULL) return; - git_strmap_foreach_value(smcfg, sm, { - submodule_release(sm,1); - }); + git_strmap_foreach_value(smcfg, sm, { git_submodule_free(sm); }); git_strmap_free(smcfg); } @@ -336,7 +330,7 @@ int git_submodule_add_finalize(git_submodule *sm) assert(sm); - if ((error = git_repository_index__weakptr(&index, sm->owner)) < 0 || + if ((error = git_repository_index__weakptr(&index, sm->repo)) < 0 || (error = git_index_add_bypath(index, GIT_MODULES_FILE)) < 0) return error; @@ -346,7 +340,7 @@ int git_submodule_add_finalize(git_submodule *sm) int git_submodule_add_to_index(git_submodule *sm, int write_index) { int error; - git_repository *repo, *sm_repo = NULL; + git_repository *sm_repo = NULL; git_index *index; git_buf path = GIT_BUF_INIT; git_commit *head; @@ -355,14 +349,12 @@ int git_submodule_add_to_index(git_submodule *sm, int write_index) assert(sm); - repo = sm->owner; - /* force reload of wd OID by git_submodule_open */ sm->flags = sm->flags & ~GIT_SUBMODULE_STATUS__WD_OID_VALID; - if ((error = git_repository_index__weakptr(&index, repo)) < 0 || + if ((error = git_repository_index__weakptr(&index, sm->repo)) < 0 || (error = git_buf_joinpath( - &path, git_repository_workdir(repo), sm->path)) < 0 || + &path, git_repository_workdir(sm->repo), sm->path)) < 0 || (error = git_submodule_open(&sm_repo, sm)) < 0) goto cleanup; @@ -422,7 +414,7 @@ int git_submodule_save(git_submodule *submodule) assert(submodule); - mods = open_gitmodules(submodule->owner, true, NULL); + mods = open_gitmodules(submodule->repo, true, NULL); if (!mods) { giterr_set(GITERR_SUBMODULE, "Adding submodules to a bare repository is not supported (for now)"); @@ -485,7 +477,7 @@ cleanup: git_repository *git_submodule_owner(git_submodule *submodule) { assert(submodule); - return submodule->owner; + return submodule->repo; } const char *git_submodule_name(git_submodule *submodule) @@ -542,20 +534,12 @@ const git_oid *git_submodule_wd_id(git_submodule *submodule) { assert(submodule); - /* if we know the submodule index timestamp and it has moved, then - * let's reload the working directory information for the submodule - */ - if (submodule->wd_head_path != NULL && - git_futils_filestamp_check( - &submodule->wd_stamp, submodule->wd_head_path)) - submodule->flags &= ~GIT_SUBMODULE_STATUS__WD_OID_VALID; - /* load unless we think we have a valid oid */ if (!(submodule->flags & GIT_SUBMODULE_STATUS__WD_OID_VALID)) { git_repository *subrepo; /* calling submodule open grabs the HEAD OID if possible */ - if (!git_submodule_open(&subrepo, submodule)) + if (!git_submodule_open_bare(&subrepo, submodule)) git_repository_free(subrepo); else giterr_clear(); @@ -674,76 +658,70 @@ int git_submodule_sync(git_submodule *submodule) submodule, "url", submodule->url, true, true); } -int git_submodule_open( - git_repository **subrepo, - git_submodule *submodule) +static int git_submodule__open( + git_repository **subrepo, git_submodule *sm, bool bare) { int error; git_buf path = GIT_BUF_INIT; - git_repository *repo; - const char *workdir; + unsigned int flags = GIT_REPOSITORY_OPEN_NO_SEARCH; + const char *wd; - assert(submodule && subrepo); + assert(sm && subrepo); - repo = submodule->owner; - workdir = git_repository_workdir(repo); + if (git_repository__ensure_not_bare( + sm->repo, "open submodule repository") < 0) + return GIT_EBAREREPO; - if (!workdir) { - giterr_set(GITERR_REPOSITORY, - "Cannot open submodule repository in a bare repo"); - return GIT_ENOTFOUND; - } + wd = git_repository_workdir(sm->repo); - if ((submodule->flags & GIT_SUBMODULE_STATUS_IN_WD) == 0) { - giterr_set(GITERR_REPOSITORY, - "Cannot open submodule repository that is not checked out"); - return GIT_ENOTFOUND; - } - - if (git_buf_joinpath(&path, workdir, submodule->path) < 0) + if (git_buf_joinpath(&path, wd, sm->path) < 0 || + git_buf_joinpath(&path, path.ptr, DOT_GIT) < 0) return -1; - error = git_repository_open(subrepo, path.ptr); + sm->flags = sm->flags & + ~(GIT_SUBMODULE_STATUS_IN_WD | + GIT_SUBMODULE_STATUS__WD_OID_VALID | + GIT_SUBMODULE_STATUS__WD_SCANNED); + + if (bare) + flags |= GIT_REPOSITORY_OPEN_BARE; + + error = git_repository_open_ext(subrepo, path.ptr, flags, wd); + + /* if we opened the submodule successfully, grab HEAD OID, etc. */ + if (!error) { + sm->flags |= GIT_SUBMODULE_STATUS_IN_WD | + GIT_SUBMODULE_STATUS__WD_SCANNED; + + if (!git_reference_name_to_id(&sm->wd_oid, *subrepo, GIT_HEAD_FILE)) + sm->flags |= GIT_SUBMODULE_STATUS__WD_OID_VALID; + else + giterr_clear(); + } else if (git_path_exists(path.ptr)) { + sm->flags |= GIT_SUBMODULE_STATUS__WD_SCANNED | + GIT_SUBMODULE_STATUS_IN_WD; + } else { + git_buf_rtruncate_at_char(&path, '/'); /* remove "/.git" */ + + if (git_path_isdir(path.ptr)) + sm->flags |= GIT_SUBMODULE_STATUS__WD_SCANNED; + } git_buf_free(&path); - /* if we have opened the submodule successfully, let's grab the HEAD OID */ - if (!error) { - git_buf buf = GIT_BUF_INIT; - - /* For now, let's just the index timestamp... - * - * git_buf_joinpath(&buf, git_repository_path(*subrepo), GIT_HEAD_FILE); - * if (!git_path_exists(buf.ptr)) { - */ - git_index *index; - if (!git_repository_index__weakptr(&index, *subrepo)) - git_buf_sets(&buf, git_index_path(index)); - else - git_buf_free(&buf); - /* } */ - - if (git_buf_len(&buf) > 0) { - git__free(submodule->wd_head_path); - submodule->wd_head_path = git_buf_detach(&buf); - } - - if (!git_reference_name_to_id( - &submodule->wd_oid, *subrepo, GIT_HEAD_FILE)) { - - submodule->flags |= GIT_SUBMODULE_STATUS__WD_OID_VALID; - - if (submodule->wd_head_path) - git_futils_filestamp_check( - &submodule->wd_stamp, submodule->wd_head_path); - } - - giterr_clear(); - } - return error; } +int git_submodule_open_bare(git_repository **subrepo, git_submodule *sm) +{ + return git_submodule__open(subrepo, sm, true); +} + +int git_submodule_open(git_repository **subrepo, git_submodule *sm) +{ + return git_submodule__open(subrepo, sm, false); +} + int git_submodule_reload_all(git_repository *repo) { assert(repo); @@ -751,74 +729,99 @@ int git_submodule_reload_all(git_repository *repo) return load_submodule_config(repo); } +static void submodule_update_from_index_entry( + git_submodule *sm, const git_index_entry *ie) +{ + bool already_found = (sm->flags & GIT_SUBMODULE_STATUS_IN_INDEX) != 0; + + if (!S_ISGITLINK(ie->mode)) { + if (!already_found) + sm->flags |= GIT_SUBMODULE_STATUS__INDEX_NOT_SUBMODULE; + } else { + if (already_found) + sm->flags |= GIT_SUBMODULE_STATUS__INDEX_MULTIPLE_ENTRIES; + else + git_oid_cpy(&sm->index_oid, &ie->oid); + + sm->flags |= GIT_SUBMODULE_STATUS_IN_INDEX | + GIT_SUBMODULE_STATUS__INDEX_OID_VALID; + } +} + +static int submodule_update_index(git_submodule *sm) +{ + git_index *index; + const git_index_entry *ie; + + if (git_repository_index__weakptr(&index, sm->repo) < 0) + return -1; + + sm->flags = sm->flags & + ~(GIT_SUBMODULE_STATUS_IN_INDEX | + GIT_SUBMODULE_STATUS__INDEX_OID_VALID); + + if (!(ie = git_index_get_bypath(index, sm->path, 0))) + return 0; + + submodule_update_from_index_entry(sm, ie); + + return 0; +} + +static void submodule_update_from_head_data( + git_submodule *sm, mode_t mode, const git_oid *id) +{ + if (!S_ISGITLINK(mode)) + sm->flags |= GIT_SUBMODULE_STATUS__HEAD_NOT_SUBMODULE; + else { + git_oid_cpy(&sm->head_oid, id); + + sm->flags |= GIT_SUBMODULE_STATUS_IN_HEAD | + GIT_SUBMODULE_STATUS__HEAD_OID_VALID; + } +} + +static int submodule_update_head(git_submodule *submodule) +{ + git_tree *head = NULL; + git_tree_entry *te; + + submodule->flags = submodule->flags & + ~(GIT_SUBMODULE_STATUS_IN_HEAD | + GIT_SUBMODULE_STATUS__HEAD_OID_VALID); + + /* if we can't look up file in current head, then done */ + if (git_repository_head_tree(&head, submodule->repo) < 0 || + git_tree_entry_bypath(&te, head, submodule->path) < 0) + giterr_clear(); + else + submodule_update_from_head_data(submodule, te->attr, &te->oid); + + git_tree_free(head); + return 0; +} + int git_submodule_reload(git_submodule *submodule) { - git_repository *repo; - git_index *index; - int error; - size_t pos; - git_tree *head; + int error = 0; git_config_backend *mods; assert(submodule); /* refresh index data */ - repo = submodule->owner; - if (git_repository_index__weakptr(&index, repo) < 0) + if (submodule_update_index(submodule) < 0) return -1; - submodule->flags = submodule->flags & - ~(GIT_SUBMODULE_STATUS_IN_INDEX | - GIT_SUBMODULE_STATUS__INDEX_OID_VALID); - - if (!git_index_find(&pos, index, submodule->path)) { - const git_index_entry *entry = git_index_get_byindex(index, pos); - - if (S_ISGITLINK(entry->mode)) { - if ((error = submodule_load_from_index(repo, entry)) < 0) - return error; - } else { - submodule_mode_mismatch( - repo, entry->path, GIT_SUBMODULE_STATUS__INDEX_NOT_SUBMODULE); - } - } - /* refresh HEAD tree data */ - if (!(error = git_repository_head_tree(&head, repo))) { - git_tree_entry *te; - - submodule->flags = submodule->flags & - ~(GIT_SUBMODULE_STATUS_IN_HEAD | - GIT_SUBMODULE_STATUS__HEAD_OID_VALID); - - if (!(error = git_tree_entry_bypath(&te, head, submodule->path))) { - - if (S_ISGITLINK(te->attr)) { - error = submodule_load_from_head(repo, submodule->path, &te->oid); - } else { - submodule_mode_mismatch( - repo, submodule->path, - GIT_SUBMODULE_STATUS__HEAD_NOT_SUBMODULE); - } - - git_tree_entry_free(te); - } - else if (error == GIT_ENOTFOUND) { - giterr_clear(); - error = 0; - } - - git_tree_free(head); - } - - if (error < 0) - return error; + if (submodule_update_head(submodule) < 0) + return -1; /* refresh config data */ - if ((mods = open_gitmodules(repo, false, NULL)) != NULL) { + mods = open_gitmodules(submodule->repo, false, NULL); + if (mods != NULL) { git_buf path = GIT_BUF_INIT; git_buf_sets(&path, "submodule\\."); @@ -829,7 +832,7 @@ int git_submodule_reload(git_submodule *submodule) error = -1; else error = git_config_file_foreach_match( - mods, path.ptr, submodule_load_from_config, repo); + mods, path.ptr, submodule_load_from_config, submodule->repo); git_buf_free(&path); git_config_file_free(mods); @@ -848,40 +851,93 @@ int git_submodule_reload(git_submodule *submodule) return error; } -int git_submodule_status( - unsigned int *status, - git_submodule *submodule) +static void submodule_copy_oid_maybe( + git_oid *tgt, const git_oid *src, bool valid) { - int error = 0; - unsigned int status_val; - - assert(status && submodule); - - status_val = GIT_SUBMODULE_STATUS__CLEAR_INTERNAL(submodule->flags); - - if (submodule->ignore != GIT_SUBMODULE_IGNORE_ALL) { - if (!(error = submodule_index_status(&status_val, submodule))) - error = submodule_wd_status(&status_val, submodule); + if (tgt) { + if (valid) + memcpy(tgt, src, sizeof(*tgt)); + else + memset(tgt, 0, sizeof(*tgt)); } - - *status = status_val; - - return error; } -int git_submodule_location( - unsigned int *location_status, - git_submodule *submodule) +int git_submodule__status( + unsigned int *out_status, + git_oid *out_head_id, + git_oid *out_index_id, + git_oid *out_wd_id, + git_submodule *sm, + git_submodule_ignore_t ign) { - assert(location_status && submodule); + unsigned int status; + git_repository *smrepo = NULL; - *location_status = submodule->flags & - (GIT_SUBMODULE_STATUS_IN_HEAD | GIT_SUBMODULE_STATUS_IN_INDEX | - GIT_SUBMODULE_STATUS_IN_CONFIG | GIT_SUBMODULE_STATUS_IN_WD); + if (ign == GIT_SUBMODULE_IGNORE_DEFAULT) + ign = sm->ignore; + + /* only return location info if ignore == all */ + if (ign == GIT_SUBMODULE_IGNORE_ALL) { + *out_status = (sm->flags & GIT_SUBMODULE_STATUS__IN_FLAGS); + return 0; + } + + /* refresh the index OID */ + if (submodule_update_index(sm) < 0) + return -1; + + /* refresh the HEAD OID */ + if (submodule_update_head(sm) < 0) + return -1; + + /* for ignore == dirty, don't scan the working directory */ + if (ign == GIT_SUBMODULE_IGNORE_DIRTY) { + /* git_submodule_open_bare will load WD OID data */ + if (git_submodule_open_bare(&smrepo, sm) < 0) + giterr_clear(); + else + git_repository_free(smrepo); + smrepo = NULL; + } else if (git_submodule_open(&smrepo, sm) < 0) { + giterr_clear(); + smrepo = NULL; + } + + status = GIT_SUBMODULE_STATUS__CLEAR_INTERNAL(sm->flags); + + submodule_get_index_status(&status, sm); + submodule_get_wd_status(&status, sm, smrepo, ign); + + git_repository_free(smrepo); + + *out_status = status; + + submodule_copy_oid_maybe(out_head_id, &sm->head_oid, + (sm->flags & GIT_SUBMODULE_STATUS__HEAD_OID_VALID) != 0); + submodule_copy_oid_maybe(out_index_id, &sm->index_oid, + (sm->flags & GIT_SUBMODULE_STATUS__INDEX_OID_VALID) != 0); + submodule_copy_oid_maybe(out_wd_id, &sm->wd_oid, + (sm->flags & GIT_SUBMODULE_STATUS__WD_OID_VALID) != 0); return 0; } +int git_submodule_status(unsigned int *status, git_submodule *sm) +{ + assert(status && sm); + + return git_submodule__status( + status, NULL, NULL, NULL, sm, GIT_SUBMODULE_IGNORE_DEFAULT); +} + +int git_submodule_location(unsigned int *location, git_submodule *sm) +{ + assert(location && sm); + + return git_submodule__status( + location, NULL, NULL, NULL, sm, GIT_SUBMODULE_IGNORE_ALL); +} + /* * INTERNAL FUNCTIONS @@ -889,54 +945,47 @@ int git_submodule_location( static git_submodule *submodule_alloc(git_repository *repo, const char *name) { + size_t namelen; git_submodule *sm; - if (!name || !strlen(name)) { + if (!name || !(namelen = strlen(name))) { giterr_set(GITERR_SUBMODULE, "Invalid submodule name"); return NULL; } - sm = git__calloc(1, sizeof(git_submodule)); + sm = git__calloc(1, sizeof(git_submodule) + namelen + 1); if (sm == NULL) - goto fail; + return NULL; - sm->path = sm->name = git__strdup(name); - if (!sm->name) - goto fail; - - sm->owner = repo; - sm->refcount = 1; + sm->name = sm->path = git__strdup(name); + if (!sm->name) { + git__free(sm); + return NULL; + } + GIT_REFCOUNT_INC(sm); + sm->repo = repo; return sm; - -fail: - submodule_release(sm, 0); - return NULL; } -static void submodule_release(git_submodule *sm, int decr) +static void submodule_release(git_submodule *sm) { if (!sm) return; - sm->refcount -= decr; + if (sm->path != sm->name) + git__free(sm->path); + git__free(sm->name); + git__free(sm->url); + git__memzero(sm, sizeof(*sm)); + git__free(sm); +} - if (sm->refcount == 0) { - if (sm->name != sm->path) { - git__free(sm->path); - sm->path = NULL; - } - - git__free(sm->name); - sm->name = NULL; - - git__free(sm->url); - sm->url = NULL; - - sm->owner = NULL; - - git__free(sm); - } +void git_submodule_free(git_submodule *sm) +{ + if (!sm) + return; + GIT_REFCOUNT_DEC(sm, submodule_release); } static int submodule_get( @@ -966,10 +1015,10 @@ static int submodule_get( pos = kh_put(str, smcfg, sm->name, &error); if (error < 0) { - submodule_release(sm, 1); + git_submodule_free(sm); sm = NULL; } else if (error == 0) { - submodule_release(sm, 1); + git_submodule_free(sm); sm = git_strmap_value_at(smcfg, pos); } else { git_strmap_set_value_at(smcfg, pos, sm); @@ -983,43 +1032,6 @@ static int submodule_get( return (sm != NULL) ? 0 : -1; } -static int submodule_load_from_index( - git_repository *repo, const git_index_entry *entry) -{ - git_submodule *sm; - - if (submodule_get(&sm, repo, entry->path, NULL) < 0) - return -1; - - if (sm->flags & GIT_SUBMODULE_STATUS_IN_INDEX) { - sm->flags |= GIT_SUBMODULE_STATUS__INDEX_MULTIPLE_ENTRIES; - return 0; - } - - sm->flags |= GIT_SUBMODULE_STATUS_IN_INDEX; - - git_oid_cpy(&sm->index_oid, &entry->oid); - sm->flags |= GIT_SUBMODULE_STATUS__INDEX_OID_VALID; - - return 0; -} - -static int submodule_load_from_head( - git_repository *repo, const char *path, const git_oid *oid) -{ - git_submodule *sm; - - if (submodule_get(&sm, repo, path, NULL) < 0) - return -1; - - sm->flags |= GIT_SUBMODULE_STATUS_IN_HEAD; - - git_oid_cpy(&sm->head_oid, oid); - sm->flags |= GIT_SUBMODULE_STATUS__HEAD_OID_VALID; - - return 0; -} - static int submodule_config_error(const char *property, const char *value) { giterr_set(GITERR_INVALID, @@ -1079,11 +1091,11 @@ static int submodule_load_from_config( git_strmap_insert2(smcfg, alternate, sm, old_sm, error); if (error >= 0) - sm->refcount++; /* inserted under a new key */ + GIT_REFCOUNT_INC(sm); /* inserted under a new key */ /* if we replaced an old module under this key, release the old one */ if (old_sm && ((git_submodule *)old_sm) != sm) { - submodule_release(old_sm, 1); + git_submodule_free(old_sm); /* TODO: log warning about multiple submodules with same path */ } } @@ -1133,13 +1145,15 @@ static int submodule_load_from_config( static int submodule_load_from_wd_lite( git_submodule *sm, const char *name, void *payload) { - git_repository *repo = git_submodule_owner(sm); git_buf path = GIT_BUF_INIT; GIT_UNUSED(name); GIT_UNUSED(payload); - if (git_buf_joinpath(&path, git_repository_workdir(repo), sm->path) < 0) + if (git_repository_is_bare(sm->repo)) + return 0; + + if (git_buf_joinpath(&path, git_repository_workdir(sm->repo), sm->path) < 0) return -1; if (git_path_isdir(path.ptr)) @@ -1153,18 +1167,6 @@ static int submodule_load_from_wd_lite( return 0; } -static void submodule_mode_mismatch( - git_repository *repo, const char *path, unsigned int flag) -{ - khiter_t pos = git_strmap_lookup_index(repo->submodules, path); - - if (git_strmap_valid_index(repo->submodules, pos)) { - git_submodule *sm = git_strmap_value_at(repo->submodules, pos); - - sm->flags |= flag; - } -} - static int load_submodule_config_from_index( git_repository *repo, git_oid *gitmodules_oid) { @@ -1178,18 +1180,21 @@ static int load_submodule_config_from_index( return error; while (!(error = git_iterator_advance(&entry, i))) { + khiter_t pos = git_strmap_lookup_index(repo->submodules, entry->path); + git_submodule *sm; - if (S_ISGITLINK(entry->mode)) { - error = submodule_load_from_index(repo, entry); - if (error < 0) - break; - } else { - submodule_mode_mismatch( - repo, entry->path, GIT_SUBMODULE_STATUS__INDEX_NOT_SUBMODULE); + if (git_strmap_valid_index(repo->submodules, pos)) { + sm = git_strmap_value_at(repo->submodules, pos); - if (strcmp(entry->path, GIT_MODULES_FILE) == 0) - git_oid_cpy(gitmodules_oid, &entry->oid); - } + if (S_ISGITLINK(entry->mode)) + submodule_update_from_index_entry(sm, entry); + else + sm->flags |= GIT_SUBMODULE_STATUS__INDEX_NOT_SUBMODULE; + } else if (S_ISGITLINK(entry->mode)) { + if (!submodule_get(&sm, repo, entry->path, NULL)) + submodule_update_from_index_entry(sm, entry); + } else if (strcmp(entry->path, GIT_MODULES_FILE) == 0) + git_oid_cpy(gitmodules_oid, &entry->oid); } if (error == GIT_ITEROVER) @@ -1220,18 +1225,24 @@ static int load_submodule_config_from_head( } while (!(error = git_iterator_advance(&entry, i))) { + khiter_t pos = git_strmap_lookup_index(repo->submodules, entry->path); + git_submodule *sm; - if (S_ISGITLINK(entry->mode)) { - error = submodule_load_from_head(repo, entry->path, &entry->oid); - if (error < 0) - break; - } else { - submodule_mode_mismatch( - repo, entry->path, GIT_SUBMODULE_STATUS__HEAD_NOT_SUBMODULE); + if (git_strmap_valid_index(repo->submodules, pos)) { + sm = git_strmap_value_at(repo->submodules, pos); - if (strcmp(entry->path, GIT_MODULES_FILE) == 0 && - git_oid_iszero(gitmodules_oid)) - git_oid_cpy(gitmodules_oid, &entry->oid); + if (S_ISGITLINK(entry->mode)) + submodule_update_from_head_data( + sm, entry->mode, &entry->oid); + else + sm->flags |= GIT_SUBMODULE_STATUS__HEAD_NOT_SUBMODULE; + } else if (S_ISGITLINK(entry->mode)) { + if (!submodule_get(&sm, repo, entry->path, NULL)) + submodule_update_from_head_data( + sm, entry->mode, &entry->oid); + } else if (strcmp(entry->path, GIT_MODULES_FILE) == 0 && + git_oid_iszero(gitmodules_oid)) { + git_oid_cpy(gitmodules_oid, &entry->oid); } } @@ -1416,7 +1427,7 @@ static int submodule_update_config( assert(submodule); - error = git_repository_config__weakptr(&config, submodule->owner); + error = git_repository_config__weakptr(&config, submodule->repo); if (error < 0) return error; @@ -1444,11 +1455,13 @@ cleanup: return error; } -static int submodule_index_status(unsigned int *status, git_submodule *sm) +static void submodule_get_index_status(unsigned int *status, git_submodule *sm) { const git_oid *head_oid = git_submodule_head_id(sm); const git_oid *index_oid = git_submodule_index_id(sm); + *status = *status & ~GIT_SUBMODULE_STATUS__INDEX_FLAGS; + if (!head_oid) { if (index_oid) *status |= GIT_SUBMODULE_STATUS_INDEX_ADDED; @@ -1457,27 +1470,22 @@ static int submodule_index_status(unsigned int *status, git_submodule *sm) *status |= GIT_SUBMODULE_STATUS_INDEX_DELETED; else if (!git_oid_equal(head_oid, index_oid)) *status |= GIT_SUBMODULE_STATUS_INDEX_MODIFIED; - - return 0; } -static int submodule_wd_status(unsigned int *status, git_submodule *sm) +static void submodule_get_wd_status( + unsigned int *status, + git_submodule *sm, + git_repository *sm_repo, + git_submodule_ignore_t ign) { - int error = 0; - const git_oid *wd_oid, *index_oid; - git_repository *sm_repo = NULL; + const git_oid *index_oid = git_submodule_index_id(sm); + const git_oid *wd_oid = + (sm->flags & GIT_SUBMODULE_STATUS__WD_OID_VALID) ? &sm->wd_oid : NULL; + git_tree *sm_head = NULL; + git_diff_options opt = GIT_DIFF_OPTIONS_INIT; + git_diff_list *diff; - /* open repo now if we need it (so wd_id() call won't reopen) */ - if ((sm->ignore == GIT_SUBMODULE_IGNORE_NONE || - sm->ignore == GIT_SUBMODULE_IGNORE_UNTRACKED) && - (sm->flags & GIT_SUBMODULE_STATUS_IN_WD) != 0) - { - if ((error = git_submodule_open(&sm_repo, sm)) < 0) - return error; - } - - index_oid = git_submodule_index_id(sm); - wd_oid = git_submodule_wd_id(sm); + *status = *status & ~GIT_SUBMODULE_STATUS__WD_FLAGS; if (!index_oid) { if (wd_oid) @@ -1493,59 +1501,49 @@ static int submodule_wd_status(unsigned int *status, git_submodule *sm) else if (!git_oid_equal(index_oid, wd_oid)) *status |= GIT_SUBMODULE_STATUS_WD_MODIFIED; - if (sm_repo != NULL) { - git_tree *sm_head; - git_diff_options opt = GIT_DIFF_OPTIONS_INIT; - git_diff_list *diff; + /* if we have no repo, then we're done */ + if (!sm_repo) + return; - /* the diffs below could be optimized with an early termination - * option to the git_diff functions, but for now this is sufficient - * (and certainly no worse that what core git does). - */ + /* the diffs below could be optimized with an early termination + * option to the git_diff functions, but for now this is sufficient + * (and certainly no worse that what core git does). + */ - /* perform head-to-index diff on submodule */ + if (ign == GIT_SUBMODULE_IGNORE_NONE) + opt.flags |= GIT_DIFF_INCLUDE_UNTRACKED; - if ((error = git_repository_head_tree(&sm_head, sm_repo)) < 0) - return error; - - if (sm->ignore == GIT_SUBMODULE_IGNORE_NONE) - opt.flags |= GIT_DIFF_INCLUDE_UNTRACKED; - - error = git_diff_tree_to_index(&diff, sm_repo, sm_head, NULL, &opt); - - if (!error) { + /* if we don't have an orphaned head, check diff with index */ + if (git_repository_head_tree(&sm_head, sm_repo) < 0) + giterr_clear(); + else { + /* perform head to index diff on submodule */ + if (git_diff_tree_to_index(&diff, sm_repo, sm_head, NULL, &opt) < 0) + giterr_clear(); + else { if (git_diff_num_deltas(diff) > 0) *status |= GIT_SUBMODULE_STATUS_WD_INDEX_MODIFIED; - git_diff_list_free(diff); diff = NULL; } git_tree_free(sm_head); - - if (error < 0) - return error; - - /* perform index-to-workdir diff on submodule */ - - error = git_diff_index_to_workdir(&diff, sm_repo, NULL, &opt); - - if (!error) { - size_t untracked = - git_diff_num_deltas_of_type(diff, GIT_DELTA_UNTRACKED); - - if (untracked > 0) - *status |= GIT_SUBMODULE_STATUS_WD_UNTRACKED; - - if (git_diff_num_deltas(diff) != untracked) - *status |= GIT_SUBMODULE_STATUS_WD_WD_MODIFIED; - - git_diff_list_free(diff); - diff = NULL; - } - - git_repository_free(sm_repo); } - return error; + /* perform index-to-workdir diff on submodule */ + if (git_diff_index_to_workdir(&diff, sm_repo, NULL, &opt) < 0) + giterr_clear(); + else { + size_t untracked = + git_diff_num_deltas_of_type(diff, GIT_DELTA_UNTRACKED); + + if (untracked > 0) + *status |= GIT_SUBMODULE_STATUS_WD_UNTRACKED; + + if (git_diff_num_deltas(diff) != untracked) + *status |= GIT_SUBMODULE_STATUS_WD_WD_MODIFIED; + + git_diff_list_free(diff); + diff = NULL; + } } diff --git a/src/submodule.h b/src/submodule.h index 88d4f97c7..8db2fff29 100644 --- a/src/submodule.h +++ b/src/submodule.h @@ -48,6 +48,10 @@ * an entry for every submodule found in the HEAD and index, and for every * submodule described in .gitmodules. The fields are as follows: * + * - `rc` tracks the refcount of how many hash table entries in the + * git_submodule_cache there are for this submodule. It only comes into + * play if the name and path of the submodule differ. + * * - `name` is the name of the submodule from .gitmodules. * - `path` is the path to the submodule from the repo root. It is almost * always the same as `name`. @@ -58,18 +62,12 @@ * - `ignore_default` is the ignore value from the config * - `fetch_recurse` is 0 or 1 - see gitmodules(5) fetchRecurseSubmodules. * - * - `owner` is the git_repository containing this submodule + * - `repo` is the parent repository that contains this submodule. * - `flags` after for internal use, tracking where this submodule has been * found (head, index, config, workdir) and known status info, etc. * - `head_oid` is the SHA1 for the submodule path in the repo HEAD. * - `index_oid` is the SHA1 for the submodule recorded in the index. * - `wd_oid` is the SHA1 for the HEAD of the checked out submodule. - * - `wd_index_path` is the path to the index of the checked out submodule - * - `wd_last_index` is a timestamp of that submodule index so we can - * quickly check if the `wd_oid` should be rechecked - * - `refcount` tracks how many hash table entries in the - * git_submodule_cache there are for this submodule. It only comes into - * play if the name and path of the submodule differ. * * If the submodule has been added to .gitmodules but not yet git added, * then the `index_oid` will be zero but still marked valid. If the @@ -77,9 +75,11 @@ * then the `index_oid` will be set, but the `url` will be NULL. */ struct git_submodule { + git_refcount rc; + /* information from config */ char *name; - char *path; /* important: may point to same string data as "name" */ + char *path; /* important: may just point to "name" string */ char *url; git_submodule_update_t update; git_submodule_update_t update_default; @@ -88,14 +88,11 @@ struct git_submodule { int fetch_recurse; /* internal information */ - git_repository *owner; + git_repository *repo; uint32_t flags; git_oid head_oid; git_oid index_oid; git_oid wd_oid; - char *wd_head_path; - git_futils_filestamp wd_stamp; - int refcount; }; /* Additional flags on top of public GIT_SUBMODULE_STATUS values */ @@ -113,4 +110,21 @@ enum { #define GIT_SUBMODULE_STATUS__CLEAR_INTERNAL(S) \ ((S) & ~(0xFFFFFFFFu << 20)) +/* Internal status fn returns status and optionally the various OIDs */ +extern int git_submodule__status( + unsigned int *out_status, + git_oid *out_head_id, + git_oid *out_index_id, + git_oid *out_wd_id, + git_submodule *sm, + git_submodule_ignore_t ign); + +/* Open submodule repository as bare repo for quick HEAD check, etc. */ +extern int git_submodule_open_bare( + git_repository **repo, + git_submodule *submodule); + +/* Release reference to submodule object - not currently for external use */ +extern void git_submodule_free(git_submodule *sm); + #endif From 2e3e273e33894bc1089cfc09d89bd2cb144b108d Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Sat, 29 Jun 2013 13:20:45 -0700 Subject: [PATCH 11/15] Update diff to new internal submodule status API Submodules now expose an internal status API that allows diff to get back the OID values from the submodule very easily and also to avoiding caching issues and to override the ignore setting for the submodule. --- src/diff.c | 14 ++++++++------ tests-clar/diff/submodules.c | 7 ------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/diff.c b/src/diff.c index cc7be451f..2b018188e 100644 --- a/src/diff.c +++ b/src/diff.c @@ -13,6 +13,7 @@ #include "pathspec.h" #include "index.h" #include "odb.h" +#include "submodule.h" #define DIFF_FLAG_IS_SET(DIFF,FLAG) (((DIFF)->opts.flags & (FLAG)) != 0) #define DIFF_FLAG_ISNT_SET(DIFF,FLAG) (((DIFF)->opts.flags & (FLAG)) == 0) @@ -595,7 +596,6 @@ static int maybe_modified_submodule( int error = 0; git_submodule *sub; unsigned int sm_status = 0; - const git_oid *sm_oid; *status = GIT_DELTA_UNMODIFIED; @@ -603,7 +603,9 @@ static int maybe_modified_submodule( !(error = git_submodule_lookup( &sub, diff->repo, info->nitem->path)) && git_submodule_ignore(sub) != GIT_SUBMODULE_IGNORE_ALL && - !(error = git_submodule_status(&sm_status, sub))) + !(error = git_submodule__status( + &sm_status, NULL, NULL, found_oid, sub, + GIT_SUBMODULE_IGNORE_DEFAULT))) { /* check IS_WD_UNMODIFIED because this case is only used * when the new side of the diff is the working directory @@ -611,10 +613,10 @@ static int maybe_modified_submodule( if (!GIT_SUBMODULE_STATUS_IS_WD_UNMODIFIED(sm_status)) *status = GIT_DELTA_MODIFIED; - /* grab OID while we are here */ - if (git_oid_iszero(&info->nitem->oid) && - (sm_oid = git_submodule_wd_id(sub)) != NULL) - git_oid_cpy(found_oid, sm_oid); + /* now that we have a HEAD OID, check if HEAD moved */ + if ((sm_status & GIT_SUBMODULE_STATUS_IN_WD) != 0 && + !git_oid_equal(&info->oitem->oid, found_oid)) + *status = GIT_DELTA_MODIFIED; } /* GIT_EEXISTS means a dir with .git in it was found - ignore it */ diff --git a/tests-clar/diff/submodules.c b/tests-clar/diff/submodules.c index c94fd57c6..4a40affb2 100644 --- a/tests-clar/diff/submodules.c +++ b/tests-clar/diff/submodules.c @@ -333,8 +333,6 @@ void test_diff_submodules__invalid_cache(void) check_diff_patches(diff, expected_unchanged); git_diff_list_free(diff); - sleep(2); - /* commit changed index of submodule */ { git_object *parent; @@ -357,11 +355,6 @@ void test_diff_submodules__invalid_cache(void) git_signature_free(sig); } - /* THIS RELOAD SHOULD NOT BE REQUIRED - cl_git_pass(git_submodule_reload_all(g_repo)); - cl_git_pass(git_submodule_lookup(&sm, g_repo, smpath)); - */ - git_submodule_set_ignore(sm, GIT_SUBMODULE_IGNORE_DIRTY); cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); From f9775a37aa4ed042839a6b2f9d8e0dfbd73a2f09 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Sat, 29 Jun 2013 23:22:31 -0700 Subject: [PATCH 12/15] Add ignore_submodules to diff options This adds correct support for an equivalent to --ignore-submodules in diff, where an actual ignore value can be passed to diff to override the per submodule settings in the configuration. This required tweaking the constants for ignore values so that zero would not be used and could represent an unset option to the diff. This was an opportunity to move the submodule values into include/git2/types.h and to rename the poorly named DEFAULT values for ignore and update constants to RESET instead. Now the GIT_DIFF_IGNORE_SUBMODULES flag is exactly the same as setting the ignore_submodules option to GIT_SUBMODULE_IGNORE_ALL (which is actually a minor change from the old behavior in that submodules will now be treated as UNMODIFIED deltas instead of being left out totally - if you set GIT_DIFF_INCLUDE_UNMODIFIED). This includes tests for the various new settings. --- include/git2/diff.h | 5 +- include/git2/submodule.h | 53 +++------------- include/git2/types.h | 34 +++++++++++ src/diff.c | 77 ++++++++++++----------- src/submodule.c | 112 +++++++++++++++++++++++----------- src/submodule.h | 8 +++ tests-clar/clar_libgit2.h | 3 + tests-clar/diff/submodules.c | 65 +++++++++++++++++++- tests-clar/submodule/modify.c | 10 +-- 9 files changed, 245 insertions(+), 122 deletions(-) diff --git a/include/git2/diff.h b/include/git2/diff.h index 121c9df5c..71a8b72bf 100644 --- a/include/git2/diff.h +++ b/include/git2/diff.h @@ -78,7 +78,7 @@ typedef enum { GIT_DIFF_IGNORE_WHITESPACE_CHANGE = (1 << 3), /** Ignore whitespace at end of line */ GIT_DIFF_IGNORE_WHITESPACE_EOL = (1 << 4), - /** Exclude submodules from the diff completely */ + /** Treat all submodules as unmodified */ GIT_DIFF_IGNORE_SUBMODULES = (1 << 5), /** Use the "patience diff" algorithm (currently unimplemented) */ GIT_DIFF_PATIENCE = (1 << 6), @@ -314,6 +314,8 @@ typedef int (*git_diff_notify_cb)( * - `notify_cb` is an optional callback function, notifying the consumer of * which files are being examined as the diff is generated * - `notify_payload` is the payload data to pass to the `notify_cb` function + * - `ignore_submodules` overrides the submodule ignore setting for all + * submodules in the diff. */ typedef struct { unsigned int version; /**< version for the struct */ @@ -326,6 +328,7 @@ typedef struct { git_off_t max_size; /**< defaults to 512MB */ git_diff_notify_cb notify_cb; void *notify_payload; + git_submodule_ignore_t ignore_submodules; /** << submodule ignore rule */ } git_diff_options; #define GIT_DIFF_OPTIONS_VERSION 1 diff --git a/include/git2/submodule.h b/include/git2/submodule.h index 3267bcd0f..ba7a558d0 100644 --- a/include/git2/submodule.h +++ b/include/git2/submodule.h @@ -14,51 +14,18 @@ /** * @file git2/submodule.h * @brief Git submodule management utilities - * @defgroup git_submodule Git submodule management routines - * @ingroup Git - * @{ - */ -GIT_BEGIN_DECL - -/** - * Opaque structure representing a submodule. * * Submodule support in libgit2 builds a list of known submodules and keeps * it in the repository. The list is built from the .gitmodules file, the * .git/config file, the index, and the HEAD tree. Items in the working * directory that look like submodules (i.e. a git repo) but are not * mentioned in those places won't be tracked. - */ -typedef struct git_submodule git_submodule; - -/** - * Values that could be specified for the update rule of a submodule. * - * Use the DEFAULT value if you have altered the update value via - * `git_submodule_set_update()` and wish to reset to the original default. + * @defgroup git_submodule Git submodule management routines + * @ingroup Git + * @{ */ -typedef enum { - GIT_SUBMODULE_UPDATE_DEFAULT = -1, - GIT_SUBMODULE_UPDATE_CHECKOUT = 0, - GIT_SUBMODULE_UPDATE_REBASE = 1, - GIT_SUBMODULE_UPDATE_MERGE = 2, - GIT_SUBMODULE_UPDATE_NONE = 3 -} git_submodule_update_t; - -/** - * Values that could be specified for how closely to examine the - * working directory when getting submodule status. - * - * Use the DEFUALT value if you have altered the ignore value via - * `git_submodule_set_ignore()` and wish to reset to the original value. - */ -typedef enum { - GIT_SUBMODULE_IGNORE_DEFAULT = -1, /* reset to default */ - GIT_SUBMODULE_IGNORE_NONE = 0, /* any change or untracked == dirty */ - GIT_SUBMODULE_IGNORE_UNTRACKED = 1, /* dirty if tracked files change */ - GIT_SUBMODULE_IGNORE_DIRTY = 2, /* only dirty if HEAD moved */ - GIT_SUBMODULE_IGNORE_ALL = 3 /* never dirty */ -} git_submodule_ignore_t; +GIT_BEGIN_DECL /** * Return codes for submodule status. @@ -377,9 +344,9 @@ GIT_EXTERN(git_submodule_ignore_t) git_submodule_ignore( * submodule is in memory. You should call `git_submodule_save()` if you * want to persist the new ignore role. * - * Calling this again with GIT_SUBMODULE_IGNORE_DEFAULT or calling - * `git_submodule_reload()` will revert the rule to the value that was in the - * original config. + * Calling this again with GIT_SUBMODULE_IGNORE_RESET or calling + * `git_submodule_reload()` will revert the rule to the value that was in + * the original config. * * @return old value for ignore */ @@ -399,9 +366,9 @@ GIT_EXTERN(git_submodule_update_t) git_submodule_update( * This sets the update rule in memory for the submodule. You should call * `git_submodule_save()` if you want to persist the new update rule. * - * Calling this again with GIT_SUBMODULE_UPDATE_DEFAULT or calling - * `git_submodule_reload()` will revert the rule to the value that was in the - * original config. + * Calling this again with GIT_SUBMODULE_UPDATE_RESET or calling + * `git_submodule_reload()` will revert the rule to the value that was in + * the original config. * * @return old value for update */ diff --git a/include/git2/types.h b/include/git2/types.h index dc344075c..1da0d3ed2 100644 --- a/include/git2/types.h +++ b/include/git2/types.h @@ -229,6 +229,40 @@ typedef struct git_transfer_progress { */ typedef int (*git_transfer_progress_callback)(const git_transfer_progress *stats, void *payload); +/** + * Opaque structure representing a submodule. + */ +typedef struct git_submodule git_submodule; + +/** + * Values that could be specified for the update rule of a submodule. + * + * Use the RESET value if you have altered the in-memory update value via + * `git_submodule_set_update()` and wish to reset to the original default. + */ +typedef enum { + GIT_SUBMODULE_UPDATE_RESET = -1, + GIT_SUBMODULE_UPDATE_CHECKOUT = 1, + GIT_SUBMODULE_UPDATE_REBASE = 2, + GIT_SUBMODULE_UPDATE_MERGE = 3, + GIT_SUBMODULE_UPDATE_NONE = 4 +} git_submodule_update_t; + +/** + * Values that could be specified for how closely to examine the + * working directory when getting submodule status. + * + * Use the RESET value if you have altered the in-memory ignore value via + * `git_submodule_set_ignore()` and wish to reset to the original value. + */ +typedef enum { + GIT_SUBMODULE_IGNORE_RESET = -1, /* reset to on-disk value */ + GIT_SUBMODULE_IGNORE_NONE = 1, /* any change or untracked == dirty */ + GIT_SUBMODULE_IGNORE_UNTRACKED = 2, /* dirty if tracked files change */ + GIT_SUBMODULE_IGNORE_DIRTY = 3, /* only dirty if HEAD moved */ + GIT_SUBMODULE_IGNORE_ALL = 4 /* never dirty */ +} git_submodule_ignore_t; + /** @} */ GIT_END_DECL diff --git a/src/diff.c b/src/diff.c index 2b018188e..5fa635f05 100644 --- a/src/diff.c +++ b/src/diff.c @@ -78,10 +78,6 @@ static int diff_delta__from_one( DIFF_FLAG_ISNT_SET(diff, GIT_DIFF_INCLUDE_UNTRACKED)) return 0; - if (entry->mode == GIT_FILEMODE_COMMIT && - DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_SUBMODULES)) - return 0; - if (!git_pathspec__match( &diff->pathspec, entry->path, DIFF_FLAG_IS_SET(diff, GIT_DIFF_DISABLE_PATHSPEC_MATCH), @@ -141,11 +137,6 @@ static int diff_delta__from_two( DIFF_FLAG_ISNT_SET(diff, GIT_DIFF_INCLUDE_UNMODIFIED)) return 0; - if (old_entry->mode == GIT_FILEMODE_COMMIT && - new_entry->mode == GIT_FILEMODE_COMMIT && - DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_SUBMODULES)) - return 0; - if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_REVERSE)) { uint32_t temp_mode = old_mode; const git_index_entry *temp_entry = old_entry; @@ -431,8 +422,18 @@ static int diff_list_apply_options( if (!opts) { diff->opts.context_lines = config_int(cfg, "diff.context", 3); - if (config_bool(cfg, "diff.ignoreSubmodules", 0)) - diff->opts.flags |= GIT_DIFF_IGNORE_SUBMODULES; + /* add other defaults here */ + } + + /* if ignore_submodules not explicitly set, check diff config */ + if (diff->opts.ignore_submodules <= 0) { + const char *str; + + if (git_config_get_string(&str , cfg, "diff.ignoreSubmodules") < 0) + giterr_clear(); + else if (str != NULL && + git_submodule_parse_ignore(&diff->opts.ignore_submodules, str) < 0) + giterr_clear(); } /* if either prefix is not set, figure out appropriate value */ @@ -596,36 +597,44 @@ static int maybe_modified_submodule( int error = 0; git_submodule *sub; unsigned int sm_status = 0; + git_submodule_ignore_t ign = diff->opts.ignore_submodules; *status = GIT_DELTA_UNMODIFIED; - if (!DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_SUBMODULES) && - !(error = git_submodule_lookup( - &sub, diff->repo, info->nitem->path)) && - git_submodule_ignore(sub) != GIT_SUBMODULE_IGNORE_ALL && - !(error = git_submodule__status( - &sm_status, NULL, NULL, found_oid, sub, - GIT_SUBMODULE_IGNORE_DEFAULT))) - { - /* check IS_WD_UNMODIFIED because this case is only used - * when the new side of the diff is the working directory - */ - if (!GIT_SUBMODULE_STATUS_IS_WD_UNMODIFIED(sm_status)) - *status = GIT_DELTA_MODIFIED; + if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_IGNORE_SUBMODULES) || + ign == GIT_SUBMODULE_IGNORE_ALL) + return 0; - /* now that we have a HEAD OID, check if HEAD moved */ - if ((sm_status & GIT_SUBMODULE_STATUS_IN_WD) != 0 && - !git_oid_equal(&info->oitem->oid, found_oid)) - *status = GIT_DELTA_MODIFIED; + if ((error = git_submodule_lookup( + &sub, diff->repo, info->nitem->path)) < 0) { + + /* GIT_EEXISTS means dir with .git in it was found - ignore it */ + if (error == GIT_EEXISTS) { + giterr_clear(); + error = 0; + } + return error; } - /* GIT_EEXISTS means a dir with .git in it was found - ignore it */ - if (error == GIT_EEXISTS) { - giterr_clear(); - error = 0; - } + if (ign <= 0 && git_submodule_ignore(sub) == GIT_SUBMODULE_IGNORE_ALL) + return 0; - return error; + if ((error = git_submodule__status( + &sm_status, NULL, NULL, found_oid, sub, ign)) < 0) + return error; + + /* check IS_WD_UNMODIFIED because this case is only used + * when the new side of the diff is the working directory + */ + if (!GIT_SUBMODULE_STATUS_IS_WD_UNMODIFIED(sm_status)) + *status = GIT_DELTA_MODIFIED; + + /* now that we have a HEAD OID, check if HEAD moved */ + if ((sm_status & GIT_SUBMODULE_STATUS_IN_WD) != 0 && + !git_oid_equal(&info->oitem->oid, found_oid)) + *status = GIT_DELTA_MODIFIED; + + return 0; } static int maybe_modified( diff --git a/src/submodule.c b/src/submodule.c index 0beedeb42..685906332 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -30,6 +30,8 @@ static git_cvar_map _sm_update_map[] = { {GIT_CVAR_STRING, "rebase", GIT_SUBMODULE_UPDATE_REBASE}, {GIT_CVAR_STRING, "merge", GIT_SUBMODULE_UPDATE_MERGE}, {GIT_CVAR_STRING, "none", GIT_SUBMODULE_UPDATE_NONE}, + {GIT_CVAR_FALSE, NULL, GIT_SUBMODULE_UPDATE_NONE}, + {GIT_CVAR_TRUE, NULL, GIT_SUBMODULE_UPDATE_CHECKOUT}, }; static git_cvar_map _sm_ignore_map[] = { @@ -37,6 +39,8 @@ static git_cvar_map _sm_ignore_map[] = { {GIT_CVAR_STRING, "untracked", GIT_SUBMODULE_IGNORE_UNTRACKED}, {GIT_CVAR_STRING, "dirty", GIT_SUBMODULE_IGNORE_DIRTY}, {GIT_CVAR_STRING, "all", GIT_SUBMODULE_IGNORE_ALL}, + {GIT_CVAR_FALSE, NULL, GIT_SUBMODULE_IGNORE_NONE}, + {GIT_CVAR_TRUE, NULL, GIT_SUBMODULE_IGNORE_ALL}, }; static kh_inline khint_t str_hash_no_trailing_slash(const char *s) @@ -406,11 +410,30 @@ cleanup: return error; } +const char *git_submodule_ignore_to_str(git_submodule_ignore_t ignore) +{ + int i; + for (i = 0; i < (int)ARRAY_SIZE(_sm_ignore_map); ++i) + if (_sm_ignore_map[i].map_value == ignore) + return _sm_ignore_map[i].str_match; + return NULL; +} + +const char *git_submodule_update_to_str(git_submodule_update_t update) +{ + int i; + for (i = 0; i < (int)ARRAY_SIZE(_sm_update_map); ++i) + if (_sm_update_map[i].map_value == update) + return _sm_update_map[i].str_match; + return NULL; +} + int git_submodule_save(git_submodule *submodule) { int error = 0; git_config_backend *mods; git_buf key = GIT_BUF_INIT; + const char *val; assert(submodule); @@ -435,22 +458,14 @@ int git_submodule_save(git_submodule *submodule) goto cleanup; if (!(error = submodule_config_key_trunc_puts(&key, "update")) && - submodule->update != GIT_SUBMODULE_UPDATE_DEFAULT) - { - const char *val = (submodule->update == GIT_SUBMODULE_UPDATE_CHECKOUT) ? - NULL : _sm_update_map[submodule->update].str_match; + (val = git_submodule_update_to_str(submodule->update)) != NULL) error = git_config_file_set_string(mods, key.ptr, val); - } if (error < 0) goto cleanup; if (!(error = submodule_config_key_trunc_puts(&key, "ignore")) && - submodule->ignore != GIT_SUBMODULE_IGNORE_DEFAULT) - { - const char *val = (submodule->ignore == GIT_SUBMODULE_IGNORE_NONE) ? - NULL : _sm_ignore_map[submodule->ignore].str_match; + (val = git_submodule_ignore_to_str(submodule->ignore)) != NULL) error = git_config_file_set_string(mods, key.ptr, val); - } if (error < 0) goto cleanup; @@ -554,7 +569,8 @@ const git_oid *git_submodule_wd_id(git_submodule *submodule) git_submodule_ignore_t git_submodule_ignore(git_submodule *submodule) { assert(submodule); - return submodule->ignore; + return (submodule->ignore < GIT_SUBMODULE_IGNORE_NONE) ? + GIT_SUBMODULE_IGNORE_NONE : submodule->ignore; } git_submodule_ignore_t git_submodule_set_ignore( @@ -564,7 +580,7 @@ git_submodule_ignore_t git_submodule_set_ignore( assert(submodule); - if (ignore == GIT_SUBMODULE_IGNORE_DEFAULT) + if (ignore == GIT_SUBMODULE_IGNORE_RESET) ignore = submodule->ignore_default; old = submodule->ignore; @@ -575,7 +591,8 @@ git_submodule_ignore_t git_submodule_set_ignore( git_submodule_update_t git_submodule_update(git_submodule *submodule) { assert(submodule); - return submodule->update; + return (submodule->update < GIT_SUBMODULE_UPDATE_CHECKOUT) ? + GIT_SUBMODULE_UPDATE_CHECKOUT : submodule->update; } git_submodule_update_t git_submodule_set_update( @@ -585,7 +602,7 @@ git_submodule_update_t git_submodule_set_update( assert(submodule); - if (update == GIT_SUBMODULE_UPDATE_DEFAULT) + if (update == GIT_SUBMODULE_UPDATE_RESET) update = submodule->update_default; old = submodule->update; @@ -616,6 +633,7 @@ int git_submodule_set_fetch_recurse_submodules( int git_submodule_init(git_submodule *submodule, int overwrite) { int error; + const char *val; /* write "submodule.NAME.url" */ @@ -632,14 +650,10 @@ int git_submodule_init(git_submodule *submodule, int overwrite) /* write "submodule.NAME.update" if not default */ - if (submodule->update == GIT_SUBMODULE_UPDATE_CHECKOUT) - error = submodule_update_config( - submodule, "update", NULL, (overwrite != 0), false); - else if (submodule->update != GIT_SUBMODULE_UPDATE_DEFAULT) - error = submodule_update_config( - submodule, "update", - _sm_update_map[submodule->update].str_match, - (overwrite != 0), false); + val = (submodule->update == GIT_SUBMODULE_UPDATE_CHECKOUT) ? + NULL : git_submodule_update_to_str(submodule->update); + error = submodule_update_config( + submodule, "update", val, (overwrite != 0), false); return error; } @@ -873,7 +887,7 @@ int git_submodule__status( unsigned int status; git_repository *smrepo = NULL; - if (ign == GIT_SUBMODULE_IGNORE_DEFAULT) + if (ign < GIT_SUBMODULE_IGNORE_NONE) ign = sm->ignore; /* only return location info if ignore == all */ @@ -926,8 +940,7 @@ int git_submodule_status(unsigned int *status, git_submodule *sm) { assert(status && sm); - return git_submodule__status( - status, NULL, NULL, NULL, sm, GIT_SUBMODULE_IGNORE_DEFAULT); + return git_submodule__status(status, NULL, NULL, NULL, sm, 0); } int git_submodule_location(unsigned int *location, git_submodule *sm) @@ -964,7 +977,10 @@ static git_submodule *submodule_alloc(git_repository *repo, const char *name) } GIT_REFCOUNT_INC(sm); - sm->repo = repo; + sm->ignore = sm->ignore_default = GIT_SUBMODULE_IGNORE_NONE; + sm->update = sm->update_default = GIT_SUBMODULE_UPDATE_CHECKOUT; + sm->repo = repo; + return sm; } @@ -1039,6 +1055,34 @@ static int submodule_config_error(const char *property, const char *value) return -1; } +int git_submodule_parse_ignore(git_submodule_ignore_t *out, const char *value) +{ + int val; + + if (git_config_lookup_map_value( + &val, _sm_ignore_map, ARRAY_SIZE(_sm_ignore_map), value) < 0) { + *out = GIT_SUBMODULE_IGNORE_NONE; + return submodule_config_error("ignore", value); + } + + *out = (git_submodule_ignore_t)val; + return 0; +} + +int git_submodule_parse_update(git_submodule_update_t *out, const char *value) +{ + int val; + + if (git_config_lookup_map_value( + &val, _sm_update_map, ARRAY_SIZE(_sm_update_map), value) < 0) { + *out = GIT_SUBMODULE_UPDATE_CHECKOUT; + return submodule_config_error("update", value); + } + + *out = (git_submodule_update_t)val; + return 0; +} + static int submodule_load_from_config( const git_config_entry *entry, void *data) { @@ -1120,22 +1164,18 @@ static int submodule_load_from_config( return -1; } else if (strcasecmp(property, "update") == 0) { - int val; - if (git_config_lookup_map_value( - &val, _sm_update_map, ARRAY_SIZE(_sm_update_map), value) < 0) - return submodule_config_error("update", value); - sm->update_default = sm->update = (git_submodule_update_t)val; + if (git_submodule_parse_update(&sm->update, value) < 0) + return -1; + sm->update_default = sm->update; } else if (strcasecmp(property, "fetchRecurseSubmodules") == 0) { if (git__parse_bool(&sm->fetch_recurse, value) < 0) return submodule_config_error("fetchRecurseSubmodules", value); } else if (strcasecmp(property, "ignore") == 0) { - int val; - if (git_config_lookup_map_value( - &val, _sm_ignore_map, ARRAY_SIZE(_sm_ignore_map), value) < 0) - return submodule_config_error("ignore", value); - sm->ignore_default = sm->ignore = (git_submodule_ignore_t)val; + if (git_submodule_parse_ignore(&sm->ignore, value) < 0) + return -1; + sm->ignore_default = sm->ignore; } /* ignore other unknown submodule properties */ diff --git a/src/submodule.h b/src/submodule.h index 8db2fff29..b05937503 100644 --- a/src/submodule.h +++ b/src/submodule.h @@ -127,4 +127,12 @@ extern int git_submodule_open_bare( /* Release reference to submodule object - not currently for external use */ extern void git_submodule_free(git_submodule *sm); +extern int git_submodule_parse_ignore( + git_submodule_ignore_t *out, const char *value); +extern int git_submodule_parse_update( + git_submodule_update_t *out, const char *value); + +extern const char *git_submodule_ignore_to_str(git_submodule_ignore_t); +extern const char *git_submodule_update_to_str(git_submodule_update_t); + #endif diff --git a/tests-clar/clar_libgit2.h b/tests-clar/clar_libgit2.h index 3fcf45a37..5371b2864 100644 --- a/tests-clar/clar_libgit2.h +++ b/tests-clar/clar_libgit2.h @@ -29,6 +29,9 @@ void cl_git_report_failure(int, const char *, int, const char *); +#define cl_assert_at_line(expr,file,line) \ + clar__assert((expr) != 0, file, line, "Expression is not true: " #expr, NULL, 1) + #define cl_assert_equal_sz(sz1,sz2) cl_assert_equal_i((int)sz1, (int)(sz2)) /* diff --git a/tests-clar/diff/submodules.c b/tests-clar/diff/submodules.c index 4a40affb2..2e425bb11 100644 --- a/tests-clar/diff/submodules.c +++ b/tests-clar/diff/submodules.c @@ -49,7 +49,7 @@ static void check_diff_patches_at_line( cl_git_pass(git_diff_get_patch(&patch, &delta, diff, d)); if (delta->status == GIT_DELTA_UNMODIFIED) { - clar__assert(expected[d] == NULL, file, line, "found UNMODIFIED delta where modified was expected", NULL, 1); + cl_assert_at_line(expected[d] == NULL, file, line); continue; } @@ -57,7 +57,7 @@ static void check_diff_patches_at_line( continue; if (expected[d] && !strcmp(expected[d], "")) { cl_git_pass(git_diff_patch_to_str(&patch_text, patch)); - clar__assert(0, file, line, "expected end of deltas, but found more", patch_text, 1); + cl_assert_at_line(!strcmp(expected[d], ""), file, line); } cl_git_pass(git_diff_patch_to_str(&patch_text, patch)); @@ -67,7 +67,7 @@ static void check_diff_patches_at_line( git__free(patch_text); } - clar__assert(expected[d] && !strcmp(expected[d], ""), file, line, "found fewer deltas than expected", expected[d], 1); + cl_assert_at_line(expected[d] && !strcmp(expected[d], ""), file, line); } #define check_diff_patches(diff, exp) \ @@ -382,3 +382,62 @@ void test_diff_submodules__invalid_cache(void) git_index_free(smindex); git_repository_free(smrepo); } + +void test_diff_submodules__diff_ignore_options(void) +{ + git_diff_options opts = GIT_DIFF_OPTIONS_INIT; + git_diff_list *diff = NULL; + static const char *expected_normal[] = { + "", /* .gitmodules */ + NULL, /* not-submodule */ + NULL, /* not */ + "diff --git a/sm_changed_file b/sm_changed_file\nindex 4800958..4800958 160000\n--- a/sm_changed_file\n+++ b/sm_changed_file\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0-dirty\n", /* sm_changed_file */ + "diff --git a/sm_changed_head b/sm_changed_head\nindex 4800958..3d9386c 160000\n--- a/sm_changed_head\n+++ b/sm_changed_head\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 3d9386c507f6b093471a3e324085657a3c2b4247\n", /* sm_changed_head */ + "diff --git a/sm_changed_index b/sm_changed_index\nindex 4800958..4800958 160000\n--- a/sm_changed_index\n+++ b/sm_changed_index\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0-dirty\n", /* sm_changed_index */ + "diff --git a/sm_changed_untracked_file b/sm_changed_untracked_file\nindex 4800958..4800958 160000\n--- a/sm_changed_untracked_file\n+++ b/sm_changed_untracked_file\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0-dirty\n", /* sm_changed_untracked_file */ + "diff --git a/sm_missing_commits b/sm_missing_commits\nindex 4800958..5e49635 160000\n--- a/sm_missing_commits\n+++ b/sm_missing_commits\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 5e4963595a9774b90524d35a807169049de8ccad\n", /* sm_missing_commits */ + "" + }; + static const char *expected_ignore_all[] = { + "", /* .gitmodules */ + NULL, /* not-submodule */ + NULL, /* not */ + "" + }; + static const char *expected_ignore_dirty[] = { + "", /* .gitmodules */ + NULL, /* not-submodule */ + NULL, /* not */ + "diff --git a/sm_changed_head b/sm_changed_head\nindex 4800958..3d9386c 160000\n--- a/sm_changed_head\n+++ b/sm_changed_head\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 3d9386c507f6b093471a3e324085657a3c2b4247\n", /* sm_changed_head */ + "diff --git a/sm_missing_commits b/sm_missing_commits\nindex 4800958..5e49635 160000\n--- a/sm_missing_commits\n+++ b/sm_missing_commits\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 5e4963595a9774b90524d35a807169049de8ccad\n", /* sm_missing_commits */ + "" + }; + + setup_submodules2(); + + opts.flags = GIT_DIFF_INCLUDE_UNTRACKED; + opts.old_prefix = "a"; opts.new_prefix = "b"; + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_normal); + git_diff_list_free(diff); + + opts.flags |= GIT_DIFF_IGNORE_SUBMODULES; + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_ignore_all); + git_diff_list_free(diff); + + opts.flags &= ~GIT_DIFF_IGNORE_SUBMODULES; + opts.ignore_submodules = GIT_SUBMODULE_IGNORE_ALL; + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_ignore_all); + git_diff_list_free(diff); + + opts.ignore_submodules = GIT_SUBMODULE_IGNORE_DIRTY; + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_ignore_dirty); + git_diff_list_free(diff); +} diff --git a/tests-clar/submodule/modify.c b/tests-clar/submodule/modify.c index 94eb3738a..c0498ce6e 100644 --- a/tests-clar/submodule/modify.c +++ b/tests-clar/submodule/modify.c @@ -204,10 +204,10 @@ void test_submodule_modify__edit_and_save(void) cl_git_pass(git_submodule_set_url(sm1, old_url)); cl_assert_equal_i( (int)GIT_SUBMODULE_IGNORE_UNTRACKED, - (int)git_submodule_set_ignore(sm1, GIT_SUBMODULE_IGNORE_DEFAULT)); + (int)git_submodule_set_ignore(sm1, GIT_SUBMODULE_IGNORE_RESET)); cl_assert_equal_i( (int)GIT_SUBMODULE_UPDATE_REBASE, - (int)git_submodule_set_update(sm1, GIT_SUBMODULE_UPDATE_DEFAULT)); + (int)git_submodule_set_update(sm1, GIT_SUBMODULE_UPDATE_RESET)); cl_assert_equal_i( 1, git_submodule_set_fetch_recurse_submodules(sm1, old_fetchrecurse)); @@ -228,10 +228,10 @@ void test_submodule_modify__edit_and_save(void) cl_git_pass(git_submodule_save(sm1)); /* attempt to "revert" values */ - git_submodule_set_ignore(sm1, GIT_SUBMODULE_IGNORE_DEFAULT); - git_submodule_set_update(sm1, GIT_SUBMODULE_UPDATE_DEFAULT); + git_submodule_set_ignore(sm1, GIT_SUBMODULE_IGNORE_RESET); + git_submodule_set_update(sm1, GIT_SUBMODULE_UPDATE_RESET); - /* but ignore and update should NOT revert because the DEFAULT + /* but ignore and update should NOT revert because the RESET * should now be the newly saved value... */ cl_assert_equal_i( From b8df28a5dae65b617ce85e1937066093600880af Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Sun, 30 Jun 2013 08:38:10 -0700 Subject: [PATCH 13/15] Clean up left over alloc change --- src/submodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/submodule.c b/src/submodule.c index 685906332..b5dacc42e 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -966,7 +966,7 @@ static git_submodule *submodule_alloc(git_repository *repo, const char *name) return NULL; } - sm = git__calloc(1, sizeof(git_submodule) + namelen + 1); + sm = git__calloc(1, sizeof(git_submodule)); if (sm == NULL) return NULL; From 9564229af42b68d205376853410c55b957546a14 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Sun, 30 Jun 2013 08:43:07 -0700 Subject: [PATCH 14/15] Add tests for diff.ignoreSubmdules config --- tests-clar/diff/submodules.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests-clar/diff/submodules.c b/tests-clar/diff/submodules.c index 2e425bb11..fe9cf1d98 100644 --- a/tests-clar/diff/submodules.c +++ b/tests-clar/diff/submodules.c @@ -387,6 +387,7 @@ void test_diff_submodules__diff_ignore_options(void) { git_diff_options opts = GIT_DIFF_OPTIONS_INIT; git_diff_list *diff = NULL; + git_config *cfg; static const char *expected_normal[] = { "", /* .gitmodules */ NULL, /* not-submodule */ @@ -440,4 +441,32 @@ void test_diff_submodules__diff_ignore_options(void) cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); check_diff_patches(diff, expected_ignore_dirty); git_diff_list_free(diff); + + opts.ignore_submodules = 0; + cl_git_pass(git_repository_config(&cfg, g_repo)); + cl_git_pass(git_config_set_bool(cfg, "diff.ignoreSubmodules", false)); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_normal); + git_diff_list_free(diff); + + cl_git_pass(git_config_set_bool(cfg, "diff.ignoreSubmodules", true)); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_ignore_all); + git_diff_list_free(diff); + + cl_git_pass(git_config_set_string(cfg, "diff.ignoreSubmodules", "none")); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_normal); + git_diff_list_free(diff); + + cl_git_pass(git_config_set_string(cfg, "diff.ignoreSubmodules", "dirty")); + + cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts)); + check_diff_patches(diff, expected_ignore_dirty); + git_diff_list_free(diff); + + git_config_free(cfg); } From 125655fe3f0caf8b3d9fff2ec45ec694b34eed04 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 2 Jul 2013 16:49:57 -0700 Subject: [PATCH 15/15] Untracked directories with .git should be ignored This restores a behavior that was accidentally lost during some diff refactoring where an untracked directory that contains a .git item should be treated as IGNORED, not as UNTRACKED. The submodule code already detects this, but the diff code was not handling the scenario right. This also updates a number of existing tests that were actually exercising the behavior but did not have the right expectations in place. It actually makes the new `test_diff_submodules__diff_ignore_options` test feel much better because the "not-a-submodule" entries are now ignored instead of showing up as untracked items. Fixes #1697 --- src/diff.c | 14 ++++++- tests-clar/diff/submodules.c | 47 ++++-------------------- tests-clar/diff/workdir.c | 7 ++-- tests-clar/status/submodules.c | 21 ++++++----- tests-clar/submodule/status.c | 35 ++++++++++++------ tests-clar/submodule/submodule_helpers.c | 35 ++++++++++++++++++ tests-clar/submodule/submodule_helpers.h | 3 ++ 7 files changed, 97 insertions(+), 65 deletions(-) diff --git a/src/diff.c b/src/diff.c index 5fa635f05..e875d09b3 100644 --- a/src/diff.c +++ b/src/diff.c @@ -735,7 +735,7 @@ static int maybe_modified( /* if we got here and decided that the files are modified, but we * haven't calculated the OID of the new item, then calculate it now */ - if (status != GIT_DELTA_UNMODIFIED && git_oid_iszero(&nitem->oid)) { + if (status == GIT_DELTA_MODIFIED && git_oid_iszero(&nitem->oid)) { if (git_oid_iszero(&noid)) { if (git_diff__oid_for_file(diff->repo, nitem->path, nitem->mode, nitem->file_size, &noid) < 0) @@ -858,7 +858,7 @@ static int handle_unmatched_new_item( git_buf_clear(&info->ignore_prefix); } - if (S_ISDIR(nitem->mode)) { + if (nitem->mode == GIT_FILEMODE_TREE) { bool recurse_into_dir = contains_oitem; /* if not already inside an ignored dir, check if this is ignored */ @@ -962,6 +962,16 @@ static int handle_unmatched_new_item( else if (info->new_iter->type != GIT_ITERATOR_TYPE_WORKDIR) delta_type = GIT_DELTA_ADDED; + else if (nitem->mode == GIT_FILEMODE_COMMIT) { + git_submodule *sm; + + /* ignore things that are not actual submodules */ + if (git_submodule_lookup(&sm, info->repo, nitem->path) != 0) { + giterr_clear(); + delta_type = GIT_DELTA_IGNORED; + } + } + /* Actually create the record for this item if necessary */ if ((error = diff_delta__from_one(diff, delta_type, nitem)) < 0) return error; diff --git a/tests-clar/diff/submodules.c b/tests-clar/diff/submodules.c index fe9cf1d98..94804db22 100644 --- a/tests-clar/diff/submodules.c +++ b/tests-clar/diff/submodules.c @@ -5,36 +5,13 @@ static git_repository *g_repo = NULL; -static void setup_submodules(void) -{ - g_repo = cl_git_sandbox_init("submodules"); - cl_fixture_sandbox("testrepo.git"); - rewrite_gitmodules(git_repository_workdir(g_repo)); - p_rename("submodules/testrepo/.gitted", "submodules/testrepo/.git"); -} - -static void setup_submodules2(void) -{ - g_repo = cl_git_sandbox_init("submod2"); - - cl_fixture_sandbox("submod2_target"); - p_rename("submod2_target/.gitted", "submod2_target/.git"); - - rewrite_gitmodules(git_repository_workdir(g_repo)); - p_rename("submod2/not-submodule/.gitted", "submod2/not-submodule/.git"); - p_rename("submod2/not/.gitted", "submod2/not/.git"); -} - void test_diff_submodules__initialize(void) { } void test_diff_submodules__cleanup(void) { - cl_git_sandbox_cleanup(); - - cl_fixture_cleanup("testrepo.git"); - cl_fixture_cleanup("submod2_target"); + cleanup_fixture_submodules(); } static void check_diff_patches_at_line( @@ -88,7 +65,7 @@ void test_diff_submodules__unmodified_submodule(void) "" }; - setup_submodules(); + g_repo = setup_fixture_submodules(); opts.flags = GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED | @@ -115,7 +92,7 @@ void test_diff_submodules__dirty_submodule(void) "" }; - setup_submodules(); + g_repo = setup_fixture_submodules(); cl_git_rewritefile("submodules/testrepo/README", "heyheyhey"); cl_git_mkfile("submodules/testrepo/all_new.txt", "never seen before"); @@ -141,7 +118,7 @@ void test_diff_submodules__dirty_submodule_2(void) "" }; - setup_submodules(); + g_repo = setup_fixture_submodules(); cl_git_pass(git_submodule_reload_all(g_repo)); @@ -190,8 +167,6 @@ void test_diff_submodules__submod2_index_to_wd(void) git_diff_list *diff = NULL; static const char *expected[] = { "", /* .gitmodules */ - NULL, /* not-submodule */ - NULL, /* not */ "diff --git a/sm_changed_file b/sm_changed_file\nindex 4800958..4800958 160000\n--- a/sm_changed_file\n+++ b/sm_changed_file\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0-dirty\n", /* sm_changed_file */ "diff --git a/sm_changed_head b/sm_changed_head\nindex 4800958..3d9386c 160000\n--- a/sm_changed_head\n+++ b/sm_changed_head\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 3d9386c507f6b093471a3e324085657a3c2b4247\n", /* sm_changed_head */ "diff --git a/sm_changed_index b/sm_changed_index\nindex 4800958..4800958 160000\n--- a/sm_changed_index\n+++ b/sm_changed_index\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0-dirty\n", /* sm_changed_index */ @@ -200,7 +175,7 @@ void test_diff_submodules__submod2_index_to_wd(void) "" }; - setup_submodules2(); + g_repo = setup_fixture_submod2(); opts.flags = GIT_DIFF_INCLUDE_UNTRACKED; opts.old_prefix = "a"; opts.new_prefix = "b"; @@ -221,7 +196,7 @@ void test_diff_submodules__submod2_head_to_index(void) "" }; - setup_submodules2(); + g_repo = setup_fixture_submod2(); cl_git_pass(git_repository_head_tree(&head, g_repo)); @@ -261,7 +236,7 @@ void test_diff_submodules__invalid_cache(void) "" }; - setup_submodules2(); + g_repo = setup_fixture_submod2(); opts.flags = GIT_DIFF_INCLUDE_UNTRACKED; opts.old_prefix = "a"; opts.new_prefix = "b"; @@ -390,8 +365,6 @@ void test_diff_submodules__diff_ignore_options(void) git_config *cfg; static const char *expected_normal[] = { "", /* .gitmodules */ - NULL, /* not-submodule */ - NULL, /* not */ "diff --git a/sm_changed_file b/sm_changed_file\nindex 4800958..4800958 160000\n--- a/sm_changed_file\n+++ b/sm_changed_file\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0-dirty\n", /* sm_changed_file */ "diff --git a/sm_changed_head b/sm_changed_head\nindex 4800958..3d9386c 160000\n--- a/sm_changed_head\n+++ b/sm_changed_head\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 3d9386c507f6b093471a3e324085657a3c2b4247\n", /* sm_changed_head */ "diff --git a/sm_changed_index b/sm_changed_index\nindex 4800958..4800958 160000\n--- a/sm_changed_index\n+++ b/sm_changed_index\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0-dirty\n", /* sm_changed_index */ @@ -401,20 +374,16 @@ void test_diff_submodules__diff_ignore_options(void) }; static const char *expected_ignore_all[] = { "", /* .gitmodules */ - NULL, /* not-submodule */ - NULL, /* not */ "" }; static const char *expected_ignore_dirty[] = { "", /* .gitmodules */ - NULL, /* not-submodule */ - NULL, /* not */ "diff --git a/sm_changed_head b/sm_changed_head\nindex 4800958..3d9386c 160000\n--- a/sm_changed_head\n+++ b/sm_changed_head\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 3d9386c507f6b093471a3e324085657a3c2b4247\n", /* sm_changed_head */ "diff --git a/sm_missing_commits b/sm_missing_commits\nindex 4800958..5e49635 160000\n--- a/sm_missing_commits\n+++ b/sm_missing_commits\n@@ -1 +1 @@\n-Subproject commit 480095882d281ed676fe5b863569520e54a7d5c0\n+Subproject commit 5e4963595a9774b90524d35a807169049de8ccad\n", /* sm_missing_commits */ "" }; - setup_submodules2(); + g_repo = setup_fixture_submod2(); opts.flags = GIT_DIFF_INCLUDE_UNTRACKED; opts.old_prefix = "a"; opts.new_prefix = "b"; diff --git a/tests-clar/diff/workdir.c b/tests-clar/diff/workdir.c index 6a2504d09..c7fac1e48 100644 --- a/tests-clar/diff/workdir.c +++ b/tests-clar/diff/workdir.c @@ -776,6 +776,7 @@ void test_diff_workdir__submodules(void) opts.flags = GIT_DIFF_INCLUDE_UNTRACKED | + GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_RECURSE_UNTRACKED_DIRS | GIT_DIFF_INCLUDE_UNTRACKED_CONTENT; @@ -806,7 +807,7 @@ void test_diff_workdir__submodules(void) * only significant difference is that those Added items will show up * as Untracked items in the pure libgit2 diff. * - * Then add in the two extra untracked items "not" and "not-submodule" + * Then add in the two extra ignored items "not" and "not-submodule" * to get the 12 files reported here. */ @@ -815,8 +816,8 @@ void test_diff_workdir__submodules(void) cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]); cl_assert_equal_i(0, exp.file_status[GIT_DELTA_DELETED]); cl_assert_equal_i(2, exp.file_status[GIT_DELTA_MODIFIED]); - cl_assert_equal_i(0, exp.file_status[GIT_DELTA_IGNORED]); - cl_assert_equal_i(10, exp.file_status[GIT_DELTA_UNTRACKED]); + cl_assert_equal_i(2, exp.file_status[GIT_DELTA_IGNORED]); + cl_assert_equal_i(8, exp.file_status[GIT_DELTA_UNTRACKED]); /* the following numbers match "git diff 873585" exactly */ diff --git a/tests-clar/status/submodules.c b/tests-clar/status/submodules.c index af8707721..7bfef503f 100644 --- a/tests-clar/status/submodules.c +++ b/tests-clar/status/submodules.c @@ -9,25 +9,19 @@ static git_repository *g_repo = NULL; void test_status_submodules__initialize(void) { - g_repo = cl_git_sandbox_init("submodules"); - - cl_fixture_sandbox("testrepo.git"); - - rewrite_gitmodules(git_repository_workdir(g_repo)); - - p_rename("submodules/testrepo/.gitted", "submodules/testrepo/.git"); } void test_status_submodules__cleanup(void) { - cl_git_sandbox_cleanup(); - cl_fixture_cleanup("testrepo.git"); + cleanup_fixture_submodules(); } void test_status_submodules__api(void) { git_submodule *sm; + g_repo = setup_fixture_submodules(); + cl_assert(git_submodule_lookup(NULL, g_repo, "nonexistent") == GIT_ENOTFOUND); cl_assert(git_submodule_lookup(NULL, g_repo, "modified") == GIT_ENOTFOUND); @@ -42,6 +36,8 @@ void test_status_submodules__0(void) { int counts = 0; + g_repo = setup_fixture_submodules(); + cl_assert(git_path_isdir("submodules/.git")); cl_assert(git_path_isdir("submodules/testrepo/.git")); cl_assert(git_path_isfile("submodules/.gitmodules")); @@ -86,6 +82,8 @@ void test_status_submodules__1(void) { status_entry_counts counts; + g_repo = setup_fixture_submodules(); + cl_assert(git_path_isdir("submodules/.git")); cl_assert(git_path_isdir("submodules/testrepo/.git")); cl_assert(git_path_isfile("submodules/.gitmodules")); @@ -104,6 +102,7 @@ void test_status_submodules__1(void) void test_status_submodules__single_file(void) { unsigned int status = 0; + g_repo = setup_fixture_submodules(); cl_git_pass( git_status_file(&status, g_repo, "testrepo") ); cl_assert(!status); } @@ -134,6 +133,8 @@ void test_status_submodules__moved_head(void) GIT_STATUS_WT_NEW }; + g_repo = setup_fixture_submodules(); + cl_git_pass(git_submodule_lookup(&sm, g_repo, "testrepo")); cl_git_pass(git_submodule_open(&smrepo, sm)); @@ -192,6 +193,8 @@ void test_status_submodules__dirty_workdir_only(void) GIT_STATUS_WT_NEW }; + g_repo = setup_fixture_submodules(); + cl_git_rewritefile("submodules/testrepo/README", "heyheyhey"); cl_git_mkfile("submodules/testrepo/all_new.txt", "never seen before"); diff --git a/tests-clar/submodule/status.c b/tests-clar/submodule/status.c index 68110bdd5..7b29ac288 100644 --- a/tests-clar/submodule/status.c +++ b/tests-clar/submodule/status.c @@ -9,21 +9,12 @@ static git_repository *g_repo = NULL; void test_submodule_status__initialize(void) { - g_repo = cl_git_sandbox_init("submod2"); - - cl_fixture_sandbox("submod2_target"); - p_rename("submod2_target/.gitted", "submod2_target/.git"); - - /* must create submod2_target before rewrite so prettify will work */ - rewrite_gitmodules(git_repository_workdir(g_repo)); - p_rename("submod2/not-submodule/.gitted", "submod2/not-submodule/.git"); - p_rename("submod2/not/.gitted", "submod2/not/.git"); + g_repo = setup_fixture_submod2(); } void test_submodule_status__cleanup(void) { - cl_git_sandbox_cleanup(); - cl_fixture_cleanup("submod2_target"); + cleanup_fixture_submodules(); } void test_submodule_status__unchanged(void) @@ -326,6 +317,7 @@ void test_submodule_status__ignore_all(void) typedef struct { size_t counter; const char **paths; + int *statuses; } submodule_expectations; static int confirm_submodule_status( @@ -336,6 +328,7 @@ static int confirm_submodule_status( while (git__suffixcmp(exp->paths[exp->counter], "/") == 0) exp->counter++; + cl_assert_equal_i(exp->statuses[exp->counter], (int)status_flags); cl_assert_equal_s(exp->paths[exp->counter++], path); GIT_UNUSED(status_flags); @@ -365,7 +358,24 @@ void test_submodule_status__iterator(void) "sm_unchanged", NULL }; - submodule_expectations exp = { 0, expected }; + static int expected_flags[] = { + GIT_STATUS_INDEX_MODIFIED | GIT_STATUS_WT_MODIFIED, /* ".gitmodules" */ + 0, /* "just_a_dir/" will be skipped */ + GIT_STATUS_CURRENT, /* "just_a_dir/contents" */ + GIT_STATUS_CURRENT, /* "just_a_file" */ + GIT_STATUS_IGNORED, /* "not" (contains .git) */ + GIT_STATUS_IGNORED, /* "not-submodule" (contains .git) */ + GIT_STATUS_CURRENT, /* "README.txt */ + GIT_STATUS_INDEX_NEW, /* "sm_added_and_uncommited" */ + GIT_STATUS_WT_MODIFIED, /* "sm_changed_file" */ + GIT_STATUS_WT_MODIFIED, /* "sm_changed_head" */ + GIT_STATUS_WT_MODIFIED, /* "sm_changed_index" */ + GIT_STATUS_WT_MODIFIED, /* "sm_changed_untracked_file" */ + GIT_STATUS_WT_MODIFIED, /* "sm_missing_commits" */ + GIT_STATUS_CURRENT, /* "sm_unchanged" */ + 0 + }; + submodule_expectations exp = { 0, expected, expected_flags }; git_status_options opts = GIT_STATUS_OPTIONS_INIT; cl_git_pass(git_iterator_for_workdir(&iter, g_repo, @@ -378,6 +388,7 @@ void test_submodule_status__iterator(void) opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED | GIT_STATUS_OPT_INCLUDE_UNMODIFIED | + GIT_STATUS_OPT_INCLUDE_IGNORED | GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS; cl_git_pass(git_status_foreach_ext( diff --git a/tests-clar/submodule/submodule_helpers.c b/tests-clar/submodule/submodule_helpers.c index 0c3e79f71..a7807522b 100644 --- a/tests-clar/submodule/submodule_helpers.c +++ b/tests-clar/submodule/submodule_helpers.c @@ -82,3 +82,38 @@ void rewrite_gitmodules(const char *workdir) git_buf_free(&out_f); git_buf_free(&path); } + +git_repository *setup_fixture_submodules(void) +{ + git_repository *repo = cl_git_sandbox_init("submodules"); + + cl_fixture_sandbox("testrepo.git"); + + rewrite_gitmodules(git_repository_workdir(repo)); + p_rename("submodules/testrepo/.gitted", "submodules/testrepo/.git"); + + return repo; +} + +git_repository *setup_fixture_submod2(void) +{ + git_repository *repo = cl_git_sandbox_init("submod2"); + + cl_fixture_sandbox("submod2_target"); + p_rename("submod2_target/.gitted", "submod2_target/.git"); + + rewrite_gitmodules(git_repository_workdir(repo)); + p_rename("submod2/not-submodule/.gitted", "submod2/not-submodule/.git"); + p_rename("submod2/not/.gitted", "submod2/not/.git"); + + return repo; +} + +void cleanup_fixture_submodules(void) +{ + cl_git_sandbox_cleanup(); + + /* just try to clean up both possible extras */ + cl_fixture_cleanup("testrepo.git"); + cl_fixture_cleanup("submod2_target"); +} diff --git a/tests-clar/submodule/submodule_helpers.h b/tests-clar/submodule/submodule_helpers.h index 6b76a832e..1de15ca17 100644 --- a/tests-clar/submodule/submodule_helpers.h +++ b/tests-clar/submodule/submodule_helpers.h @@ -1,2 +1,5 @@ extern void rewrite_gitmodules(const char *workdir); +extern git_repository *setup_fixture_submodules(void); +extern git_repository *setup_fixture_submod2(void); +extern void cleanup_fixture_submodules(void);