diff --git a/include/git2/submodule.h b/include/git2/submodule.h index 28057d26f..b00fad2d4 100644 --- a/include/git2/submodule.h +++ b/include/git2/submodule.h @@ -120,8 +120,15 @@ typedef enum { } git_submodule_status_t; #define GIT_SUBMODULE_STATUS_IS_UNMODIFIED(S) \ - (((S) & ~(GIT_SUBMODULE_STATUS_IN_HEAD | GIT_SUBMODULE_STATUS_IN_INDEX | \ - GIT_SUBMODULE_STATUS_IN_CONFIG | GIT_SUBMODULE_STATUS_IN_WD)) == 0) + (((S) & ~(GIT_SUBMODULE_STATUS_IN_HEAD | \ + GIT_SUBMODULE_STATUS_IN_INDEX | \ + GIT_SUBMODULE_STATUS_IN_CONFIG | \ + GIT_SUBMODULE_STATUS_IN_WD)) == 0) + +#define GIT_SUBMODULE_STATUS_IS_WD_DIRTY(S) \ + (((S) & (GIT_SUBMODULE_STATUS_WD_INDEX_MODIFIED | \ + GIT_SUBMODULE_STATUS_WD_WD_MODIFIED | \ + GIT_SUBMODULE_STATUS_WD_UNTRACKED)) != 0) /** * Lookup submodule information by name or path. diff --git a/src/diff.c b/src/diff.c index 8718e5ada..8ab8af3a1 100644 --- a/src/diff.c +++ b/src/diff.c @@ -224,7 +224,10 @@ static int diff_delta__from_one( } delta->old_file.flags |= GIT_DIFF_FILE_VALID_OID; - delta->new_file.flags |= GIT_DIFF_FILE_VALID_OID; + + if (delta->status == GIT_DELTA_DELETED || + !git_oid_iszero(&delta->new_file.oid)) + delta->new_file.flags |= GIT_DIFF_FILE_VALID_OID; if (git_vector_insert(&diff->deltas, delta) < 0) { git__free(delta); @@ -441,17 +444,28 @@ static int oid_for_workdir_item( const git_index_entry *item, git_oid *oid) { - int result; + int result = 0; git_buf full_path = GIT_BUF_INIT; - if (git_buf_joinpath(&full_path, git_repository_workdir(repo), item->path) < 0) + if (git_buf_joinpath( + &full_path, git_repository_workdir(repo), item->path) < 0) return -1; - /* calculate OID for file if possible*/ + /* calculate OID for file if possible */ if (S_ISGITLINK(item->mode)) { - /* Don't bother to figure out an oid for a submodule. We won't use it anyway. */ - memset(oid, 0, sizeof(*oid)); - result = 0; + git_submodule *sm; + const git_oid *sm_oid; + + if (!git_submodule_lookup(&sm, repo, item->path) && + (sm_oid = git_submodule_wd_oid(sm)) != NULL) + git_oid_cpy(oid, sm_oid); + else { + /* if submodule lookup failed probably just in an intermediate + * state where some init hasn't happened, so ignore the error + */ + giterr_clear(); + memset(oid, 0, sizeof(*oid)); + } } else if (S_ISLNK(item->mode)) result = git_odb__hashlink(oid, full_path.ptr); else if (!git__is_sizet(item->file_size)) { @@ -570,6 +584,15 @@ static int maybe_modified( return -1; status = GIT_SUBMODULE_STATUS_IS_UNMODIFIED(sm_status) ? GIT_DELTA_UNMODIFIED : GIT_DELTA_MODIFIED; + + /* grab OID while we are here */ + if (git_oid_iszero(&nitem->oid)) { + const git_oid *sm_oid = git_submodule_wd_oid(sub); + if (sub != NULL) { + git_oid_cpy(&noid, sm_oid); + use_noid = &noid; + } + } } } } diff --git a/src/diff_output.c b/src/diff_output.c index 9fee127c6..10fbd391c 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -275,30 +275,34 @@ static int get_workdir_sm_content( int error = 0; git_buf content = GIT_BUF_INIT; git_submodule* sm = NULL; - const git_oid* sm_head = NULL; unsigned int sm_status = 0; const char* sm_status_text = ""; char oidstr[GIT_OID_HEXSZ+1]; - if ((error = git_submodule_lookup(&sm, ctxt->repo, file->path)) < 0) { + if ((error = git_submodule_lookup(&sm, ctxt->repo, file->path)) < 0 || + (error = git_submodule_status(&sm_status, sm)) < 0) return error; + + /* update OID if we didn't have it previously */ + if ((file->flags & GIT_DIFF_FILE_VALID_OID) == 0) { + const git_oid* sm_head; + + if ((sm_head = git_submodule_wd_oid(sm)) != NULL || + (sm_head = git_submodule_head_oid(sm)) != NULL) + { + git_oid_cpy(&file->oid, sm_head); + file->flags |= GIT_DIFF_FILE_VALID_OID; + } } - if ((sm_head = git_submodule_head_oid(sm)) == NULL) { - giterr_set(GITERR_SUBMODULE, "Cannot find head of submodule '%s'", file->path); - return -1; - } + git_oid_fmt(oidstr, &file->oid); + oidstr[GIT_OID_HEXSZ] = '\0'; - if ((error = git_submodule_status(&sm_status, sm)) < 0) { - return -1; - } - if (!GIT_SUBMODULE_STATUS_IS_UNMODIFIED(sm_status)) { + if (GIT_SUBMODULE_STATUS_IS_WD_DIRTY(sm_status)) sm_status_text = "-dirty"; - } - git_oid_fmt(oidstr, sm_head); - oidstr[GIT_OID_HEXSZ] = 0; - git_buf_printf(&content, "Subproject commit %s%s\n", oidstr, sm_status_text ); + git_buf_printf(&content, "Subproject commit %s%s\n", + oidstr, sm_status_text); map->data = git_buf_detach(&content); map->len = strlen(map->data); @@ -318,7 +322,7 @@ static int get_workdir_content( git_buf path = GIT_BUF_INIT; const char *wd = git_repository_workdir(ctxt->repo); - if (file->mode == GIT_FILEMODE_COMMIT) + if (S_ISGITLINK(file->mode)) return get_workdir_sm_content(ctxt, file, map); if (S_ISDIR(file->mode)) diff --git a/tests-clar/diff/workdir.c b/tests-clar/diff/workdir.c index 916113178..e184c28b4 100644 --- a/tests-clar/diff/workdir.c +++ b/tests-clar/diff/workdir.c @@ -744,3 +744,77 @@ void test_diff_workdir__larger_hunks(void) git_tree_free(a); git_tree_free(b); } + +/* Set up a test that exercises this code. The easiest test using existing + * test data is probably to create a sandbox of submod2 and then run a + * git_diff_workdir_to_tree against tree + * 873585b94bdeabccea991ea5e3ec1a277895b698. As for what you should actually + * test, you can start by just checking that the number of lines of diff + * content matches the actual output of git diff. That will at least + * demonstrate that the submodule content is being used to generate somewhat + * comparable outputs. It is a test that would fail without this code and + * will succeed with it. + */ + +#include "../submodule/submodule_helpers.h" + +void test_diff_workdir__submodules(void) +{ + const char *a_commit = "873585b94bdeabccea991ea5e3ec1a277895b698"; + git_tree *a; + git_diff_options opts = {0}; + git_diff_list *diff = NULL; + diff_expects exp; + + 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"); + + cl_fixture_cleanup("submod2_target"); + + a = resolve_commit_oid_to_tree(g_repo, a_commit); + + opts.flags = + GIT_DIFF_INCLUDE_UNTRACKED | + GIT_DIFF_RECURSE_UNTRACKED_DIRS | + GIT_DIFF_INCLUDE_UNTRACKED_CONTENT; + + cl_git_pass(git_diff_workdir_to_tree(g_repo, &opts, a, &diff)); + + /* diff_print(stderr, diff); */ + + /* essentially doing: git diff 873585b94bdeabccea991ea5e3ec1a277895b698 */ + + memset(&exp, 0, sizeof(exp)); + cl_git_pass(git_diff_foreach( + diff, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn)); + + /* the following differs from "git diff 873585" by one "untracked" file + * because the diff list includes the "not_submodule/" directory which + * is not displayed in the text diff. + */ + + cl_assert_equal_i(10, exp.files); + + cl_assert_equal_i(0, exp.file_adds); + cl_assert_equal_i(0, exp.file_dels); + cl_assert_equal_i(1, exp.file_mods); + cl_assert_equal_i(0, exp.file_ignored); + cl_assert_equal_i(9, exp.file_untracked); + + /* the following numbers match "git diff 873585" exactly */ + + cl_assert_equal_i(9, exp.hunks); + + cl_assert_equal_i(33, exp.lines); + cl_assert_equal_i(2, exp.line_ctxt); + cl_assert_equal_i(30, exp.line_adds); + cl_assert_equal_i(1, exp.line_dels); + + git_diff_list_free(diff); + git_tree_free(a); +}