Re: [PATCH 3/4] Fix diff regression for submodules not checked out

[Junio C Hamano <gitster@pobox.com>]

Ping Yin <pkufranky@gmail.com> writes:

> This regression is introduced by f58dbf23c3, which calls
> check_work_tree_entity in run_diff_files.  While check_work_tree_entity
> treats submodule not checked out as non stagable which causes that
> diff-files shows these submodules as deleted.

>  int run_diff_files(struct rev_info *revs, unsigned int option)
> @@ -403,7 +407,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
>  			       sizeof(struct combine_diff_parent)*5);
>  
>  			changed = check_work_tree_entity(ce, &st, symcache);
> -			if (!changed)
> +			if (changed != ENT_INEXISTENT)
>  				dpath->mode = ce_mode_from_stat(ce, st.st_mode);

>  			else {
>  				if (changed < 0) {
> @@ -467,7 +471,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
>  			continue;
>  
>  		changed = check_work_tree_entity(ce, &st, symcache);
> -		if (changed) {
> +		if (changed == ENT_INEXISTENT) {
>  			if (changed < 0) {
>  				perror(ce->name);
>  				continue;
> @@ -527,7 +531,7 @@ static int get_stat_data(struct cache_entry *ce,
>  		changed = check_work_tree_entity(ce, &st, cbdata->symcache);
>  		if (changed < 0)
>  			return -1;
> -		else if (changed) {
> +		else if (changed == ENT_INEXISTENT) {
>  			if (match_missing) {
>  				*sha1p = sha1;
>  				*modep = mode;

Earier I said we may have to teach the Porcelain layer (status, diff) to
equate a submodule that is not checked out and a submodule that is not
modified while keeping low-level plumbing (diff-files and diff-index)
still aware that the submodule is missing from the work tree, but that was
because I incorrectly thought there are only two cases (either the
submodule is fully checked out or the submodule directory itself does not
even exist) and treating the latter the same as an unmodified case would
mean there won't be an easy way to remove the submodule from the
superproject for Porcelains that are written in terms of diff-files and
diff-index.

But that was a faulty thinking on my part (heh, why didn't anybody correct
me?).  Actually, there are three cases:

 - submodule directory exists and it is a full fledged repository.  It may
   or may not be modified, but we can tell by looking at its .git/HEAD.

 - submodule directory exists but there is nothing there (no "init" nor
   "update" was done, just an empty directory checked out).  This is how a
   superproject with a submodule is checked out by default.

 - submodule directory itself does not even exist.

The second case is "not checked out -- treat me as unmodified", and the
third case is "the user does not want the submodule there", and the latter
is still reported as "removed".  That is exactly what your patch does.

I like it.

By the way, "inexistent" is a word, but somehow it sounds quite awkward.
Perhaps one of NONEXISTENT (more common), REMOVED (run_diff_files() takes
a SILENT_ON_REMOVED option) or or MISSING (update-index --refresh takes an
IGNORE_MISSING option) is better?  I dunno.