From 125fd98434ce773de45c4a40927c222ec5c43ae1 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 24 Jan 2010 00:10:20 -0800 Subject: [PATCH 1/3] Make ce_uptodate() trustworthy again The rule has always been that a cache entry that is ce_uptodate(ce) means that we already have checked the work tree entity and we know there is no change in the work tree compared to the index, and nobody should have to double check. Note that false ce_uptodate(ce) does not mean it is known to be dirty---it only means we don't know if it is clean. There are a few codepaths (refresh-index and preload-index are among them) that mark a cache entry as up-to-date based solely on the return value from ie_match_stat(); this function uses lstat() to see if the work tree entity has been touched, and for a submodule entry, if its HEAD points at the same commit as the commit recorded in the index of the superproject (a submodule that is not even cloned is considered clean). A submodule is no longer considered unmodified merely because its HEAD matches the index of the superproject these days, in order to prevent people from forgetting to commit in the submodule and updating the superproject index with the new submodule commit, before commiting the state in the superproject. However, the patch to do so didn't update the codepath that marks cache entries up-to-date based on the updated definition and instead worked it around by saying "we don't trust the return value of ce_uptodate() for submodules." This makes ce_uptodate() trustworthy again by not marking submodule entries up-to-date. The next step _could_ be to introduce a few "in-core" flag bits to cache_entry structure to record "this entry is _known_ to be dirty", call is_submodule_modified() from ie_match_stat(), and use these new bits to avoid running this rather expensive check more than once, but that can be a separate patch. Signed-off-by: Junio C Hamano --- diff-lib.c | 2 +- preload-index.c | 2 ++ read-cache.c | 6 ++++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/diff-lib.c b/diff-lib.c index 23e180eed1..c6c425e624 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -161,7 +161,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option) continue; } - if ((ce_uptodate(ce) && !S_ISGITLINK(ce->ce_mode)) || ce_skip_worktree(ce)) + if (ce_uptodate(ce) || ce_skip_worktree(ce)) continue; /* If CE_VALID is set, don't look at workdir for file removal */ diff --git a/preload-index.c b/preload-index.c index 92899333c2..e3d0bda31a 100644 --- a/preload-index.c +++ b/preload-index.c @@ -47,6 +47,8 @@ static void *preload_thread(void *_data) if (ce_stage(ce)) continue; + if (S_ISGITLINK(ce->ce_mode)) + continue; if (ce_uptodate(ce)) continue; if (!ce_path_match(ce, p->pathspec)) diff --git a/read-cache.c b/read-cache.c index 79938bf09a..309b77a6c9 100644 --- a/read-cache.c +++ b/read-cache.c @@ -612,7 +612,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st, if (alias && !ce_stage(alias) && !ie_match_stat(istate, alias, st, ce_option)) { /* Nothing changed, really */ free(ce); - ce_mark_uptodate(alias); + if (!S_ISGITLINK(alias->ce_mode)) + ce_mark_uptodate(alias); alias->ce_flags |= CE_ADDED; return 0; } @@ -1050,7 +1051,8 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate, * because CE_UPTODATE flag is in-core only; * we are not going to write this change out. */ - ce_mark_uptodate(ce); + if (!S_ISGITLINK(ce->ce_mode)) + ce_mark_uptodate(ce); return ce; } } From 4d34477f4c5dbebc55aa1362fd705440590a85f1 Mon Sep 17 00:00:00 2001 From: Jens Lehmann Date: Sat, 23 Jan 2010 17:37:26 +0100 Subject: [PATCH 2/3] git diff: Don't test submodule dirtiness with --ignore-submodules The diff family suppresses the output of submodule changes when requested but checks them nonetheless. But since recently submodules get examined for their dirtiness, which is rather expensive. There is no need to do that when the --ignore-submodules option is used, as the gathered information is never used anyway. Signed-off-by: Jens Lehmann Signed-off-by: Junio C Hamano --- diff-lib.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/diff-lib.c b/diff-lib.c index c6c425e624..899034d354 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -179,6 +179,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option) } changed = ce_match_stat(ce, &st, ce_option); if (S_ISGITLINK(ce->ce_mode) + && !DIFF_OPT_TST(&revs->diffopt, IGNORE_SUBMODULES) && (!changed || (revs->diffopt.output_format & DIFF_FORMAT_PATCH)) && is_submodule_modified(ce->name)) { changed = 1; @@ -220,7 +221,7 @@ static int get_stat_data(struct cache_entry *ce, const unsigned char **sha1p, unsigned int *modep, int cached, int match_missing, - unsigned *dirty_submodule, int output_format) + unsigned *dirty_submodule, struct diff_options *diffopt) { const unsigned char *sha1 = ce->sha1; unsigned int mode = ce->ce_mode; @@ -241,7 +242,8 @@ static int get_stat_data(struct cache_entry *ce, } changed = ce_match_stat(ce, &st, 0); if (S_ISGITLINK(ce->ce_mode) - && (!changed || (output_format & DIFF_FORMAT_PATCH)) + && !DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES) + && (!changed || (diffopt->output_format & DIFF_FORMAT_PATCH)) && is_submodule_modified(ce->name)) { changed = 1; *dirty_submodule = 1; @@ -270,7 +272,7 @@ static void show_new_file(struct rev_info *revs, * the working copy. */ if (get_stat_data(new, &sha1, &mode, cached, match_missing, - &dirty_submodule, revs->diffopt.output_format) < 0) + &dirty_submodule, &revs->diffopt) < 0) return; diff_index_show_file(revs, "+", new, sha1, mode, dirty_submodule); @@ -287,7 +289,7 @@ static int show_modified(struct rev_info *revs, unsigned dirty_submodule = 0; if (get_stat_data(new, &sha1, &mode, cached, match_missing, - &dirty_submodule, revs->diffopt.output_format) < 0) { + &dirty_submodule, &revs->diffopt) < 0) { if (report_missing) diff_index_show_file(revs, "-", old, old->sha1, old->ce_mode, 0); From 721ceec1ad12625e90b395fd0be0fae9049ebc22 Mon Sep 17 00:00:00 2001 From: Jens Lehmann Date: Sun, 24 Jan 2010 15:09:00 +0100 Subject: [PATCH 3/3] Teach diff --submodule that modified submodule directory is dirty Since commit 8e08b4 git diff does append "-dirty" to the work tree side if the working directory of a submodule contains new or modified files. Lets do the same when the --submodule option is used. Signed-off-by: Jens Lehmann Signed-off-by: Junio C Hamano --- diff.c | 2 +- submodule.c | 3 ++ submodule.h | 1 + t/t4041-diff-submodule.sh | 67 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 1 deletion(-) diff --git a/diff.c b/diff.c index 160dbfd718..0190ec6d9f 100644 --- a/diff.c +++ b/diff.c @@ -1615,7 +1615,7 @@ static void builtin_diff(const char *name_a, const char *del = diff_get_color_opt(o, DIFF_FILE_OLD); const char *add = diff_get_color_opt(o, DIFF_FILE_NEW); show_submodule_summary(o->file, one ? one->path : two->path, - one->sha1, two->sha1, + one->sha1, two->sha1, two->dirty_submodule, del, add, reset); return; } diff --git a/submodule.c b/submodule.c index f657bee379..ca0527fbcb 100644 --- a/submodule.c +++ b/submodule.c @@ -36,6 +36,7 @@ static int add_submodule_odb(const char *path) void show_submodule_summary(FILE *f, const char *path, unsigned char one[20], unsigned char two[20], + unsigned dirty_submodule, const char *del, const char *add, const char *reset) { struct rev_info rev; @@ -85,6 +86,8 @@ void show_submodule_summary(FILE *f, const char *path, if (!fast_backward && !fast_forward) strbuf_addch(&sb, '.'); strbuf_addf(&sb, "%s", find_unique_abbrev(two, DEFAULT_ABBREV)); + if (dirty_submodule) + strbuf_add(&sb, "-dirty", 6); if (message) strbuf_addf(&sb, " %s\n", message); else diff --git a/submodule.h b/submodule.h index 0773121eb5..233696555e 100644 --- a/submodule.h +++ b/submodule.h @@ -3,6 +3,7 @@ void show_submodule_summary(FILE *f, const char *path, unsigned char one[20], unsigned char two[20], + unsigned dirty_submodule, const char *del, const char *add, const char *reset); int is_submodule_modified(const char *path); diff --git a/t/t4041-diff-submodule.sh b/t/t4041-diff-submodule.sh index 5bb4fed3f5..464305405a 100755 --- a/t/t4041-diff-submodule.sh +++ b/t/t4041-diff-submodule.sh @@ -191,6 +191,73 @@ EOF " commit_file sm1 && +test_expect_success 'submodule is up to date' " + git diff-index -p --submodule=log HEAD >actual && + diff actual - <<-EOF +EOF +" + +test_expect_success 'submodule contains untracked content' " + echo new > sm1/new-file && + git diff-index -p --submodule=log HEAD >actual && + diff actual - <<-EOF +Submodule sm1 $head6..$head6-dirty: +EOF +" + +test_expect_success 'submodule contains untracked and modifed content' " + echo new > sm1/foo6 && + git diff-index -p --submodule=log HEAD >actual && + diff actual - <<-EOF +Submodule sm1 $head6..$head6-dirty: +EOF +" + +test_expect_success 'submodule contains modifed content' " + rm -f sm1/new-file && + git diff-index -p --submodule=log HEAD >actual && + diff actual - <<-EOF +Submodule sm1 $head6..$head6-dirty: +EOF +" + +(cd sm1; git commit -mchange foo6 >/dev/null) && +head8=$(cd sm1; git rev-parse --verify HEAD | cut -c1-7) && +test_expect_success 'submodule is modified' " + git diff-index -p --submodule=log HEAD >actual && + diff actual - <<-EOF +Submodule sm1 $head6..$head8: + > change +EOF +" + +test_expect_success 'modified submodule contains untracked content' " + echo new > sm1/new-file && + git diff-index -p --submodule=log HEAD >actual && + diff actual - <<-EOF +Submodule sm1 $head6..$head8-dirty: + > change +EOF +" + +test_expect_success 'modified submodule contains untracked and modifed content' " + echo modification >> sm1/foo6 && + git diff-index -p --submodule=log HEAD >actual && + diff actual - <<-EOF +Submodule sm1 $head6..$head8-dirty: + > change +EOF +" + +test_expect_success 'modified submodule contains modifed content' " + rm -f sm1/new-file && + git diff-index -p --submodule=log HEAD >actual && + diff actual - <<-EOF +Submodule sm1 $head6..$head8-dirty: + > change +EOF +" + rm -rf sm1 test_expect_success 'deleted submodule' " git diff-index -p --submodule=log HEAD >actual &&