diff options
author | Junio C Hamano <gitster@pobox.com> | 2010-01-22 16:08:10 -0800 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2010-01-22 16:08:10 -0800 |
commit | c6ec7efdd4c63f5c53184651cb05748e7bc71075 (patch) | |
tree | 4f9d0de8273ca2d608e172367b167c0b1774b05c | |
parent | Merge branch 'il/remote-updates' (diff) | |
parent | Performance optimization for detection of modified submodules (diff) | |
download | tgif-c6ec7efdd4c63f5c53184651cb05748e7bc71075.tar.xz |
Merge branch 'jl/submodule-diff'
* jl/submodule-diff:
Performance optimization for detection of modified submodules
git status: Show uncommitted submodule changes too when enabled
Teach diff that modified submodule directory is dirty
Show submodules as modified when they contain a dirty work tree
-rw-r--r-- | diff-lib.c | 44 | ||||
-rw-r--r-- | diff.c | 22 | ||||
-rw-r--r-- | diff.h | 10 | ||||
-rw-r--r-- | diffcore.h | 1 | ||||
-rwxr-xr-x | git-submodule.sh | 6 | ||||
-rw-r--r-- | revision.c | 5 | ||||
-rw-r--r-- | submodule.c | 49 | ||||
-rw-r--r-- | submodule.h | 1 | ||||
-rwxr-xr-x | t/t4027-diff-submodule.sh | 84 | ||||
-rwxr-xr-x | t/t7401-submodule-summary.sh | 2 | ||||
-rwxr-xr-x | t/t7506-status-submodule.sh | 83 | ||||
-rwxr-xr-x | t/t7508-status.sh | 4 | ||||
-rw-r--r-- | tree-diff.c | 8 | ||||
-rw-r--r-- | wt-status.c | 12 |
14 files changed, 279 insertions, 52 deletions
diff --git a/diff-lib.c b/diff-lib.c index 1c7e652a80..23e180eed1 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -10,6 +10,7 @@ #include "cache-tree.h" #include "unpack-trees.h" #include "refs.h" +#include "submodule.h" /* * diff-files @@ -72,6 +73,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option) unsigned int oldmode, newmode; struct cache_entry *ce = active_cache[i]; int changed; + unsigned dirty_submodule = 0; if (DIFF_OPT_TST(&revs->diffopt, QUICK) && DIFF_OPT_TST(&revs->diffopt, HAS_CHANGES)) @@ -159,7 +161,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option) continue; } - if (ce_uptodate(ce) || ce_skip_worktree(ce)) + if ((ce_uptodate(ce) && !S_ISGITLINK(ce->ce_mode)) || ce_skip_worktree(ce)) continue; /* If CE_VALID is set, don't look at workdir for file removal */ @@ -172,10 +174,16 @@ int run_diff_files(struct rev_info *revs, unsigned int option) if (silent_on_removed) continue; diff_addremove(&revs->diffopt, '-', ce->ce_mode, - ce->sha1, ce->name); + ce->sha1, ce->name, 0); continue; } changed = ce_match_stat(ce, &st, ce_option); + if (S_ISGITLINK(ce->ce_mode) + && (!changed || (revs->diffopt.output_format & DIFF_FORMAT_PATCH)) + && is_submodule_modified(ce->name)) { + changed = 1; + dirty_submodule = 1; + } if (!changed) { ce_mark_uptodate(ce); if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER)) @@ -185,7 +193,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option) newmode = ce_mode_from_stat(ce, st.st_mode); diff_change(&revs->diffopt, oldmode, newmode, ce->sha1, (changed ? null_sha1 : ce->sha1), - ce->name); + ce->name, 0, dirty_submodule); } diffcore_std(&revs->diffopt); @@ -201,16 +209,18 @@ int run_diff_files(struct rev_info *revs, unsigned int option) static void diff_index_show_file(struct rev_info *revs, const char *prefix, struct cache_entry *ce, - const unsigned char *sha1, unsigned int mode) + const unsigned char *sha1, unsigned int mode, + unsigned dirty_submodule) { diff_addremove(&revs->diffopt, prefix[0], mode, - sha1, ce->name); + sha1, ce->name, dirty_submodule); } static int get_stat_data(struct cache_entry *ce, const unsigned char **sha1p, unsigned int *modep, - int cached, int match_missing) + int cached, int match_missing, + unsigned *dirty_submodule, int output_format) { const unsigned char *sha1 = ce->sha1; unsigned int mode = ce->ce_mode; @@ -230,6 +240,12 @@ static int get_stat_data(struct cache_entry *ce, return -1; } changed = ce_match_stat(ce, &st, 0); + if (S_ISGITLINK(ce->ce_mode) + && (!changed || (output_format & DIFF_FORMAT_PATCH)) + && is_submodule_modified(ce->name)) { + changed = 1; + *dirty_submodule = 1; + } if (changed) { mode = ce_mode_from_stat(ce, st.st_mode); sha1 = null_sha1; @@ -247,15 +263,17 @@ static void show_new_file(struct rev_info *revs, { const unsigned char *sha1; unsigned int mode; + unsigned dirty_submodule = 0; /* * New file in the index: it might actually be different in * the working copy. */ - if (get_stat_data(new, &sha1, &mode, cached, match_missing) < 0) + if (get_stat_data(new, &sha1, &mode, cached, match_missing, + &dirty_submodule, revs->diffopt.output_format) < 0) return; - diff_index_show_file(revs, "+", new, sha1, mode); + diff_index_show_file(revs, "+", new, sha1, mode, dirty_submodule); } static int show_modified(struct rev_info *revs, @@ -266,11 +284,13 @@ static int show_modified(struct rev_info *revs, { unsigned int mode, oldmode; const unsigned char *sha1; + unsigned dirty_submodule = 0; - if (get_stat_data(new, &sha1, &mode, cached, match_missing) < 0) { + if (get_stat_data(new, &sha1, &mode, cached, match_missing, + &dirty_submodule, revs->diffopt.output_format) < 0) { if (report_missing) diff_index_show_file(revs, "-", old, - old->sha1, old->ce_mode); + old->sha1, old->ce_mode, 0); return -1; } @@ -305,7 +325,7 @@ static int show_modified(struct rev_info *revs, return 0; diff_change(&revs->diffopt, oldmode, mode, - old->sha1, sha1, old->name); + old->sha1, sha1, old->name, 0, dirty_submodule); return 0; } @@ -352,7 +372,7 @@ static void do_oneway_diff(struct unpack_trees_options *o, * Something removed from the tree? */ if (!idx) { - diff_index_show_file(revs, "-", tree, tree->sha1, tree->ce_mode); + diff_index_show_file(revs, "-", tree, tree->sha1, tree->ce_mode, 0); return; } @@ -2029,9 +2029,14 @@ static int populate_from_stdin(struct diff_filespec *s) static int diff_populate_gitlink(struct diff_filespec *s, int size_only) { int len; - char *data = xmalloc(100); + char *data = xmalloc(100), *dirty = ""; + + /* Are we looking at the work tree? */ + if (!s->sha1_valid && s->dirty_submodule) + dirty = "-dirty"; + len = snprintf(data, 100, - "Subproject commit %s\n", sha1_to_hex(s->sha1)); + "Subproject commit %s%s\n", sha1_to_hex(s->sha1), dirty); s->data = data; s->size = len; s->should_free = 1; @@ -3714,7 +3719,7 @@ int diff_result_code(struct diff_options *opt, int status) void diff_addremove(struct diff_options *options, int addremove, unsigned mode, const unsigned char *sha1, - const char *concatpath) + const char *concatpath, unsigned dirty_submodule) { struct diff_filespec *one, *two; @@ -3746,8 +3751,10 @@ void diff_addremove(struct diff_options *options, if (addremove != '+') fill_filespec(one, sha1, mode); - if (addremove != '-') + if (addremove != '-') { fill_filespec(two, sha1, mode); + two->dirty_submodule = dirty_submodule; + } diff_queue(&diff_queued_diff, one, two); if (!DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) @@ -3758,7 +3765,8 @@ void diff_change(struct diff_options *options, unsigned old_mode, unsigned new_mode, const unsigned char *old_sha1, const unsigned char *new_sha1, - const char *concatpath) + const char *concatpath, + unsigned old_dirty_submodule, unsigned new_dirty_submodule) { struct diff_filespec *one, *two; @@ -3771,6 +3779,8 @@ void diff_change(struct diff_options *options, const unsigned char *tmp_c; tmp = old_mode; old_mode = new_mode; new_mode = tmp; tmp_c = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_c; + tmp = old_dirty_submodule; old_dirty_submodule = new_dirty_submodule; + new_dirty_submodule = tmp; } if (options->prefix && @@ -3781,6 +3791,8 @@ void diff_change(struct diff_options *options, two = alloc_filespec(concatpath); fill_filespec(one, old_sha1, old_mode); fill_filespec(two, new_sha1, new_mode); + one->dirty_submodule = old_dirty_submodule; + two->dirty_submodule = new_dirty_submodule; diff_queue(&diff_queued_diff, one, two); if (!DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) @@ -14,12 +14,13 @@ typedef void (*change_fn_t)(struct diff_options *options, unsigned old_mode, unsigned new_mode, const unsigned char *old_sha1, const unsigned char *new_sha1, - const char *fullpath); + const char *fullpath, + unsigned old_dirty_submodule, unsigned new_dirty_submodule); typedef void (*add_remove_fn_t)(struct diff_options *options, int addremove, unsigned mode, const unsigned char *sha1, - const char *fullpath); + const char *fullpath, unsigned dirty_submodule); typedef void (*diff_format_fn_t)(struct diff_queue_struct *q, struct diff_options *options, void *data); @@ -177,13 +178,14 @@ extern void diff_addremove(struct diff_options *, int addremove, unsigned mode, const unsigned char *sha1, - const char *fullpath); + const char *fullpath, unsigned dirty_submodule); extern void diff_change(struct diff_options *, unsigned mode1, unsigned mode2, const unsigned char *sha1, const unsigned char *sha2, - const char *fullpath); + const char *fullpath, + unsigned dirty_submodule1, unsigned dirty_submodule2); extern void diff_unmerge(struct diff_options *, const char *path, diff --git a/diffcore.h b/diffcore.h index 5b634585e8..66687c3fe5 100644 --- a/diffcore.h +++ b/diffcore.h @@ -42,6 +42,7 @@ struct diff_filespec { #define DIFF_FILE_VALID(spec) (((spec)->mode) != 0) unsigned should_free : 1; /* data should be free()'ed */ unsigned should_munmap : 1; /* data should be munmap()'ed */ + unsigned dirty_submodule : 1; /* For submodules: its work tree is dirty */ struct userdiff_driver *driver; /* data should be considered "binary"; -1 means "don't know yet" */ diff --git a/git-submodule.sh b/git-submodule.sh index 77d223292c..664f21721c 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -688,7 +688,11 @@ cmd_summary() { echo done | if test -n "$for_status"; then - echo "# Modified submodules:" + if [ -n "$files" ]; then + echo "# Submodules changed but not updated:" + else + echo "# Submodule changes to be committed:" + fi echo "#" sed -e 's|^|# |' -e 's|^# $|#|' else diff --git a/revision.c b/revision.c index f3b82d97bc..4e1a299bfb 100644 --- a/revision.c +++ b/revision.c @@ -268,7 +268,7 @@ static int tree_difference = REV_TREE_SAME; static void file_add_remove(struct diff_options *options, int addremove, unsigned mode, const unsigned char *sha1, - const char *fullpath) + const char *fullpath, unsigned dirty_submodule) { int diff = addremove == '+' ? REV_TREE_NEW : REV_TREE_OLD; @@ -281,7 +281,8 @@ static void file_change(struct diff_options *options, unsigned old_mode, unsigned new_mode, const unsigned char *old_sha1, const unsigned char *new_sha1, - const char *fullpath) + const char *fullpath, + unsigned old_dirty_submodule, unsigned new_dirty_submodule) { tree_difference = REV_TREE_DIFFERENT; DIFF_OPT_SET(options, HAS_CHANGES); diff --git a/submodule.c b/submodule.c index 3007f7d5a6..f657bee379 100644 --- a/submodule.c +++ b/submodule.c @@ -4,6 +4,7 @@ #include "diff.h" #include "commit.h" #include "revision.h" +#include "run-command.h" static int add_submodule_odb(const char *path) { @@ -112,3 +113,51 @@ void show_submodule_summary(FILE *f, const char *path, } strbuf_release(&sb); } + +int is_submodule_modified(const char *path) +{ + int len; + struct child_process cp; + const char *argv[] = { + "status", + "--porcelain", + NULL, + }; + char *env[3]; + struct strbuf buf = STRBUF_INIT; + + strbuf_addf(&buf, "%s/.git/", path); + if (!is_directory(buf.buf)) { + strbuf_release(&buf); + /* The submodule is not checked out, so it is not modified */ + return 0; + + } + strbuf_reset(&buf); + + strbuf_addf(&buf, "GIT_WORK_TREE=%s", path); + env[0] = strbuf_detach(&buf, NULL); + strbuf_addf(&buf, "GIT_DIR=%s/.git", path); + env[1] = strbuf_detach(&buf, NULL); + env[2] = NULL; + + memset(&cp, 0, sizeof(cp)); + cp.argv = argv; + cp.env = (const char *const *)env; + cp.git_cmd = 1; + cp.no_stdin = 1; + cp.out = -1; + if (start_command(&cp)) + die("Could not run git status --porcelain"); + + len = strbuf_read(&buf, cp.out, 1024); + close(cp.out); + + if (finish_command(&cp)) + die("git status --porcelain failed"); + + free(env[0]); + free(env[1]); + strbuf_release(&buf); + return len != 0; +} diff --git a/submodule.h b/submodule.h index 4c0269d679..0773121eb5 100644 --- a/submodule.h +++ b/submodule.h @@ -4,5 +4,6 @@ void show_submodule_summary(FILE *f, const char *path, unsigned char one[20], unsigned char two[20], const char *del, const char *add, const char *reset); +int is_submodule_modified(const char *path); #endif diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh index 5cf8924b21..83c1914771 100755 --- a/t/t4027-diff-submodule.sh +++ b/t/t4027-diff-submodule.sh @@ -32,7 +32,8 @@ test_expect_success setup ' cd sub && git rev-list HEAD ) && - echo ":160000 160000 $3 $_z40 M sub" >expect + echo ":160000 160000 $3 $_z40 M sub" >expect && + subtip=$3 subprev=$2 ' test_expect_success 'git diff --raw HEAD' ' @@ -50,6 +51,87 @@ test_expect_success 'git diff-files --raw' ' test_cmp expect actual.files ' +expect_from_to () { + printf "%sSubproject commit %s\n+Subproject commit %s\n" \ + "-" "$1" "$2" +} + +test_expect_success 'git diff HEAD' ' + git diff HEAD >actual && + sed -e "1,/^@@/d" actual >actual.body && + expect_from_to >expect.body $subtip $subprev && + test_cmp expect.body actual.body +' + +test_expect_success 'git diff HEAD with dirty submodule (work tree)' ' + echo >>sub/world && + git diff HEAD >actual && + sed -e "1,/^@@/d" actual >actual.body && + expect_from_to >expect.body $subtip $subprev-dirty && + test_cmp expect.body actual.body +' + +test_expect_success 'git diff HEAD with dirty submodule (index)' ' + ( + cd sub && + git reset --hard && + echo >>world && + git add world + ) && + git diff HEAD >actual && + sed -e "1,/^@@/d" actual >actual.body && + expect_from_to >expect.body $subtip $subprev-dirty && + test_cmp expect.body actual.body +' + +test_expect_success 'git diff HEAD with dirty submodule (untracked)' ' + ( + cd sub && + git reset --hard && + git clean -qfdx && + >cruft + ) && + git diff HEAD >actual && + sed -e "1,/^@@/d" actual >actual.body && + expect_from_to >expect.body $subtip $subprev-dirty && + test_cmp expect.body actual.body +' + +test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)' ' + git commit -m "x" sub && + echo >>sub/world && + git diff HEAD >actual && + sed -e "1,/^@@/d" actual >actual.body && + expect_from_to >expect.body $subprev $subprev-dirty && + test_cmp expect.body actual.body +' + +test_expect_success 'git diff HEAD with dirty submodule (index, refs match)' ' + ( + cd sub && + git reset --hard && + echo >>world && + git add world + ) && + git diff HEAD >actual && + sed -e "1,/^@@/d" actual >actual.body && + expect_from_to >expect.body $subprev $subprev-dirty && + test_cmp expect.body actual.body +' + +test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)' ' + ( + cd sub && + git reset --hard && + git clean -qfdx && + >cruft + ) && + git diff HEAD >actual && + sed -e "1,/^@@/d" actual >actual.body && + expect_from_to >expect.body $subprev $subprev-dirty && + test_cmp expect.body actual.body +' + test_expect_success 'git diff (empty submodule dir)' ' : >empty && rm -rf sub/* sub/.git && diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh index 6cc16c39fe..d3c039f724 100755 --- a/t/t7401-submodule-summary.sh +++ b/t/t7401-submodule-summary.sh @@ -213,7 +213,7 @@ EOF test_expect_success '--for-status' " git submodule summary --for-status HEAD^ >actual && test_cmp actual - <<EOF -# Modified submodules: +# Submodule changes to be committed: # # * sm1 $head6...0000000: # diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh index 3ca17abad1..253c334319 100755 --- a/t/t7506-status-submodule.sh +++ b/t/t7506-status-submodule.sh @@ -5,34 +5,87 @@ test_description='git status for submodule' . ./test-lib.sh test_expect_success 'setup' ' - test_create_repo sub - cd sub && - : >bar && - git add bar && - git commit -m " Add bar" && - cd .. && - git add sub && + test_create_repo sub && + ( + cd sub && + : >bar && + git add bar && + git commit -m " Add bar" && + : >foo && + git add foo && + git commit -m " Add foo" + ) && + echo output > .gitignore && + git add sub .gitignore && git commit -m "Add submodule sub" ' test_expect_success 'status clean' ' - git status | - grep "nothing to commit" + git status >output && + grep "nothing to commit" output ' + test_expect_success 'commit --dry-run -a clean' ' - git commit --dry-run -a | - grep "nothing to commit" + test_must_fail git commit --dry-run -a >output && + grep "nothing to commit" output +' + +test_expect_success 'status with modified file in submodule' ' + (cd sub && git reset --hard) && + echo "changed" >sub/foo && + git status >output && + grep "modified: sub" output +' + +test_expect_success 'status with modified file in submodule (porcelain)' ' + (cd sub && git reset --hard) && + echo "changed" >sub/foo && + git status --porcelain >output && + diff output - <<-\EOF + M sub + EOF +' + +test_expect_success 'status with added file in submodule' ' + (cd sub && git reset --hard && echo >foo && git add foo) && + git status >output && + grep "modified: sub" output +' + +test_expect_success 'status with added file in submodule (porcelain)' ' + (cd sub && git reset --hard && echo >foo && git add foo) && + git status --porcelain >output && + diff output - <<-\EOF + M sub + EOF +' + +test_expect_success 'status with untracked file in submodule' ' + (cd sub && git reset --hard) && + echo "content" >sub/new-file && + git status >output && + grep "modified: sub" output +' + +test_expect_success 'status with untracked file in submodule (porcelain)' ' + git status --porcelain >output && + diff output - <<-\EOF + M sub + EOF ' + test_expect_success 'rm submodule contents' ' rm -rf sub/* sub/.git ' + test_expect_success 'status clean (empty submodule dir)' ' - git status | - grep "nothing to commit" + git status >output && + grep "nothing to commit" output ' + test_expect_success 'status -a clean (empty submodule dir)' ' - git commit --dry-run -a | - grep "nothing to commit" + test_must_fail git commit --dry-run -a >output && + grep "nothing to commit" output ' test_done diff --git a/t/t7508-status.sh b/t/t7508-status.sh index cf67fe3a4a..556d0faa77 100755 --- a/t/t7508-status.sh +++ b/t/t7508-status.sh @@ -579,7 +579,7 @@ cat >expect <<EOF # # modified: dir1/modified # -# Modified submodules: +# Submodule changes to be committed: # # * sm 0000000...$head (1): # > Add foo @@ -672,7 +672,7 @@ cat >expect <<EOF # # modified: dir1/modified # -# Modified submodules: +# Submodule changes to be committed: # # * sm 0000000...$head (1): # > Add foo diff --git a/tree-diff.c b/tree-diff.c index 7d745b4406..fe9f52c479 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -68,7 +68,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE)) { newbase[baselen + pathlen1] = 0; opt->change(opt, mode1, mode2, - sha1, sha2, newbase); + sha1, sha2, newbase, 0, 0); newbase[baselen + pathlen1] = '/'; } retval = diff_tree_sha1(sha1, sha2, newbase, opt); @@ -77,7 +77,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const } fullname = malloc_fullname(base, baselen, path1, pathlen1); - opt->change(opt, mode1, mode2, sha1, sha2, fullname); + opt->change(opt, mode1, mode2, sha1, sha2, fullname, 0, 0); free(fullname); return 0; } @@ -241,7 +241,7 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE)) { newbase[baselen + pathlen] = 0; - opt->add_remove(opt, *prefix, mode, sha1, newbase); + opt->add_remove(opt, *prefix, mode, sha1, newbase, 0); newbase[baselen + pathlen] = '/'; } @@ -252,7 +252,7 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree free(newbase); } else { char *fullname = malloc_fullname(base, baselen, path, pathlen); - opt->add_remove(opt, prefix[0], mode, sha1, fullname); + opt->add_remove(opt, prefix[0], mode, sha1, fullname, 0); free(fullname); } } diff --git a/wt-status.c b/wt-status.c index 65feb29f2e..5807fc3211 100644 --- a/wt-status.c +++ b/wt-status.c @@ -459,7 +459,7 @@ static void wt_status_print_changed(struct wt_status *s) wt_status_print_trailer(s); } -static void wt_status_print_submodule_summary(struct wt_status *s) +static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitted) { struct child_process sm_summary; char summary_limit[64]; @@ -468,11 +468,11 @@ static void wt_status_print_submodule_summary(struct wt_status *s) const char *argv[] = { "submodule", "summary", - "--cached", + uncommitted ? "--files" : "--cached", "--for-status", "--summary-limit", summary_limit, - s->amend ? "HEAD^" : "HEAD", + uncommitted ? NULL : (s->amend ? "HEAD^" : "HEAD"), NULL }; @@ -580,8 +580,10 @@ void wt_status_print(struct wt_status *s) wt_status_print_updated(s); wt_status_print_unmerged(s); wt_status_print_changed(s); - if (s->submodule_summary) - wt_status_print_submodule_summary(s); + if (s->submodule_summary) { + wt_status_print_submodule_summary(s, 0); /* staged */ + wt_status_print_submodule_summary(s, 1); /* unstaged */ + } if (s->show_untracked_files) wt_status_print_untracked(s); else if (s->commitable) |